fix(compiler-cli): correctly get the type of nested function call expressions (angular#57010)

This PR fixes a bug where the type of a nested function call expression was incorrectly being returned as null.

PR Close angular#57010

Author: eneajaho

packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/nullish_coalescing_not_nullable/nullish_coalescing_not_nullable_spec.ts

@@ -125,6 +125,29 @@ runInEachFileSystem(() => {
       expect(diags.length).toBe(0);
     });
 
+    it('should not produce diagnostics for nullish coalescing in a chain', () => {
+      const fileName = absoluteFrom('/main.ts');
+      const {program, templateTypeChecker} = setup([
+        {
+          fileName,
+          templates: {
+            'TestCmp': `{{ var1 !== '' && (items?.length ?? 0) > 0 }}`,
+          },
+          source: 'export class TestCmp { var1 = "text"; items: string[] | undefined = [] }',
+        },
+      ]);
+      const sf = getSourceFileOrError(program, fileName);
+      const component = getClass(sf, 'TestCmp');
+      const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
+        templateTypeChecker,
+        program.getTypeChecker(),
+        [nullishCoalescingNotNullableFactory],
+        {strictNullChecks: true} /* options */,
+      );
+      const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
+      expect(diags.length).toBe(0);
+    });
+
     it('should not produce nullish coalescing warning for the any type', () => {
       const fileName = absoluteFrom('/main.ts');
       const {program, templateTypeChecker} = setup([

packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts

@@ -8,6 +8,7 @@
 
 import {
   AST,
+  ASTWithName,
   ASTWithSource,
   BindingPipe,
   ParseSourceSpan,
@@ -708,9 +709,13 @@ export class SymbolBuilder {
 
     let withSpan = expression.sourceSpan;
 
-    // The `name` part of a `PropertyWrite` and a non-safe `Call` does not have its own
+    // The `name` part of a `PropertyWrite` and `ASTWithName` do not have their own
     // AST so there is no way to retrieve a `Symbol` for just the `name` via a specific node.
-    if (expression instanceof PropertyWrite) {
+    // Also skipping SafePropertyReads as it breaks nullish coalescing not nullable extended diagnostic
+    if (
+      expression instanceof PropertyWrite ||
+      (expression instanceof ASTWithName && !(expression instanceof SafePropertyRead))
+    ) {
       withSpan = expression.nameSpan;
     }
 
@@ -770,6 +775,8 @@ export class SymbolBuilder {
     let tsSymbol: ts.Symbol | undefined;
     if (ts.isPropertyAccessExpression(node)) {
       tsSymbol = this.getTypeChecker().getSymbolAtLocation(node.name);
+    } else if (ts.isCallExpression(node)) {
+      tsSymbol = this.getTypeChecker().getSymbolAtLocation(node.expression);
     } else {
       tsSymbol = this.getTypeChecker().getSymbolAtLocation(node);
     }

packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts

@@ -758,14 +758,15 @@ runInEachFileSystem(() => {
 
       it('should get a symbol for function on a component used in an input binding', () => {
         const fileName = absoluteFrom('/main.ts');
-        const templateString = `<div [inputA]="helloWorld"></div>`;
+        const templateString = `<div [inputA]="helloWorld" [nestedFunction]="nested.helloWorld1()"></div>`;
         const {templateTypeChecker, program} = setup([
           {
             fileName,
             templates: {'Cmp': templateString},
             source: `
             export class Cmp {
               helloWorld() { return ''; }
+              nested = { helloWorld1() { return ''; } };
             }`,
           },
         ]);
@@ -777,6 +778,13 @@ runInEachFileSystem(() => {
         assertExpressionSymbol(symbol);
         expect(program.getTypeChecker().symbolToString(symbol.tsSymbol!)).toEqual('helloWorld');
         expect(program.getTypeChecker().typeToString(symbol.tsType)).toEqual('() => string');
+
+        const nestedSymbol = templateTypeChecker.getSymbolOfNode(nodes[0].inputs[1].value, cmp)!;
+        assertExpressionSymbol(nestedSymbol);
+        expect(program.getTypeChecker().symbolToString(nestedSymbol.tsSymbol!)).toEqual(
+          'helloWorld1',
+        );
+        expect(program.getTypeChecker().typeToString(nestedSymbol.tsType)).toEqual('string');
       });
 
       it('should get a symbol for binary expressions', () => {
@@ -1118,8 +1126,15 @@ runInEachFileSystem(() => {
         const {templateTypeChecker, program} = setup([
           {
             fileName,
-            templates: {'Cmp': '<div [input]="toString(123)"></div>'},
-            source: `export class Cmp { toString(v: any): string { return String(v); } }`,
+            templates: {
+              'Cmp': '<div [input]="toString(123)" [nestedFunction]="nested.toString(123)"></div>',
+            },
+            source: `
+              export class Cmp {
+                toString(v: any): string { return String(v); }
+                nested = { toString(v: any): string { return String(v); } };
+              }
+            `,
           },
         ]);
         const sf = getSourceFileOrError(program, fileName);
@@ -1128,8 +1143,16 @@ runInEachFileSystem(() => {
         const callSymbol = templateTypeChecker.getSymbolOfNode(node.inputs[0].value, cmp)!;
         assertExpressionSymbol(callSymbol);
         // Note that the symbol returned is for the return value of the Call.
-        expect(callSymbol.tsSymbol).toBeNull();
+        expect(callSymbol.tsSymbol).toBeTruthy();
+        expect(callSymbol.tsSymbol?.getName()).toEqual('toString');
         expect(program.getTypeChecker().typeToString(callSymbol.tsType)).toBe('string');
+
+        const nestedCallSymbol = templateTypeChecker.getSymbolOfNode(node.inputs[1].value, cmp)!;
+        assertExpressionSymbol(nestedCallSymbol);
+        // Note that the symbol returned is for the return value of the Call.
+        expect(nestedCallSymbol.tsSymbol).toBeTruthy();
+        expect(nestedCallSymbol.tsSymbol?.getName()).toEqual('toString');
+        expect(program.getTypeChecker().typeToString(nestedCallSymbol.tsType)).toBe('string');
       });
 
       it('should get a symbol for SafeCall expressions', () => {

packages/language-service/src/template_target.ts

@@ -229,6 +229,7 @@ export function getTargetAtPosition(
   }
 
   const candidate = path[path.length - 1];
+
   // Walk up the result nodes to find the nearest `TmplAstTemplate` which contains the targeted
   // node.
   let context: TmplAstTemplate | null = null;

packages/language-service/test/legacy/template_target_spec.ts

@@ -155,6 +155,15 @@ describe('getTargetAtPosition for template AST', () => {
     expect(node).toBeInstanceOf(e.PropertyRead);
   });
 
+  it('should locate bound event nested value', () => {
+    const {errors, nodes, position} = parse(`<test-cmp (foo)="nested.b¦ar()"></test-cmp>`);
+    expect(errors).toBe(null);
+    const {context} = getTargetAtPosition(nodes, position)!;
+    const {node} = context as SingleNodeTarget;
+    expect(isExpressionNode(node!)).toBe(true);
+    expect(node).toBeInstanceOf(e.PropertyRead);
+  });
+
   it('should locate element children', () => {
     const {errors, nodes, position} = parse(`<div><sp¦an></span></div>`);
     expect(errors).toBe(null);

packages/language-service/test/quick_info_spec.ts

@@ -14,7 +14,7 @@ import {createModuleAndProjectWithDeclarations, LanguageServiceTestEnv, Project}
 function quickInfoSkeleton(): {[fileName: string]: string} {
   return {
     'app.ts': `
-        import {Component, Directive, EventEmitter, Input, NgModule, Output, Pipe, PipeTransform, model} from '@angular/core';
+        import {Component, Directive, EventEmitter, Input, NgModule, Output, Pipe, PipeTransform, model, signal} from '@angular/core';
         import {CommonModule} from '@angular/common';
 
         export interface Address {
@@ -60,6 +60,18 @@ function quickInfoSkeleton(): {[fileName: string]: string} {
           setTitle(newTitle: string) {}
           trackByFn!: any;
           name!: any;
+          someObject = {
+            someProp: 'prop',
+            someSignal: signal<number>(0),
+            someMethod: (): number => 1,
+            nested: {
+              helloWorld: () => {
+                return {
+                  nestedMethod: () => 1
+                }
+              }
+            }
+          };
         }
 
         @Directive({
@@ -331,14 +343,6 @@ describe('quick info', () => {
         });
       });
 
-      it('should work for $event from native element', () => {
-        expectQuickInfo({
-          templateOverride: `<div (click)="myClick($e¦vent)"></div>`,
-          expectedSpanText: '$event',
-          expectedDisplayString: '(parameter) $event: MouseEvent',
-        });
-      });
-
       it('should work for click output from native element', () => {
         expectQuickInfo({
           templateOverride: `<div (cl¦ick)="myClick($event)"></div>`,
@@ -422,6 +426,23 @@ describe('quick info', () => {
         });
       });
 
+      it('should work for accessed function calls', () => {
+        expectQuickInfo({
+          templateOverride: `<div (click)="someObject.some¦Method()"></div>`,
+          expectedSpanText: 'someMethod',
+          expectedDisplayString: '(property) someMethod: () => number',
+        });
+      });
+
+      it('should work for accessed very nested function calls', () => {
+        expectQuickInfo({
+          templateOverride: `<div (click)="someObject.nested.helloWor¦ld().nestedMethod()"></div>`,
+          expectedSpanText: 'helloWorld',
+          expectedDisplayString:
+            '(property) helloWorld: () => {\n    nestedMethod: () => number;\n}',
+        });
+      });
+
       it('should find members in an attribute interpolation', () => {
         expectQuickInfo({
           templateOverride: `<div string-model model="{{tit¦le}}"></div>`,
@@ -481,6 +502,44 @@ describe('quick info', () => {
         expect(toText(info.documentation)).toEqual('Documentation for myFunc.');
       });
 
+      it('should work for safe signal calls', () => {
+        const files = {
+          'app.ts': `import {Component, Signal} from '@angular/core';
+            @Component({template: '<div [id]="something?.value()"></div>'})
+            export class AppCmp {
+              something!: {
+                /** Documentation for value. */
+                value: Signal<number>;
+              };
+            }`,
+        };
+        const project = createModuleAndProjectWithDeclarations(env, 'test_project', files);
+        const appFile = project.openFile('app.ts');
+        appFile.moveCursorToText('something?.va¦lue()');
+        const info = appFile.getQuickInfoAtPosition()!;
+        expect(toText(info.displayParts)).toEqual('(property) value: Signal<number>');
+        expect(toText(info.documentation)).toEqual('Documentation for value.');
+      });
+
+      it('should work for signal calls', () => {
+        const files = {
+          'app.ts': `import {Component, signal} from '@angular/core';
+            @Component({template: '<div [id]="something.value()"></div>'})
+            export class AppCmp {
+              something = {
+                /** Documentation for value. */
+                value: signal(0)
+              };
+            }`,
+        };
+        const project = createModuleAndProjectWithDeclarations(env, 'test_project', files);
+        const appFile = project.openFile('app.ts');
+        appFile.moveCursorToText('something.va¦lue()');
+        const info = appFile.getQuickInfoAtPosition()!;
+        expect(toText(info.displayParts)).toEqual('(property) value: WritableSignal\n() => number');
+        expect(toText(info.documentation)).toEqual('Documentation for value.');
+      });
+
       it('should work for accessed properties in writes', () => {
         expectQuickInfo({
           templateOverride: `<div (click)="hero.i¦d = 2"></div>`,
@@ -719,6 +778,14 @@ describe('quick info', () => {
           expectedDisplayString: '(variable) aliasName: [{ readonly name: "name"; }]',
         });
       });
+
+      it('if block alias variable', () => {
+        expectQuickInfo({
+          templateOverride: `@if (someObject.some¦Signal(); as aliasName) {}`,
+          expectedSpanText: 'someSignal',
+          expectedDisplayString: '(property) someSignal: WritableSignal\n() => number',
+        });
+      });
     });
 
     describe('let declarations', () => {

packages/language-service/test/references_and_rename_spec.ts

@@ -170,6 +170,42 @@ describe('find references and rename locations', () => {
     });
   });
 
+  describe('when cursor in on argument to a nested function call in an external template', () => {
+    let appFile: OpenBuffer;
+
+    beforeEach(() => {
+      const files = {
+        'app.ts': `
+          import {Component} from '@angular/core';
+          @Component({template: '<div (click)="nested.setTitle(title)"></div>'})
+          export class AppCmp {
+            title = '';
+            nested = {
+              setTitle(s: string) {}
+            }
+          }`,
+      };
+      env = LanguageServiceTestEnv.setup();
+      const project = createModuleAndProjectWithDeclarations(env, 'test', files);
+      appFile = project.openFile('app.ts');
+      appFile.moveCursorToText('(ti¦tle)');
+    });
+
+    it('gets member reference in ts file', () => {
+      const refs = getReferencesAtPosition(appFile)!;
+      expect(refs.length).toBe(2);
+
+      assertTextSpans(refs, ['title']);
+    });
+
+    it('finds rename location in ts file', () => {
+      const refs = getRenameLocationsAtPosition(appFile)!;
+      expect(refs.length).toBe(2);
+
+      assertTextSpans(refs, ['title']);
+    });
+  });
+
   describe('when cursor is on $event in method call arguments', () => {
     let file: OpenBuffer;
 

packages/language-service/test/signature_help_spec.ts

@@ -131,6 +131,30 @@ describe('signature help', () => {
     expect(items.argumentIndex).toEqual(1);
     expect(items.items.length).toEqual(1);
   });
+
+  it('should handle a single argument if the function is nested', () => {
+    const main = setup(`
+      import {Component} from '@angular/core';
+      @Component({
+        template: '{{ someObj.foo("test") }}',
+      })
+      export class MainCmp {
+        someObj = {
+          foo(alpha: string, beta: number): string {
+            return 'blah';
+          }
+        }
+      }
+    `);
+    main.moveCursorToText('foo("test"¦)');
+
+    const items = main.getSignatureHelpItems()!;
+    expect(items).toBeDefined();
+    expect(getText(main.contents, items.applicableSpan)).toEqual('"test"');
+    expect(items.argumentCount).toEqual(1);
+    expect(items.argumentIndex).toEqual(0);
+    expect(items.items.length).toEqual(1);
+  });
 });
 
 function setup(mainTs: string): OpenBuffer {