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
This commit is contained in:
Kristiyan Kostadinov
2024-06-05 09:07:40 +02:00
committed by Jessica Janiuk
parent 1653b40d14
commit dd869e4d68
15 changed files with 346 additions and 31 deletions
+6
View File
@@ -36,6 +36,12 @@ export interface PluginConfig {
* Version of `@angular/core` that was detected in the user's workspace.
*/
angularCoreVersion?: string;
// TODO(crisbeto): type this as `false` when the syntax is enabled by default.
/**
* If false, disables parsing of `@let` declarations in the compiler.
*/
enableLetSyntax?: boolean;
}
export type GetTcbResponse = {
+28 -8
View File
@@ -81,6 +81,8 @@ type ElementAnimationCompletionBuilder = CompletionBuilder<
type BlockCompletionBuilder = CompletionBuilder<UnknownBlock>;
type LetCompletionBuilder = CompletionBuilder<TmplAstLetDeclaration>;
export enum CompletionNodeContext {
None,
ElementTag,
@@ -154,11 +156,17 @@ export class CompletionBuilder<N extends TmplAstNode | AST> {
return this.getLiteralCompletions(options);
} else if (this.isBlockCompletion()) {
return this.getBlockCompletions(options);
} else if (this.isLetCompletion()) {
return this.getGlobalPropertyExpressionCompletion(options);
} else {
return undefined;
}
}
private isLetCompletion(): this is LetCompletionBuilder {
return this.node instanceof TmplAstLetDeclaration;
}
private isBlockCompletion(): this is BlockCompletionBuilder {
return this.node instanceof UnknownBlock;
}
@@ -484,7 +492,7 @@ export class CompletionBuilder<N extends TmplAstNode | AST> {
* Get completions for a property expression in a global context (e.g. `{{y|}}`).
*/
private getGlobalPropertyExpressionCompletion(
this: PropertyExpressionCompletionBuilder,
this: PropertyExpressionCompletionBuilder | LetCompletionBuilder,
options: ts.GetCompletionsAtPositionOptions | undefined,
): ts.WithMetadata<ts.CompletionInfo> | undefined {
const completions = this.templateTypeChecker.getGlobalCompletions(
@@ -544,16 +552,22 @@ export class CompletionBuilder<N extends TmplAstNode | AST> {
}
for (const [name, entity] of templateContext) {
let displayInfo: DisplayInfoKind;
if (entity.kind === CompletionKind.Reference) {
displayInfo = DisplayInfoKind.REFERENCE;
} else if (entity.kind === CompletionKind.LetDeclaration) {
displayInfo = DisplayInfoKind.LET;
} else {
displayInfo = DisplayInfoKind.VARIABLE;
}
entries.push({
name,
sortText: name,
replacementSpan,
kindModifiers: ts.ScriptElementKindModifier.none,
kind: unsafeCastDisplayInfoKindToScriptElementKind(
entity.kind === CompletionKind.Reference
? DisplayInfoKind.REFERENCE
: DisplayInfoKind.VARIABLE,
),
kind: unsafeCastDisplayInfoKindToScriptElementKind(displayInfo),
});
}
@@ -1185,9 +1199,15 @@ function makeReplacementSpanFromAst(
| BindingPipe
| EmptyExpr
| LiteralPrimitive
| BoundEvent,
| BoundEvent
| TmplAstLetDeclaration,
): ts.TextSpan | undefined {
if (node instanceof EmptyExpr || node instanceof LiteralPrimitive || node instanceof BoundEvent) {
if (
node instanceof EmptyExpr ||
node instanceof LiteralPrimitive ||
node instanceof BoundEvent ||
node instanceof TmplAstLetDeclaration
) {
// empty nodes do not replace any existing text
return undefined;
}
+7 -8
View File
@@ -111,9 +111,6 @@ export class DefinitionBuilder {
component,
}: DefinitionMeta & TemplateInfo): readonly ts.DefinitionInfo[] | undefined {
switch (symbol.kind) {
case SymbolKind.LetDeclaration:
// TODO(crisbeto): will integrate let declarations in a future commit.
return undefined;
case SymbolKind.Directive:
case SymbolKind.Element:
case SymbolKind.Template:
@@ -141,14 +138,15 @@ export class DefinitionBuilder {
const directiveDefs = this.getDirectiveTypeDefsForBindingNode(node, parent, component);
return [...bindingDefs, ...directiveDefs];
}
case SymbolKind.LetDeclaration:
case SymbolKind.Variable:
case SymbolKind.Reference: {
const definitions: ts.DefinitionInfo[] = [];
if (symbol.declaration !== node) {
const tcbLocation =
symbol.kind === SymbolKind.Variable
? symbol.localVarLocation
: symbol.referenceVarLocation;
symbol.kind === SymbolKind.Reference
? symbol.referenceVarLocation
: symbol.localVarLocation;
const mapping = getTemplateLocationFromTcbLocation(
this.compiler.getTemplateTypeChecker(),
tcbLocation.tcbPath,
@@ -167,7 +165,7 @@ export class DefinitionBuilder {
});
}
}
if (symbol.kind === SymbolKind.Variable) {
if (symbol.kind === SymbolKind.Variable || symbol.kind === SymbolKind.LetDeclaration) {
definitions.push(
...this.getDefinitionsForSymbols({tcbLocation: symbol.initializerLocation}),
);
@@ -271,7 +269,8 @@ export class DefinitionBuilder {
case SymbolKind.Expression:
definitions.push(...this.getTypeDefinitionsForSymbols(symbol));
break;
case SymbolKind.Variable: {
case SymbolKind.Variable:
case SymbolKind.LetDeclaration: {
definitions.push(
...this.getTypeDefinitionsForSymbols({tcbLocation: symbol.initializerLocation}),
);
@@ -8,6 +8,7 @@
import {isNamedClassDeclaration} from '@angular/compiler-cli/src/ngtsc/reflection';
import {
LetDeclarationSymbol,
PotentialDirective,
ReferenceSymbol,
Symbol,
@@ -42,6 +43,7 @@ export enum DisplayInfoKind {
METHOD = 'method',
TEMPLATE = 'template',
KEYWORD = 'keyword',
LET = 'let',
}
export interface DisplayInfo {
@@ -54,13 +56,15 @@ export interface DisplayInfo {
export function getSymbolDisplayInfo(
tsLS: ts.LanguageService,
typeChecker: ts.TypeChecker,
symbol: ReferenceSymbol | VariableSymbol,
symbol: ReferenceSymbol | VariableSymbol | LetDeclarationSymbol,
): DisplayInfo {
let kind: DisplayInfoKind;
if (symbol.kind === SymbolKind.Reference) {
kind = DisplayInfoKind.REFERENCE;
} else if (symbol.kind === SymbolKind.Variable) {
kind = DisplayInfoKind.VARIABLE;
} else if (symbol.kind === SymbolKind.LetDeclaration) {
kind = DisplayInfoKind.LET;
} else {
throw new Error(
`AssertionError: unexpected symbol kind ${SymbolKind[(symbol as Symbol).kind]}`,
@@ -642,6 +642,10 @@ function parseNgCompilerOptions(
if (config['enableBlockSyntax'] === false) {
options['_enableBlockSyntax'] = false;
}
// TODO(crisbeto): only allow `false` when the syntax is enabled by default.
if (config['enableLetSyntax'] != null) {
options['_enableLetSyntax'] = config['enableLetSyntax'];
}
options['_angularCoreVersion'] = config['angularCoreVersion'];
+15 -3
View File
@@ -19,6 +19,7 @@ import {
DomBindingSymbol,
ElementSymbol,
InputBindingSymbol,
LetDeclarationSymbol,
OutputBindingSymbol,
PipeSymbol,
ReferenceSymbol,
@@ -84,9 +85,6 @@ export class QuickInfoBuilder {
private getQuickInfoForSymbol(symbol: Symbol): ts.QuickInfo | undefined {
switch (symbol.kind) {
case SymbolKind.LetDeclaration:
// TODO(crisbeto): will integrate let declarations in a future commit.
return undefined;
case SymbolKind.Input:
case SymbolKind.Output:
return this.getQuickInfoForBindingSymbol(symbol);
@@ -96,6 +94,8 @@ export class QuickInfoBuilder {
return this.getQuickInfoForElementSymbol(symbol);
case SymbolKind.Variable:
return this.getQuickInfoForVariableSymbol(symbol);
case SymbolKind.LetDeclaration:
return this.getQuickInfoForLetDeclarationSymbol(symbol);
case SymbolKind.Reference:
return this.getQuickInfoForReferenceSymbol(symbol);
case SymbolKind.DomBinding:
@@ -151,6 +151,18 @@ export class QuickInfoBuilder {
);
}
private getQuickInfoForLetDeclarationSymbol(symbol: LetDeclarationSymbol): ts.QuickInfo {
const documentation = this.getDocumentationFromTypeDefAtLocation(symbol.initializerLocation);
return createQuickInfo(
symbol.declaration.name,
DisplayInfoKind.LET,
getTextSpanOfNode(this.node),
undefined /* containerName */,
this.typeChecker.typeToString(symbol.tsType),
documentation,
);
}
private getQuickInfoForReferenceSymbol(symbol: ReferenceSymbol): ts.QuickInfo {
const documentation = this.getDocumentationFromTypeDefAtLocation(symbol.targetLocation);
return createQuickInfo(
@@ -14,6 +14,7 @@ import {
SafePropertyRead,
TmplAstBoundAttribute,
TmplAstBoundEvent,
TmplAstLetDeclaration,
TmplAstNode,
TmplAstReference,
TmplAstTextAttribute,
@@ -180,6 +181,20 @@ export function getTargetDetailsAtTemplatePosition(
}
break;
}
case SymbolKind.LetDeclaration:
// If the templateNode isn't on a let declaration, it has to be on a usage of it
// somewhere in the template. Otherwise only pick up when it's within the name.
if (
!(templateTarget instanceof TmplAstLetDeclaration) ||
isWithin(position, templateTarget.nameSpan)
) {
details.push({
typescriptLocations: [toFilePosition(symbol.localVarLocation)],
templateTarget,
symbol,
});
}
break;
case SymbolKind.Input:
case SymbolKind.Output: {
details.push({
@@ -317,10 +332,9 @@ export function getRenameTextAndSpanAtPosition(
node instanceof TmplAstTextAttribute ||
node instanceof TmplAstBoundEvent
) {
if (node.keySpan === undefined) {
return null;
}
return {text: node.name, span: toTextSpan(node.keySpan)};
return node.keySpan === undefined ? null : {text: node.name, span: toTextSpan(node.keySpan)};
} else if (node instanceof TmplAstLetDeclaration && isWithin(position, node.nameSpan)) {
return {text: node.nameSpan.toString(), span: toTextSpan(node.nameSpan)};
} else if (node instanceof TmplAstVariable || node instanceof TmplAstReference) {
if (isWithin(position, node.keySpan)) {
return {text: node.keySpan.toString(), span: toTextSpan(node.keySpan)};
@@ -1952,6 +1952,65 @@ describe('completions', () => {
expectReplacementText(completions, templateFile.contents, 'mat-');
});
});
describe('let declarations', () => {
it('should complete a let declaration', () => {
const {templateFile} = setup(
`
@let message = 'hello';
{{mess}}
`,
'',
);
templateFile.moveCursorToText('{{mess¦}}');
const completions = templateFile.getCompletionsAtPosition();
expectContain(completions, DisplayInfoKind.LET, ['message']);
});
it('should complete an empty let declaration with a terminating character', () => {
const {templateFile} = setup('@let foo = ;', `title!: string; hero!52: number;`);
templateFile.moveCursorToText('@let foo = ¦;');
const completions = templateFile.getCompletionsAtPosition();
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['title', 'hero']);
});
it('should complete a single let declaration without a terminating character', () => {
const {templateFile} = setup('@let foo = ', `title!: string; hero!52: number;`);
templateFile.moveCursorToText('@let foo = ¦');
const completions = templateFile.getCompletionsAtPosition();
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['title', 'hero']);
});
it('should complete a let declaration property in the global scope', () => {
const {templateFile} = setup(
`
@let hobbit = {name: 'Frodo', age: 53};
{{hobbit.}}
`,
'',
);
templateFile.moveCursorToText('{{hobbit.¦}}');
const completions = templateFile.getCompletionsAtPosition();
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['age', 'name']);
});
it('should complete a shadowed let declaration property', () => {
const {templateFile} = setup(
`
@let hobbit = {name: 'Frodo', age: 53};
@if (true) {
@let hobbit = {hasRing: true, size: 'small'};
{{hobbit.}}
}
`,
'',
);
templateFile.moveCursorToText('{{hobbit.¦}}');
const completions = templateFile.getCompletionsAtPosition();
expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['hasRing', 'size']);
});
});
});
function expectContainInsertText(
@@ -2061,8 +2120,10 @@ function setup(
const otherDirectiveClassDecls = Object.values(otherDeclarations).join('\n\n');
const env = LanguageServiceTestEnv.setup();
const project = env.addProject('test', {
'test.ts': `
const project = env.addProject(
'test',
{
'test.ts': `
import {Component,
input,
output,
@@ -2093,8 +2154,14 @@ function setup(
})
export class AppModule {}
`,
'test.html': template,
});
'test.html': template,
},
// Note: this object is cast to `any`, because for some reason the typing
// changes to the `TestableOption` type aren't being picked up in tests.
{
_enableLetSyntax: true,
} as any,
);
return {templateFile: project.openFile('test.html')};
}
@@ -7,6 +7,7 @@
*/
import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
import ts from 'typescript';
import {
assertFileNames,
@@ -381,6 +382,36 @@ describe('definitions', () => {
assertFileNames(Array.from(definitions!), ['app.html']);
});
it('gets definition for a let declaration', () => {
initMockFileSystem('Native');
const files = {
'app.html': '',
'app.ts': `
import {Component} from '@angular/core';
@Component({templateUrl: '/app.html'})
export class AppCmp {}
`,
};
const env = LanguageServiceTestEnv.setup();
const project = createModuleAndProjectWithDeclarations(env, 'test', files, {
// Note: this object is cast to `any`, because for some reason the typing
// changes to the `TestableOption` type aren't being picked up in tests.
_enableLetSyntax: true,
} as any);
const template = project.openFile('app.html');
template.contents = '@let foo = {value: 123}; {{foo.value}}';
project.expectNoSourceDiagnostics();
template.moveCursorToText('{{fo¦o.value}}');
const {definitions} = getDefinitionsAndAssertBoundSpan(env, template);
expect(definitions[0].name).toEqual('foo');
expect(definitions[0].kind).toBe(ts.ScriptElementKind.variableElement);
expect(definitions[0].textSpan).toBe('@let foo = {value: 123}');
assertFileNames(Array.from(definitions), ['app.html']);
});
function getDefinitionsAndAssertBoundSpan(env: LanguageServiceTestEnv, file: OpenBuffer) {
env.expectNoSourceDiagnostics();
const definitionAndBoundSpan = file.getDefinitionAndBoundSpan();
@@ -18,7 +18,9 @@ describe('definitions', () => {
beforeAll(() => {
const {project, service: _service, tsLS} = setup();
service = _service;
ngLS = new LanguageService(project, tsLS, {});
ngLS = new LanguageService(project, tsLS, {
enableLetSyntax: true,
});
});
beforeEach(() => {
@@ -287,6 +289,19 @@ describe('definitions', () => {
});
});
describe('@let declarations', () => {
it('should work for a @let declaration', () => {
const definitions = getDefinitionsAndAssertBoundSpan({
templateOverride: `@let value = 42; {{val¦ue}}`,
expectedSpanText: 'value',
});
expect(definitions.length).toBe(1);
const [def] = definitions;
expect(def.textSpan).toBe('@let value = 42');
});
});
describe('pipes', () => {
it('should work for pipes', () => {
const templateOverride = `<p>The hero's birthday is {{birthday | da¦te: "MM/dd/yy"}}</p>`;
@@ -105,6 +105,19 @@ describe('language service adapter', () => {
);
});
it('should always disable let declarations if enableLetSyntax is false', () => {
const {project, tsLS} = setup();
const ngLS = new LanguageService(project, tsLS, {
enableLetSyntax: false,
});
expect(ngLS.getCompilerOptions()).toEqual(
jasmine.objectContaining({
'_enableLetSyntax': false,
}),
);
});
it('should pass the @angular/core version along to the compiler', () => {
const {project, tsLS} = setup();
const ngLS = new LanguageService(project, tsLS, {
@@ -41,6 +41,7 @@ function parse(template: string): ParseResult {
leadingTriviaChars: [],
preserveWhitespaces: true,
alwaysAttemptHtmlToR3AstConversion: true,
enableLetSyntax: true,
}),
position,
};
@@ -333,6 +334,15 @@ describe('getTargetAtPosition for template AST', () => {
expect(node).toBeInstanceOf(t.Variable);
});
it('should locate a @let name', () => {
const {errors, nodes, position} = parse(`@let va¦lue = 1337;`);
expect(errors).toBe(null);
const {context} = getTargetAtPosition(nodes, position)!;
const {node} = context as SingleNodeTarget;
expect(isTemplateNode(node!)).toBe(true);
expect(node).toBeInstanceOf(t.LetDeclaration);
});
it('should locate template children', () => {
const {errors, nodes, position} = parse(`<ng-template><d¦iv></div></ng-template>`);
expect(errors).toBe(null);
@@ -661,6 +671,16 @@ describe('getTargetAtPosition for expression AST', () => {
expect(node).toBeInstanceOf(e.Conditional);
});
it('should locate a @let value', () => {
const {errors, nodes, position} = parse(`@let value = 13¦37;`);
expect(errors).toBe(null);
const {context} = getTargetAtPosition(nodes, position)!;
const {node} = context as SingleNodeTarget;
expect(isExpressionNode(node!)).toBe(true);
expect(node).toBeInstanceOf(e.LiteralPrimitive);
expect((node as e.LiteralPrimitive).value).toBe(1337);
});
describe('object literal shorthand', () => {
it('should locate on literal with one shorthand property', () => {
const {errors, nodes, position} = parse(`{{ {va¦l1} }}`);
@@ -18,7 +18,7 @@ describe('type definitions', () => {
beforeAll(() => {
const {project, service: _service, tsLS} = setup();
service = _service;
ngLS = new LanguageService(project, tsLS, {});
ngLS = new LanguageService(project, tsLS, {enableLetSyntax: true});
});
const possibleArrayDefFiles: readonly string[] = [
@@ -231,6 +231,18 @@ describe('type definitions', () => {
});
});
describe('@let declarations', () => {
it('should work for a let declaration', () => {
const definitions = getTypeDefinitions({
templateOverride: `@let address = hero.address; {{addr¦ess}}`,
});
expect(definitions.length).toEqual(1);
expect(definitions[0].textSpan).toBe('Address');
expect(definitions[0].contextSpan).toContain('export interface Address');
});
});
describe('pipes', () => {
it('should work for pipes', () => {
const templateOverride = `<p>The hero's birthday is {{birthday | da¦te: "MM/dd/yy"}}</p>`;
@@ -110,7 +110,11 @@ describe('quick info', () => {
beforeEach(() => {
initMockFileSystem('Native');
env = LanguageServiceTestEnv.setup();
project = env.addProject('test', quickInfoSkeleton());
project = env.addProject('test', quickInfoSkeleton(), {
// Note: this object is cast to `any`, because for some reason the typing
// changes to the `TestableOption` type aren't being picked up in tests.
_enableLetSyntax: true,
} as any);
});
describe('elements', () => {
@@ -705,6 +709,16 @@ describe('quick info', () => {
});
});
describe('let declarations', () => {
it('should get quick info for a let declaration', () => {
expectQuickInfo({
templateOverride: `@let na¦me = 'Frodo';`,
expectedSpanText: `@let name = 'Frodo'`,
expectedDisplayString: `(let) name: "Frodo"`,
});
});
});
it('should work for object literal with shorthand property declarations', () => {
initMockFileSystem('Native');
env = LanguageServiceTestEnv.setup();
@@ -1262,6 +1262,90 @@ describe('find references and rename locations', () => {
});
});
describe('let declarations', () => {
// Note: this object is cast to `any`, because for some reason the typing
// changes to the `TestableOption` type aren't being picked up in tests.
const parseConfig: any = {
_enableLetSyntax: true,
};
describe('when cursor is on the name of the declaration', () => {
let file: OpenBuffer;
beforeEach(() => {
const files = {
'app.ts': `
import {Component} from '@angular/core';
@Component({templateUrl: './template.ng.html'})
export class AppCmp {
}`,
'template.ng.html': `
@let hobbit = 'Frodo';
@let greeting = 'Hello, ' + hobbit;
{{hobbit}}
{{greeting}}
`,
};
env = LanguageServiceTestEnv.setup();
const project = createModuleAndProjectWithDeclarations(env, 'test', files, parseConfig);
file = project.openFile('template.ng.html');
file.moveCursorToText('@let hob¦bit =');
});
it('should find references', () => {
const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(3);
assertFileNames(refs, ['template.ng.html']);
assertTextSpans(refs, ['hobbit']);
});
it('should find rename locations', () => {
const renameLocations = getRenameLocationsAtPosition(file)!;
expect(renameLocations.length).toBe(3);
assertFileNames(renameLocations, ['template.ng.html']);
assertTextSpans(renameLocations, ['hobbit']);
});
});
describe('when cursor is on a usage of the declaration name', () => {
let file: OpenBuffer;
beforeEach(() => {
const files = {
'app.ts': `
import {Component} from '@angular/core';
@Component({templateUrl: './template.ng.html'})
export class AppCmp {
}`,
'template.ng.html': `
@let hobbit = 'Frodo';
@let greeting = 'Hello, ' + hobbit;
{{hobbit}}
{{greeting}}
`,
};
env = LanguageServiceTestEnv.setup();
const project = createModuleAndProjectWithDeclarations(env, 'test', files, parseConfig);
file = project.openFile('template.ng.html');
file.moveCursorToText(`'Hello, ' + hob¦bit`);
});
it('should find references', () => {
const refs = getReferencesAtPosition(file)!;
expect(refs.length).toBe(3);
assertFileNames(refs, ['template.ng.html']);
assertTextSpans(refs, ['hobbit']);
});
it('should find rename locations', () => {
const renameLocations = getRenameLocationsAtPosition(file)!;
expect(renameLocations.length).toBe(3);
assertFileNames(renameLocations, ['template.ng.html']);
assertTextSpans(renameLocations, ['hobbit']);
});
});
});
it('should get references to both input and output for two-way binding', () => {
const files = {
'dir.ts': `