From ce8160ecb28d6765d438eb65035835984eb956ec Mon Sep 17 00:00:00 2001 From: Dylan Hunn Date: Tue, 1 Nov 2022 20:08:16 -0700 Subject: [PATCH] fix(language-service): Prevent crashes on unemitable references (#47938) Currently, when generating an import of a selector, the language service might crash if the compiler cannot emit a reference to the new symbol's file from the target component's file. (This might happen because the two are the same file.) We should handle that case by reusing the existing import if possible, or otherwise failing gracefully. PR Close #47938 --- .../compiler-cli/src/ngtsc/typecheck/api/scope.ts | 3 ++- .../src/ngtsc/typecheck/src/checker.ts | 15 ++++++++++++--- .../src/codefixes/fix_missing_import.ts | 12 +++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts index cc3faf62f88..23bbb09d3bf 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts @@ -18,7 +18,8 @@ import {SymbolWithValueDeclaration} from '../../util/src/typescript'; */ export interface PotentialImport { kind: PotentialImportKind; - moduleSpecifier: string; + // If no moduleSpecifier is present, the given symbol name is already in scope. + moduleSpecifier?: string; symbolName: string; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index 9788fb26fae..f4512eda299 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, CssSelector, DomElementSchemaRegistry, ExternalExpr, 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, WrappedNodeExpr} from '@angular/compiler'; import ts from 'typescript'; import {ErrorCode, ngErrorCode} from '../../diagnostics'; @@ -682,6 +682,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { if (toImport.ngModule !== null) { ngModuleRef = this.metaReader.getNgModuleMetadata(new Reference(toImport.ngModule))?.ref; } + const kind = ngModuleRef ? PotentialImportKind.NgModule : PotentialImportKind.Standalone; // Import the ngModule if one exists. Otherwise, import the standalone trait directly. const importTarget = ngModuleRef ?? toImport.ref; @@ -691,9 +692,17 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { // 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 []; + const emittedExpression = emittedRef.expression; + + // This is not be a true import if an appropriate identifier is already in scope. + if (emittedExpression instanceof WrappedNodeExpr) { + return [{kind, symbolName: emittedExpression.node.getText()}]; + } + // Otherwise, it must be a genuine external expression. + if (!(emittedExpression instanceof ExternalExpr)) { + 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 []; diff --git a/packages/language-service/src/codefixes/fix_missing_import.ts b/packages/language-service/src/codefixes/fix_missing_import.ts index 7682f6d6d18..23a6f632ec1 100644 --- a/packages/language-service/src/codefixes/fix_missing_import.ts +++ b/packages/language-service/src/codefixes/fix_missing_import.ts @@ -93,10 +93,13 @@ function getCodeActions( } // Create a code action for this import. + let description = `Import ${importName}`; + if (potentialImport.moduleSpecifier !== undefined) { + description += ` from '${potentialImport.moduleSpecifier}' on ${importOn.name!.text}`; + } codeActions.push({ fixName: FixIdForCodeFixesAll.FIX_MISSING_IMPORT, - description: `Import ${importName} from '${potentialImport.moduleSpecifier}' on ${ - importOn.name!.text}`, + description, changes: [{ fileName: importOn.getSourceFile().fileName, textChanges: [...fileImportChanges, ...traitImportChanges], @@ -115,7 +118,10 @@ function getCodeActions( function updateImportsForTypescriptFile( tsChecker: ts.TypeChecker, file: ts.SourceFile, newImport: PotentialImport, tsFileToImport: ts.SourceFile): [ts.TextChange[], string] { - const changes = new Array(); + // If the expression is already imported, we can just return its name. + if (newImport.moduleSpecifier === undefined) { + return [[], newImport.symbolName]; + } // The trait might already be imported, possibly under a different name. If so, determine the // local name of the imported trait.