Skip to content

Commit e2b9980

Browse files
crisbetothePunderWoman
crisbeto
authored and
thePunderWoman
committed
refactor(language-service): integrate let declarations (#56270)
Integrates let declarations in the various places within the language service (quick info, completions etc). PR Close #56270
1 parent 014fac8 commit e2b9980

15 files changed

+346
-31
lines changed

‎packages/language-service/api.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ export interface PluginConfig {
3636
* Version of `@angular/core` that was detected in the user's workspace.
3737
*/
3838
angularCoreVersion?: string;
39+
40+
// TODO(crisbeto): type this as `false` when the syntax is enabled by default.
41+
/**
42+
* If false, disables parsing of `@let` declarations in the compiler.
43+
*/
44+
enableLetSyntax?: boolean;
3945
}
4046

4147
export type GetTcbResponse = {

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

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ type ElementAnimationCompletionBuilder = CompletionBuilder<
8181

8282
type BlockCompletionBuilder = CompletionBuilder<UnknownBlock>;
8383

84+
type LetCompletionBuilder = CompletionBuilder<TmplAstLetDeclaration>;
85+
8486
export enum CompletionNodeContext {
8587
None,
8688
ElementTag,
@@ -154,11 +156,17 @@ export class CompletionBuilder<N extends TmplAstNode | AST> {
154156
return this.getLiteralCompletions(options);
155157
} else if (this.isBlockCompletion()) {
156158
return this.getBlockCompletions(options);
159+
} else if (this.isLetCompletion()) {
160+
return this.getGlobalPropertyExpressionCompletion(options);
157161
} else {
158162
return undefined;
159163
}
160164
}
161165

166+
private isLetCompletion(): this is LetCompletionBuilder {
167+
return this.node instanceof TmplAstLetDeclaration;
168+
}
169+
162170
private isBlockCompletion(): this is BlockCompletionBuilder {
163171
return this.node instanceof UnknownBlock;
164172
}
@@ -484,7 +492,7 @@ export class CompletionBuilder<N extends TmplAstNode | AST> {
484492
* Get completions for a property expression in a global context (e.g. `{{y|}}`).
485493
*/
486494
private getGlobalPropertyExpressionCompletion(
487-
this: PropertyExpressionCompletionBuilder,
495+
this: PropertyExpressionCompletionBuilder | LetCompletionBuilder,
488496
options: ts.GetCompletionsAtPositionOptions | undefined,
489497
): ts.WithMetadata<ts.CompletionInfo> | undefined {
490498
const completions = this.templateTypeChecker.getGlobalCompletions(
@@ -544,16 +552,22 @@ export class CompletionBuilder<N extends TmplAstNode | AST> {
544552
}
545553

546554
for (const [name, entity] of templateContext) {
555+
let displayInfo: DisplayInfoKind;
556+
557+
if (entity.kind === CompletionKind.Reference) {
558+
displayInfo = DisplayInfoKind.REFERENCE;
559+
} else if (entity.kind === CompletionKind.LetDeclaration) {
560+
displayInfo = DisplayInfoKind.LET;
561+
} else {
562+
displayInfo = DisplayInfoKind.VARIABLE;
563+
}
564+
547565
entries.push({
548566
name,
549567
sortText: name,
550568
replacementSpan,
551569
kindModifiers: ts.ScriptElementKindModifier.none,
552-
kind: unsafeCastDisplayInfoKindToScriptElementKind(
553-
entity.kind === CompletionKind.Reference
554-
? DisplayInfoKind.REFERENCE
555-
: DisplayInfoKind.VARIABLE,
556-
),
570+
kind: unsafeCastDisplayInfoKindToScriptElementKind(displayInfo),
557571
});
558572
}
559573

@@ -1185,9 +1199,15 @@ function makeReplacementSpanFromAst(
11851199
| BindingPipe
11861200
| EmptyExpr
11871201
| LiteralPrimitive
1188-
| BoundEvent,
1202+
| BoundEvent
1203+
| TmplAstLetDeclaration,
11891204
): ts.TextSpan | undefined {
1190-
if (node instanceof EmptyExpr || node instanceof LiteralPrimitive || node instanceof BoundEvent) {
1205+
if (
1206+
node instanceof EmptyExpr ||
1207+
node instanceof LiteralPrimitive ||
1208+
node instanceof BoundEvent ||
1209+
node instanceof TmplAstLetDeclaration
1210+
) {
11911211
// empty nodes do not replace any existing text
11921212
return undefined;
11931213
}

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,6 @@ export class DefinitionBuilder {
111111
component,
112112
}: DefinitionMeta & TemplateInfo): readonly ts.DefinitionInfo[] | undefined {
113113
switch (symbol.kind) {
114-
case SymbolKind.LetDeclaration:
115-
// TODO(crisbeto): will integrate let declarations in a future commit.
116-
return undefined;
117114
case SymbolKind.Directive:
118115
case SymbolKind.Element:
119116
case SymbolKind.Template:
@@ -141,14 +138,15 @@ export class DefinitionBuilder {
141138
const directiveDefs = this.getDirectiveTypeDefsForBindingNode(node, parent, component);
142139
return [...bindingDefs, ...directiveDefs];
143140
}
141+
case SymbolKind.LetDeclaration:
144142
case SymbolKind.Variable:
145143
case SymbolKind.Reference: {
146144
const definitions: ts.DefinitionInfo[] = [];
147145
if (symbol.declaration !== node) {
148146
const tcbLocation =
149-
symbol.kind === SymbolKind.Variable
150-
? symbol.localVarLocation
151-
: symbol.referenceVarLocation;
147+
symbol.kind === SymbolKind.Reference
148+
? symbol.referenceVarLocation
149+
: symbol.localVarLocation;
152150
const mapping = getTemplateLocationFromTcbLocation(
153151
this.compiler.getTemplateTypeChecker(),
154152
tcbLocation.tcbPath,
@@ -167,7 +165,7 @@ export class DefinitionBuilder {
167165
});
168166
}
169167
}
170-
if (symbol.kind === SymbolKind.Variable) {
168+
if (symbol.kind === SymbolKind.Variable || symbol.kind === SymbolKind.LetDeclaration) {
171169
definitions.push(
172170
...this.getDefinitionsForSymbols({tcbLocation: symbol.initializerLocation}),
173171
);
@@ -271,7 +269,8 @@ export class DefinitionBuilder {
271269
case SymbolKind.Expression:
272270
definitions.push(...this.getTypeDefinitionsForSymbols(symbol));
273271
break;
274-
case SymbolKind.Variable: {
272+
case SymbolKind.Variable:
273+
case SymbolKind.LetDeclaration: {
275274
definitions.push(
276275
...this.getTypeDefinitionsForSymbols({tcbLocation: symbol.initializerLocation}),
277276
);

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import {isNamedClassDeclaration} from '@angular/compiler-cli/src/ngtsc/reflection';
1010
import {
11+
LetDeclarationSymbol,
1112
PotentialDirective,
1213
ReferenceSymbol,
1314
Symbol,
@@ -42,6 +43,7 @@ export enum DisplayInfoKind {
4243
METHOD = 'method',
4344
TEMPLATE = 'template',
4445
KEYWORD = 'keyword',
46+
LET = 'let',
4547
}
4648

4749
export interface DisplayInfo {
@@ -54,13 +56,15 @@ export interface DisplayInfo {
5456
export function getSymbolDisplayInfo(
5557
tsLS: ts.LanguageService,
5658
typeChecker: ts.TypeChecker,
57-
symbol: ReferenceSymbol | VariableSymbol,
59+
symbol: ReferenceSymbol | VariableSymbol | LetDeclarationSymbol,
5860
): DisplayInfo {
5961
let kind: DisplayInfoKind;
6062
if (symbol.kind === SymbolKind.Reference) {
6163
kind = DisplayInfoKind.REFERENCE;
6264
} else if (symbol.kind === SymbolKind.Variable) {
6365
kind = DisplayInfoKind.VARIABLE;
66+
} else if (symbol.kind === SymbolKind.LetDeclaration) {
67+
kind = DisplayInfoKind.LET;
6468
} else {
6569
throw new Error(
6670
`AssertionError: unexpected symbol kind ${SymbolKind[(symbol as Symbol).kind]}`,

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,10 @@ function parseNgCompilerOptions(
642642
if (config['enableBlockSyntax'] === false) {
643643
options['_enableBlockSyntax'] = false;
644644
}
645+
// TODO(crisbeto): only allow `false` when the syntax is enabled by default.
646+
if (config['enableLetSyntax'] != null) {
647+
options['_enableLetSyntax'] = config['enableLetSyntax'];
648+
}
645649

646650
options['_angularCoreVersion'] = config['angularCoreVersion'];
647651

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
DomBindingSymbol,
2020
ElementSymbol,
2121
InputBindingSymbol,
22+
LetDeclarationSymbol,
2223
OutputBindingSymbol,
2324
PipeSymbol,
2425
ReferenceSymbol,
@@ -84,9 +85,6 @@ export class QuickInfoBuilder {
8485

8586
private getQuickInfoForSymbol(symbol: Symbol): ts.QuickInfo | undefined {
8687
switch (symbol.kind) {
87-
case SymbolKind.LetDeclaration:
88-
// TODO(crisbeto): will integrate let declarations in a future commit.
89-
return undefined;
9088
case SymbolKind.Input:
9189
case SymbolKind.Output:
9290
return this.getQuickInfoForBindingSymbol(symbol);
@@ -96,6 +94,8 @@ export class QuickInfoBuilder {
9694
return this.getQuickInfoForElementSymbol(symbol);
9795
case SymbolKind.Variable:
9896
return this.getQuickInfoForVariableSymbol(symbol);
97+
case SymbolKind.LetDeclaration:
98+
return this.getQuickInfoForLetDeclarationSymbol(symbol);
9999
case SymbolKind.Reference:
100100
return this.getQuickInfoForReferenceSymbol(symbol);
101101
case SymbolKind.DomBinding:
@@ -151,6 +151,18 @@ export class QuickInfoBuilder {
151151
);
152152
}
153153

154+
private getQuickInfoForLetDeclarationSymbol(symbol: LetDeclarationSymbol): ts.QuickInfo {
155+
const documentation = this.getDocumentationFromTypeDefAtLocation(symbol.initializerLocation);
156+
return createQuickInfo(
157+
symbol.declaration.name,
158+
DisplayInfoKind.LET,
159+
getTextSpanOfNode(this.node),
160+
undefined /* containerName */,
161+
this.typeChecker.typeToString(symbol.tsType),
162+
documentation,
163+
);
164+
}
165+
154166
private getQuickInfoForReferenceSymbol(symbol: ReferenceSymbol): ts.QuickInfo {
155167
const documentation = this.getDocumentationFromTypeDefAtLocation(symbol.targetLocation);
156168
return createQuickInfo(

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
SafePropertyRead,
1515
TmplAstBoundAttribute,
1616
TmplAstBoundEvent,
17+
TmplAstLetDeclaration,
1718
TmplAstNode,
1819
TmplAstReference,
1920
TmplAstTextAttribute,
@@ -180,6 +181,20 @@ export function getTargetDetailsAtTemplatePosition(
180181
}
181182
break;
182183
}
184+
case SymbolKind.LetDeclaration:
185+
// If the templateNode isn't on a let declaration, it has to be on a usage of it
186+
// somewhere in the template. Otherwise only pick up when it's within the name.
187+
if (
188+
!(templateTarget instanceof TmplAstLetDeclaration) ||
189+
isWithin(position, templateTarget.nameSpan)
190+
) {
191+
details.push({
192+
typescriptLocations: [toFilePosition(symbol.localVarLocation)],
193+
templateTarget,
194+
symbol,
195+
});
196+
}
197+
break;
183198
case SymbolKind.Input:
184199
case SymbolKind.Output: {
185200
details.push({
@@ -317,10 +332,9 @@ export function getRenameTextAndSpanAtPosition(
317332
node instanceof TmplAstTextAttribute ||
318333
node instanceof TmplAstBoundEvent
319334
) {
320-
if (node.keySpan === undefined) {
321-
return null;
322-
}
323-
return {text: node.name, span: toTextSpan(node.keySpan)};
335+
return node.keySpan === undefined ? null : {text: node.name, span: toTextSpan(node.keySpan)};
336+
} else if (node instanceof TmplAstLetDeclaration && isWithin(position, node.nameSpan)) {
337+
return {text: node.nameSpan.toString(), span: toTextSpan(node.nameSpan)};
324338
} else if (node instanceof TmplAstVariable || node instanceof TmplAstReference) {
325339
if (isWithin(position, node.keySpan)) {
326340
return {text: node.keySpan.toString(), span: toTextSpan(node.keySpan)};

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

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1952,6 +1952,65 @@ describe('completions', () => {
19521952
expectReplacementText(completions, templateFile.contents, 'mat-');
19531953
});
19541954
});
1955+
1956+
describe('let declarations', () => {
1957+
it('should complete a let declaration', () => {
1958+
const {templateFile} = setup(
1959+
`
1960+
@let message = 'hello';
1961+
{{mess}}
1962+
`,
1963+
'',
1964+
);
1965+
templateFile.moveCursorToText('{{mess¦}}');
1966+
const completions = templateFile.getCompletionsAtPosition();
1967+
expectContain(completions, DisplayInfoKind.LET, ['message']);
1968+
});
1969+
1970+
it('should complete an empty let declaration with a terminating character', () => {
1971+
const {templateFile} = setup('@let foo = ;', `title!: string; hero!52: number;`);
1972+
templateFile.moveCursorToText('@let foo = ¦;');
1973+
const completions = templateFile.getCompletionsAtPosition();
1974+
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['title', 'hero']);
1975+
});
1976+
1977+
it('should complete a single let declaration without a terminating character', () => {
1978+
const {templateFile} = setup('@let foo = ', `title!: string; hero!52: number;`);
1979+
templateFile.moveCursorToText('@let foo = ¦');
1980+
const completions = templateFile.getCompletionsAtPosition();
1981+
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['title', 'hero']);
1982+
});
1983+
1984+
it('should complete a let declaration property in the global scope', () => {
1985+
const {templateFile} = setup(
1986+
`
1987+
@let hobbit = {name: 'Frodo', age: 53};
1988+
{{hobbit.}}
1989+
`,
1990+
'',
1991+
);
1992+
templateFile.moveCursorToText('{{hobbit.¦}}');
1993+
const completions = templateFile.getCompletionsAtPosition();
1994+
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['age', 'name']);
1995+
});
1996+
1997+
it('should complete a shadowed let declaration property', () => {
1998+
const {templateFile} = setup(
1999+
`
2000+
@let hobbit = {name: 'Frodo', age: 53};
2001+
2002+
@if (true) {
2003+
@let hobbit = {hasRing: true, size: 'small'};
2004+
{{hobbit.}}
2005+
}
2006+
`,
2007+
'',
2008+
);
2009+
templateFile.moveCursorToText('{{hobbit.¦}}');
2010+
const completions = templateFile.getCompletionsAtPosition();
2011+
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['hasRing', 'size']);
2012+
});
2013+
});
19552014
});
19562015

19572016
function expectContainInsertText(
@@ -2061,8 +2120,10 @@ function setup(
20612120
const otherDirectiveClassDecls = Object.values(otherDeclarations).join('\n\n');
20622121

20632122
const env = LanguageServiceTestEnv.setup();
2064-
const project = env.addProject('test', {
2065-
'test.ts': `
2123+
const project = env.addProject(
2124+
'test',
2125+
{
2126+
'test.ts': `
20662127
import {Component,
20672128
input,
20682129
output,
@@ -2093,8 +2154,14 @@ function setup(
20932154
})
20942155
export class AppModule {}
20952156
`,
2096-
'test.html': template,
2097-
});
2157+
'test.html': template,
2158+
},
2159+
// Note: this object is cast to `any`, because for some reason the typing
2160+
// changes to the `TestableOption` type aren't being picked up in tests.
2161+
{
2162+
_enableLetSyntax: true,
2163+
} as any,
2164+
);
20982165
return {templateFile: project.openFile('test.html')};
20992166
}
21002167

0 commit comments

Comments
 (0)