Skip to content

Commit

Permalink
feat(language-service): Quick fix to import a component when its sele…
Browse files Browse the repository at this point in the history
…ctor is used

The language service can now generate an import corresponding to a selector. This includes both the TypeScript module import and the decorator import. This applies to both standalone components and components declared in NgModules.
  • Loading branch information
dylhunn committed Oct 5, 2022
1 parent ab4ef26 commit 3046f95
Show file tree
Hide file tree
Showing 7 changed files with 297 additions and 5 deletions.
8 changes: 7 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts
Expand Up @@ -14,7 +14,7 @@ import {ErrorCode} from '../../diagnostics';

import {FullTemplateMapping, NgTemplateDiagnostic, TypeCheckableDirectiveMeta} from './api';
import {GlobalCompletion} from './completion';
import {PotentialDirective, PotentialPipe} from './scope';
import {PotentialDirective, PotentialImport, PotentialPipe} from './scope';
import {ElementSymbol, Symbol, TcbLocation, TemplateSymbol} from './symbols';

/**
Expand Down Expand Up @@ -146,6 +146,12 @@ export interface TemplateTypeChecker {
*/
getPotentialElementTags(component: ts.ClassDeclaration): Map<string, PotentialDirective|null>;

/**
* In the context of an Angular trait, generate potential imports for a directive.
*/
getPotentialImportsFor(directive: PotentialDirective, inComponent: ts.ClassDeclaration):
ReadonlyArray<PotentialImport>;

/**
* Get the primary decorator for an Angular class (such as @Component). This does not work for
* `@Injectable`.
Expand Down
20 changes: 20 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts
Expand Up @@ -12,6 +12,26 @@ import {EmittedReference, Reference} from '../../imports';
import {ClassDeclaration} from '../../reflection';
import {SymbolWithValueDeclaration} from '../../util/src/typescript';

/**
* A PotentialImport for some Angular trait has a TypeScript module specifier, which can be
* relative, as well as an identifier name.
*/
export interface PotentialImport {
kind: PotentialImportKind;
moduleSpecifier: string;
symbolName: string;
}

/**
* Which kind of Angular Trait the import targets.
*/
export enum PotentialImportKind {
NgModule,
Directive,
Component,
Pipe,
}

/**
* Metadata on a directive which is available in a template.
*/
Expand Down
41 changes: 37 additions & 4 deletions packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
Expand Up @@ -6,21 +6,21 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AST, CssSelector, DomElementSchemaRegistry, LiteralPrimitive, ParseSourceSpan, PropertyRead, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute} from '@angular/compiler';
import {AST, CssSelector, DomElementSchemaRegistry, ExternalExpr, LiteralPrimitive, ParseSourceSpan, PropertyRead, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute} from '@angular/compiler';
import ts from 'typescript';

