fix(language-service): Prevent crashes on unemitable references (#47938)

dylhunn · AndrewKushnir · commit ce8160ecb28d · 2022-11-03T17:49:11.000-07:00
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
diff --git 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
@@ -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
@@ -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<ts.TextChange>();
+  // 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.

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,8 @@ import {SymbolWithValueDeclaration} from '../../util/src/typescript';`
`18`	`18`	`*/`
`19`	`19`	`export interface PotentialImport {`
`20`	`20`	`kind: PotentialImportKind;`
`21`		`- moduleSpecifier: string;`
	`21`	`+ // If no moduleSpecifier is present, the given symbol name is already in scope.`
	`22`	`+ moduleSpecifier?: string;`
`22`	`23`	`symbolName: string;`
`23`	`24`	`}`
`24`	`25`