Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dylhunn wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix(language-service): Prevent crashes on unemitable references 
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.
  • Loading branch information
@dylhunn
dylhunn committed Nov 2, 2022
commit 3620fff43a910afd419ebad17b8fa296b6c6a1b6
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
15 changes: 12 additions & 3 deletions packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -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 [];

Expand Down
12 changes: 9 additions & 3 deletions packages/language-service/src/codefixes/fix_missing_import.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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.
Expand Down