Skip to content

Commit 58b8a23

Browse files
devversionthePunderWoman
devversion
authored and
thePunderWoman
committed
fix(compiler-cli): support jumping to definitions of signal-based inputs (#54053)
This fixes the definitions for signal-based inputs in the language service and type checking symbol builder. Signal inputs emit a slightly different output. The output works well for comppletion and was designed to affect language service minimally. Turns out there is a small adjustment needed for the definition symbols. PR Close #54053
1 parent b78042f commit 58b8a23

File tree

4 files changed

+184
-24
lines changed

4 files changed

+184
-24
lines changed

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

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {AST, ASTWithSource, BindingPipe, ParseSourceSpan, PropertyRead, PropertyWrite, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
9+
import {AST, ASTWithSource, BindingPipe, ParseSourceSpan, PropertyRead, PropertyWrite, R3Identifiers, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
1010
import ts from 'typescript';
1111

1212
import {AbsoluteFsPath} from '../../file_system';
@@ -311,7 +311,6 @@ export class SymbolBuilder {
311311
continue;
312312
}
313313

314-
315314
const target = this.getDirectiveSymbolForAccessExpression(outputFieldAccess, consumer);
316315
if (target === null) {
317316
continue;
@@ -359,12 +358,33 @@ export class SymbolBuilder {
359358
continue;
360359
}
361360

362-
const symbolInfo = this.getSymbolOfTsNode(node.left);
361+
const signalInputAssignment = unwrapSignalInputWriteTAccessor(node.left);
362+
let symbolInfo: TsNodeSymbolInfo|null = null;
363+
364+
// Signal inputs need special treatment because they are generated with an extra keyed
365+
// access. E.g. `_t1.prop[WriteT_ACCESSOR_SYMBOL]`. Observations:
366+
// - The keyed access for the write type needs to be resolved for the "input type".
367+
// - The definition symbol of the input should be the input class member, and not the
368+
// internal write accessor. Symbol should resolve `_t1.prop`.
369+
if (signalInputAssignment !== null) {
370+
const fieldSymbol = this.getSymbolOfTsNode(signalInputAssignment.fieldExpr);
371+
const typeSymbol = this.getSymbolOfTsNode(signalInputAssignment.typeExpr);
372+
373+
symbolInfo = fieldSymbol === null || typeSymbol === null ? null : {
374+
tcbLocation: fieldSymbol.tcbLocation,
375+
tsSymbol: fieldSymbol.tsSymbol,
376+
tsType: typeSymbol.tsType,
377+
};
378+
} else {
379+
symbolInfo = this.getSymbolOfTsNode(node.left);
380+
}
381+
363382
if (symbolInfo === null || symbolInfo.tsSymbol === null) {
364383
continue;
365384
}
366385

367-
const target = this.getDirectiveSymbolForAccessExpression(node.left, consumer);
386+
const target = this.getDirectiveSymbolForAccessExpression(
387+
signalInputAssignment?.fieldExpr ?? node.left, consumer);
368388
if (target === null) {
369389
continue;
370390
}
@@ -383,11 +403,10 @@ export class SymbolBuilder {
383403
}
384404

385405
private getDirectiveSymbolForAccessExpression(
386-
node: ts.ElementAccessExpression|ts.PropertyAccessExpression,
406+
fieldAccessExpr: ts.ElementAccessExpression|ts.PropertyAccessExpression,
387407
{isComponent, selector, isStructural}: TypeCheckableDirectiveMeta): DirectiveSymbol|null {
388-
// In either case, `_t1["index"]` or `_t1.index`, `node.expression` is _t1.
389-
// The retrieved symbol for _t1 will be the variable declaration.
390-
const tsSymbol = this.getTypeChecker().getSymbolAtLocation(node.expression);
408+
// In all cases, `_t1["index"]` or `_t1.index`, `node.expression` is _t1.
409+
const tsSymbol = this.getTypeChecker().getSymbolAtLocation(fieldAccessExpr.expression);
391410
if (tsSymbol?.declarations === undefined || tsSymbol.declarations.length === 0 ||
392411
selector === null) {
393412
return null;
@@ -625,8 +644,6 @@ export class SymbolBuilder {
625644
let tsSymbol: ts.Symbol|undefined;
626645
if (ts.isPropertyAccessExpression(node)) {
627646
tsSymbol = this.getTypeChecker().getSymbolAtLocation(node.name);
628-
} else if (ts.isElementAccessExpression(node)) {
629-
tsSymbol = this.getTypeChecker().getSymbolAtLocation(node.argumentExpression);
630647
} else {
631648
tsSymbol = this.getTypeChecker().getSymbolAtLocation(node);
632649
}
@@ -670,3 +687,35 @@ function anyNodeFilter(n: ts.Node): n is ts.Node {
670687
function sourceSpanEqual(a: ParseSourceSpan, b: ParseSourceSpan) {
671688
return a.start.offset === b.start.offset && a.end.offset === b.end.offset;
672689
}
690+
691+
function unwrapSignalInputWriteTAccessor(expr: ts.LeftHandSideExpression): (null|{
692+
fieldExpr: ts.ElementAccessExpression | ts.PropertyAccessExpression,
693+
typeExpr: ts.ElementAccessExpression
694+
}) {
695+
// e.g. `_t2.inputA[i2.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]`
696+
// 1. Assert that we are dealing with an element access expression.
697+
// 2. Assert that we are dealing with a signal brand symbol access in the argument expression.
698+
if (!ts.isElementAccessExpression(expr) ||
699+
!ts.isPropertyAccessExpression(expr.argumentExpression)) {
700+
return null;
701+
}
702+
703+
// Assert that the property access in the element access is a simple identifier and
704+
// refers to `ɵINPUT_SIGNAL_BRAND_WRITE_TYPE`.
705+
if (!ts.isIdentifier(expr.argumentExpression.name) ||
706+
expr.argumentExpression.name.text !== R3Identifiers.InputSignalBrandWriteType.name) {
707+
return null;
708+
}
709+
710+
// Assert that the `_t2.inputA` is actually either a keyed element access, or
711+
// property access expression. This is checked for type safety and to catch unexpected cases.
712+
if (!ts.isPropertyAccessExpression(expr.expression) &&
713+
!ts.isElementAccessExpression(expr.expression)) {
714+
throw new Error('Unexpected expression for signal input write type.');
715+
}
716+
717+
return {
718+
fieldExpr: expr.expression,
719+
typeExpr: expr,
720+
};
721+
}

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,6 +1135,69 @@ runInEachFileSystem(() => {
11351135
.toEqual('inputB');
11361136
});
11371137

1138+
it('can retrieve a symbol for a signal-input binding', () => {
1139+
const fileName = absoluteFrom('/main.ts');
1140+
const dirFile = absoluteFrom('/dir.ts');
1141+
const templateString = `<div dir [inputA]="'my input A'" [aliased]="'my inputB'"></div>`;
1142+
const {program, templateTypeChecker} = setup([
1143+
{
1144+
fileName,
1145+
templates: {'Cmp': templateString},
1146+
declarations: [{
1147+
name: 'TestDir',
1148+
selector: '[dir]',
1149+
file: dirFile,
1150+
type: 'directive',
1151+
inputs: {
1152+
inputA: {
1153+
bindingPropertyName: 'inputA',
1154+
isSignal: true,
1155+
classPropertyName: 'inputA',
1156+
required: false,
1157+
transform: null,
1158+
},
1159+
inputB: {
1160+
bindingPropertyName: 'aliased',
1161+
isSignal: true,
1162+
classPropertyName: 'inputB',
1163+
required: true,
1164+
transform: null,
1165+
}
1166+
},
1167+
}]
1168+
},
1169+
{
1170+
fileName: dirFile,
1171+
source: `
1172+
import {InputSignal} from '@angular/core';
1173+
1174+
export class TestDir {
1175+
inputA: InputSignal<string> = null!;
1176+
inputB: InputSignal<string> = null!;
1177+
}`,
1178+
templates: {},
1179+
}
1180+
]);
1181+
const sf = getSourceFileOrError(program, fileName);
1182+
const cmp = getClass(sf, 'Cmp');
1183+
1184+
const nodes = templateTypeChecker.getTemplate(cmp)!;
1185+
1186+
const inputAbinding = (nodes[0] as TmplAstElement).inputs[0];
1187+
const aSymbol = templateTypeChecker.getSymbolOfNode(inputAbinding, cmp)!;
1188+
assertInputBindingSymbol(aSymbol);
1189+
expect((aSymbol.bindings[0].tsSymbol!.declarations![0] as ts.PropertyDeclaration)
1190+
.name.getText())
1191+
.toEqual('inputA');
1192+
1193+
const inputBbinding = (nodes[0] as TmplAstElement).inputs[1];
1194+
const bSymbol = templateTypeChecker.getSymbolOfNode(inputBbinding, cmp)!;
1195+
assertInputBindingSymbol(bSymbol);
1196+
expect((bSymbol.bindings[0].tsSymbol!.declarations![0] as ts.PropertyDeclaration)
1197+
.name.getText())
1198+
.toEqual('inputB');
1199+
});
1200+
11381201
it('does not retrieve a symbol for an input when undeclared', () => {
11391202
const fileName = absoluteFrom('/main.ts');
11401203
const dirFile = absoluteFrom('/dir.ts');

‎packages/language-service/src/definitions.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ export class DefinitionBuilder {
4848
}
4949
return getDefinitionForExpressionAtPosition(fileName, position, this.compiler);
5050
}
51+
5152
const definitionMetas = this.getDefinitionMetaAtPosition(templateInfo, position);
53+
5254
if (definitionMetas === undefined) {
5355
return undefined;
5456
}
@@ -66,6 +68,7 @@ export class DefinitionBuilder {
6668
...(this.getDefinitionsForSymbol({...definitionMeta, ...templateInfo}) ?? []));
6769
}
6870

71+
6972
if (definitions.length === 0) {
7073
return undefined;
7174
}

‎packages/language-service/test/definitions_spec.ts‎

Lines changed: 59 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ describe('definitions', () => {
1717
'app.html': '',
1818
'app.ts': `
1919
import {Component} from '@angular/core';
20-
20+
2121
@Component({templateUrl: '/app.html'})
2222
export class AppCmp {}
2323
`,
@@ -41,7 +41,7 @@ describe('definitions', () => {
4141
'app.ts': `
4242
import {Component, NgModule} from '@angular/core';
4343
import {CommonModule} from '@angular/common';
44-
44+
4545
@Component({templateUrl: 'app.html'})
4646
export class AppCmp {}
4747
`,
@@ -69,21 +69,21 @@ describe('definitions', () => {
6969
'app.ts': `
7070
import {Component, NgModule} from '@angular/core';
7171
import {CommonModule} from '@angular/common';
72-
72+
7373
@Component({templateUrl: 'app.html'})
7474
export class AppCmp {}
7575
`,
7676
'app.html': '<div dir inputA="abc"></div>',
7777
'dir.ts': `
7878
import {Directive, Input} from '@angular/core';
79-
79+
8080
@Directive({selector: '[dir]'})
8181
export class MyDir {
8282
@Input() inputA!: any;
8383
}`,
8484
'dir2.ts': `
8585
import {Directive, Input} from '@angular/core';
86-
86+
8787
@Directive({selector: '[dir]'})
8888
export class MyDir2 {
8989
@Input() inputA!: any;
@@ -108,28 +108,73 @@ describe('definitions', () => {
108108
assertFileNames([def, def2], ['dir2.ts', 'dir.ts']);
109109
});
110110

111+
it('gets definitions for all signal-inputs when attribute matches more than one', () => {
112+
initMockFileSystem('Native');
113+
const files = {
114+
'app.ts': `
115+
import {Component, NgModule} from '@angular/core';
116+
import {CommonModule} from '@angular/common';
117+
118+
@Component({templateUrl: 'app.html'})
119+
export class AppCmp {}
120+
`,
121+
'app.html': '<div dir inputA="abc"></div>',
122+
'dir.ts': `
123+
import {Directive, input} from '@angular/core';
124+
125+
@Directive({selector: '[dir]'})
126+
export class MyDir {
127+
inputA = input();
128+
}`,
129+
'dir2.ts': `
130+
import {Directive, input} from '@angular/core';
131+
132+
@Directive({selector: '[dir]'})
133+
export class MyDir2 {
134+
inputA = input();
135+
}`
136+
137+
};
138+
const env = LanguageServiceTestEnv.setup();
139+
const project = createModuleAndProjectWithDeclarations(env, 'test', files);
140+
const template = project.openFile('app.html');
141+
template.moveCursorToText('inpu¦tA="abc"');
142+
143+
const {textSpan, definitions} = getDefinitionsAndAssertBoundSpan(env, template);
144+
expect(template.contents.slice(textSpan.start, textSpan.start + textSpan.length))
145+
.toEqual('inputA');
146+
147+
expect(definitions!.length).toEqual(2);
148+
const [def, def2] = definitions!;
149+
expect(def.textSpan).toContain('inputA');
150+
expect(def2.textSpan).toContain('inputA');
151+
// TODO(atscott): investigate why the text span includes more than just 'inputA'
152+
// assertTextSpans([def, def2], ['inputA']);
153+
assertFileNames([def, def2], ['dir2.ts', 'dir.ts']);
154+
});
155+
111156
it('gets definitions for all outputs when attribute matches more than one', () => {
112157
initMockFileSystem('Native');
113158
const files = {
114159
'app.html': '<div dir (someEvent)="doSomething()"></div>',
115160
'dir.ts': `
116161
import {Directive, Output, EventEmitter} from '@angular/core';
117-
162+
118163
@Directive({selector: '[dir]'})
119164
export class MyDir {
120165
@Output() someEvent = new EventEmitter<void>();
121166
}`,
122167
'dir2.ts': `
123168
import {Directive, Output, EventEmitter} from '@angular/core';
124-
169+
125170
@Directive({selector: '[dir]'})
126171
export class MyDir2 {
127172
@Output() someEvent = new EventEmitter<void>();
128173
}`,
129174
'app.ts': `
130175
import {Component, NgModule} from '@angular/core';
131176
import {CommonModule} from '@angular/common';
132-
177+
133178
@Component({templateUrl: 'app.html'})
134179
export class AppCmp {
135180
doSomething() {}
@@ -159,7 +204,7 @@ describe('definitions', () => {
159204
const files = {
160205
'app.ts': `
161206
import {Component} from '@angular/core';
162-
207+
163208
@Component({
164209
template: '',
165210
styleUrls: ['./style.css'],
@@ -190,7 +235,7 @@ describe('definitions', () => {
190235
`,
191236
'app.ts': `
192237
import {Component} from '@angular/core';
193-
238+
194239
@Component({templateUrl: '/app.html'})
195240
export class AppCmp {
196241
myVal = {name: 'Andrew'};
@@ -230,12 +275,12 @@ describe('definitions', () => {
230275
export class DollarCmp {
231276
@Input() obs$!: string;
232277
}
233-
278+
234279
@Component({template: '<dollar-cmp [obs$]="greeting"></dollar-cmp>'})
235280
export class AppCmp {
236281
greeting = 'hello';
237282
}
238-
283+
239284
@NgModule({declarations: [AppCmp, DollarCmp]})
240285
export class AppModule {}
241286
`,
@@ -277,12 +322,12 @@ describe('definitions', () => {
277322
export class DollarDir {
278323
@Input() dollar$!: string;
279324
}
280-
325+
281326
@Component({template: '<div [dollar$]="greeting"></div>'})
282327
export class AppCmp {
283328
greeting = 'hello';
284329
}
285-
330+
286331
@NgModule({declarations: [AppCmp, DollarDir]})
287332
export class AppModule {}
288333
`,

0 commit comments

Comments
 (0)