import {ErrorCode, ngErrorCode} from '../../diagnostics';
import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath, getSourceFileOrError} from '../../file_system';
import {Reference, ReferenceEmitter} from '../../imports';
import {Reference, ReferenceEmitKind, ReferenceEmitter} from '../../imports';
import {IncrementalBuild} from '../../incremental/api';
import {DirectiveMeta, MetadataReader, MetadataReaderWithIndex, MetaKind} from '../../metadata';
import {DirectiveMeta, MetadataReader, MetadataReaderWithIndex, MetaKind, NgModuleMeta} from '../../metadata';
import {PerfCheckpoint, PerfEvent, PerfPhase, PerfRecorder} from '../../perf';
import {ProgramDriver, UpdateMode} from '../../program_driver';
import {ClassDeclaration, DeclarationNode, isNamedClassDeclaration, ReflectionHost} from '../../reflection';
import {ComponentScopeKind, ComponentScopeReader, TypeCheckScopeRegistry} from '../../scope';
import {isShim} from '../../shims';
import {getSourceFileOrNull, isSymbolWithValueDeclaration} from '../../util/src/typescript';
import {ElementSymbol, FullTemplateMapping, GlobalCompletion, NgTemplateDiagnostic, OptimizeFor, PotentialDirective, PotentialPipe, ProgramTypeCheckAdapter, Symbol, TcbLocation, TemplateDiagnostic, TemplateId, TemplateSymbol, TemplateTypeChecker, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../api';
import {ElementSymbol, FullTemplateMapping, GlobalCompletion, NgTemplateDiagnostic, OptimizeFor, PotentialDirective, PotentialImport, PotentialImportKind, PotentialPipe, ProgramTypeCheckAdapter, Symbol, TcbLocation, TemplateDiagnostic, TemplateId, TemplateSymbol, TemplateTypeChecker, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../api';
import {makeTemplateDiagnostic} from '../diagnostics';

import {CompletionEngine} from './completion';
Expand Down Expand Up @@ -673,6 +673,39 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
return scope.ngModule;
}

getPotentialImportsFor(toImport: PotentialDirective, inContext: ts.ClassDeclaration):
ReadonlyArray<PotentialImport> {
// Look up the original reference associated with the trait's ngModule, so we don't lose the
// Reference context (such as identifiers). If the trait is standalone, this will be
// `undefined`.
let ngModuleRef: Reference<ClassDeclaration<DeclarationNode>>|undefined;
if (toImport.ngModule !== null) {
ngModuleRef = this.metaReader.getNgModuleMetadata(new Reference(toImport.ngModule))?.ref;
}

// Import the ngModule if one exists. Otherwise, import the standalone trait directly.
const importTarget = ngModuleRef ?? toImport.ref;

// Using the compiler's ReferenceEmitter, try to emit a reference to the trait.
// TODO(dylhunn): In the future, we can use a more sophisticated strategy for generating and
// ranking references, such as keeping a record of import specifiers used in existing code.
const emittedRef = this.refEmitter.emit(importTarget, inContext.getSourceFile());
if (emittedRef.kind === ReferenceEmitKind.Failed) return [];

// The resulting import expression should have a module name and an identifier name.
const emittedExpression: ExternalExpr = emittedRef.expression as ExternalExpr;
if (emittedExpression.value.moduleName === null || emittedExpression.value.name === null)
return [];

// Extract and return the TS module and identifier names.
const preferredImport: PotentialImport = {
kind: ngModuleRef ? PotentialImportKind.NgModule : PotentialImportKind.Directive,
moduleSpecifier: emittedExpression.value.moduleName,
symbolName: emittedExpression.value.name,
};
return [preferredImport];
}

private getScopeData(component: ts.ClassDeclaration): ScopeData|null {
if (this.scopeCache.has(component)) {
return this.scopeCache.get(component)!;
Expand Down
93 changes: 93 additions & 0 deletions packages/compiler-cli/test/ngtsc/ls_typecheck_helpers_spec.ts
Expand Up @@ -201,5 +201,98 @@ runInEachFileSystem(() => {
expect(directives.map(d => d.selector)).toContain('two-cmp');
});
});

describe('can generate imports` ', () => {
it('for out of scope standalone components', () => {
env.write('one.ts', `
import {Component} from '@angular/core';
@Component({
standalone: true,
selector: 'one-cmp',
template: '<div></div>',
})
export class OneCmp {}
`);

env.write('two.ts', `
import {Component} from '@angular/core';
@Component({
standalone: true,
selector: 'two-cmp',
template: '<div></div>',
})
export class TwoCmp {}
`);
const {program, checker} = env.driveTemplateTypeChecker();
const sfOne = program.getSourceFile(_('/one.ts'));
expect(sfOne).not.toBeNull();
const OneCmpClass = getClass(sfOne!, 'OneCmp');

const TwoCmpDir = checker.getPotentialTemplateDirectives(OneCmpClass)
.filter(d => d.selector === 'two-cmp')[0];
const imports = checker.getPotentialImportsFor(TwoCmpDir, OneCmpClass);

expect(imports.length).toBe(1);
expect(imports[0].moduleSpecifier).toBe('./two');
expect(imports[0].symbolName).toBe('TwoCmp');
});

it('for out of scope ngModules', () => {
env.write('one.ts', `
import {Component} from '@angular/core';
@Component({
standalone: true,
selector: 'one-cmp',
template: '<div></div>',
})
export class OneCmp {}
`);

env.write('two.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'two-cmp',
template: '<div></div>',
})
export class TwoCmp {}
`);

env.write('twomod.ts', `
import { NgModule } from '@angular/core';
import { CommonModule } from '@angular/common';
import { TwoCmp } from './two';
@NgModule({
declarations: [
TwoCmp
],
exports: [
TwoCmp
],
imports: [
CommonModule
]
})
export class TwoModule { }
`);

const {program, checker} = env.driveTemplateTypeChecker();
const sfOne = program.getSourceFile(_('/one.ts'));
expect(sfOne).not.toBeNull();
const OneCmpClass = getClass(sfOne!, 'OneCmp');

const TwoNgMod = checker.getPotentialTemplateDirectives(OneCmpClass)
.filter(d => d.selector === 'two-cmp')[0];
const imports = checker.getPotentialImportsFor(TwoNgMod, OneCmpClass);

expect(imports.length).toBe(1);
expect(imports[0].moduleSpecifier).toBe('./twomod');
expect(imports[0].symbolName).toBe('TwoModule');
});
});
});
});
Expand Up @@ -7,10 +7,12 @@
*/

import {fixInvalidBananaInBoxMeta} from './fix_invalid_banana_in_box';
import {missingImportMeta} from './fix_missing_import';
import {missingMemberMeta} from './fix_missing_member';
import {CodeActionMeta} from './utils';

export const ALL_CODE_FIXES_METAS: CodeActionMeta[] = [
missingMemberMeta,
fixInvalidBananaInBoxMeta,
missingImportMeta,
];
137 changes: 137 additions & 0 deletions packages/language-service/src/codefixes/fix_missing_import.ts
@@ -0,0 +1,137 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {ErrorCode as NgCompilerErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics/index';
import {TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
import {findFirstMatchingNode} from '@angular/compiler-cli/src/ngtsc/typecheck/src/comments';
import * as e from '@angular/compiler/src/expression_parser/ast'; // e for expression AST
import ts from 'typescript';

import {getTargetAtPosition, TargetNodeKind} from '../template_target';
import {updateObjectValueForKey} from '../ts_utils';

import {CodeActionMeta, convertFileTextChangeInTcb, FixIdForCodeFixesAll} from './utils';

const errorCodes: number[] = [
ngErrorCode(NgCompilerErrorCode.SCHEMA_INVALID_ELEMENT),
];

let printer: ts.Printer|null = null;

function print(node: ts.Node, sourceFile: ts.SourceFile): string {
if (printer === null) printer = ts.createPrinter();
return printer.printNode(ts.EmitHint.Unspecified, node, sourceFile);
}

function updateImportsForAngularTrait(
checker: TemplateTypeChecker, trait: ts.ClassDeclaration,
importName: string): ts.FileTextChanges|null {
const decorator = checker.getPrimaryAngularDecorator(trait);
if (decorator === null) return null;
const decoratorProps = findFirstMatchingNode(decorator, {filter: ts.isObjectLiteralExpression});
if (decoratorProps === null) return null;
const appendArrayValFn = (oldValue?: ts.Expression) => {
const newComponentStringLiteral = ts.factory.createIdentifier(importName);
if (!oldValue) return ts.factory.createArrayLiteralExpression([newComponentStringLiteral]);
if (!ts.isArrayLiteralExpression(oldValue)) return oldValue;
if (oldValue.elements.some(e => ts.isIdentifier(e) && e.text === importName)) {
return oldValue;
}
return ts.factory.updateArrayLiteralExpression(oldValue, [
...oldValue.elements.filter((v) => ts.isIdentifier(v) && v.text !== importName),
ts.factory.createIdentifier(importName),
]);
};
const newDecoratorProps = updateObjectValueForKey(decoratorProps, 'imports', appendArrayValFn);
return {
fileName: trait.getSourceFile().fileName,
textChanges: [{
span: {
start: decoratorProps.getStart(),
length: decoratorProps.getEnd() - decoratorProps.getStart()
},
newText: print(newDecoratorProps, trait.getSourceFile())
}],
};
}

/**
* This code action will generate a new import for an unknown selector.
*/
export const missingImportMeta: CodeActionMeta = {
errorCodes,
getCodeActions: function(
{templateInfo, start, compiler, formatOptions, preferences, errorCode, tsLs}) {
const checker = compiler.getTemplateTypeChecker();

// The error must be an invalid element in tag, which is interpreted as an intended selector.
// TODO: Support unknown selectors in additional positions, such as attributes.
const target = getTargetAtPosition(templateInfo.template, start);
if (target?.context.kind !== TargetNodeKind.ElementInTagContext) return [];
const missingComponentSelector = (target.context.node as any).name as string | undefined;
if (!missingComponentSelector) return [];

// The class which has an imports array; either a standalone trait or its owning NgModule.
const clazz = checker.getOwningNgModule(templateInfo.component) ?? templateInfo.component;

// Call type checker APIs to determine the most likely directive for that selector, and the best
// import for it.
// TODO: We can be smarter here about multiple matches.
const allPossibleDirectives = checker.getPotentialTemplateDirectives(templateInfo.component);
const matchingDirectives =
allPossibleDirectives.filter((d) => d.selector === missingComponentSelector);
if (matchingDirectives.length === 0) return [];
const bestMatch = matchingDirectives[0];
const originalImportedSymbolName = bestMatch.tsSymbol.name;
const potentialImports = checker.getPotentialImportsFor(bestMatch, clazz);
if (potentialImports.length == 0) return [];
const bestPotentialImport = potentialImports[0];

// Update the imports at the top of the file.
// TODO: Find the import if already present (possibly qualified).
// TODO: Use AST builder.
const importString = `import { ${bestPotentialImport.symbolName} } from '${
bestPotentialImport.moduleSpecifier}';\n`;
const fileImportChange = {
fileName: clazz.getSourceFile().fileName,
textChanges: [{span: {start: 0, length: 0}, newText: importString}],
};

// Update the imports in the decorator's `imports` array.
const componentImportChange =
updateImportsForAngularTrait(checker, clazz, bestPotentialImport.symbolName);
if (componentImportChange === null) return [];

// Create the code action to insert the new imports.
const codeActions: ts.CodeFixAction[] = [{
fixName: FixIdForCodeFixesAll.FIX_MISSING_IMPORT,
description: `Import ${bestPotentialImport.symbolName} from '${
bestPotentialImport.moduleSpecifier}' on ${clazz.name?.text}`,
changes: [componentImportChange, fileImportChange],
}];
return codeActions.map(codeAction => {
return {
fixName: codeAction.fixName,
fixId: codeAction.fixId,
fixAllDescription: codeAction.fixAllDescription,
description: codeAction.description,
changes: convertFileTextChangeInTcb(codeAction.changes, compiler),
commands: codeAction.commands,
};
});
},
fixIds: [FixIdForCodeFixesAll.FIX_MISSING_IMPORT],
getAllCodeActions: function(
{tsLs, scope, fixId, formatOptions, preferences, compiler, diagnostics}) {
// 'Fix All' is not supported, because the imports may come from different files and should be
// examined individually.
return {
changes: [],
};
}
};
1 change: 1 addition & 0 deletions packages/language-service/src/codefixes/utils.ts
Expand Up @@ -134,4 +134,5 @@ export enum FixIdForCodeFixesAll {
FIX_SPELLING = 'fixSpelling',
FIX_MISSING_MEMBER = 'fixMissingMember',
FIX_INVALID_BANANA_IN_BOX = 'fixInvalidBananaInBox',
FIX_MISSING_IMPORT = 'fixMissingImport',
}

0 comments on commit 3046f95

Please sign in to comment.