diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts index 28ba61e4fb8..2660b45982c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts @@ -145,6 +145,14 @@ export enum PotentialImportMode { ForceDirect, } -export interface PotentialDirectiveModuleSpecifierResolver { - resolve(toImport: Reference, importOn: ts.Node | null): string | undefined; +export interface DirectiveModuleExportDetails { + moduleSpecifier: string; + exportName: string; +} + +export interface PotentialDirectiveModuleSpecifierResolver { + resolve( + toImport: Reference, + importOn: ts.Node | null, + ): DirectiveModuleExportDetails | null; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index 99036c7b7d2..b308cf6baa8 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -26,7 +26,7 @@ import { WrappedNodeExpr, } from '@angular/compiler'; -import {isDirectiveDeclaration} from './ts_util'; +import {isDirectiveDeclaration, isSymbolAliasOf} from './ts_util'; import ts from 'typescript'; @@ -66,6 +66,7 @@ import { isSymbolWithValueDeclaration, } from '../../util/src/typescript'; import { + DirectiveModuleExportDetails, ElementSymbol, FullSourceMapping, GetPotentialAngularMetaOptions, @@ -1053,11 +1054,15 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { const cachedCompletionEntryInfos = resultingDirectives.get(directiveDecl.ref.node)?.tsCompletionEntryInfos ?? []; - cachedCompletionEntryInfos.push({ - tsCompletionEntryData: data, - tsCompletionEntrySymbolFileName: symbolFileName, - tsCompletionEntrySymbolName: symbolName, - }); + appendOrReplaceTsEntryInfo( + cachedCompletionEntryInfos, + { + tsCompletionEntryData: data, + tsCompletionEntrySymbolFileName: symbolFileName, + tsCompletionEntrySymbolName: symbolName, + }, + this.programDriver.getProgram(), + ); if (resultingDirectives.has(directiveDecl.ref.node)) { const directiveInfo = resultingDirectives.get(directiveDecl.ref.node)!; @@ -1283,37 +1288,37 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { */ let highestImportPriority = -1; - const collectImports = (emit: PotentialImport | null, moduleSpecifier: string | undefined) => { + const collectImports = ( + emit: PotentialImport | null, + moduleSpecifierDetail: DirectiveModuleExportDetails | null, + ) => { if (emit === null) { return; } imports.push({ ...emit, - moduleSpecifier: moduleSpecifier ?? emit.moduleSpecifier, + moduleSpecifier: moduleSpecifierDetail?.moduleSpecifier ?? emit.moduleSpecifier, + symbolName: moduleSpecifierDetail?.exportName ?? emit.symbolName, }); - if (moduleSpecifier !== undefined && highestImportPriority === -1) { + if (moduleSpecifierDetail !== null && highestImportPriority === -1) { highestImportPriority = imports.length - 1; } }; if (meta.isStandalone || importMode === PotentialImportMode.ForceDirect) { const emitted = this.emit(PotentialImportKind.Standalone, toImport, inContext); - const moduleSpecifier = potentialDirectiveModuleSpecifierResolver?.resolve( - toImport, - inContext, - ); - collectImports(emitted, moduleSpecifier); + const moduleSpecifierDetail = + potentialDirectiveModuleSpecifierResolver?.resolve(toImport, inContext) ?? null; + collectImports(emitted, moduleSpecifierDetail); } const exportingNgModules = this.ngModuleIndex.getNgModulesExporting(meta.ref.node); if (exportingNgModules !== null) { for (const exporter of exportingNgModules) { const emittedRef = this.emit(PotentialImportKind.NgModule, exporter, inContext); - const moduleSpecifier = potentialDirectiveModuleSpecifierResolver?.resolve( - exporter, - inContext, - ); - collectImports(emittedRef, moduleSpecifier); + const moduleSpecifierDetail = + potentialDirectiveModuleSpecifierResolver?.resolve(exporter, inContext) ?? null; + collectImports(emittedRef, moduleSpecifierDetail); } } @@ -1787,3 +1792,110 @@ type TsDeprecatedDiagnostics = Required { + const currentTsEntrySymbol = getSymbolFromTsEntryInfo(currentTsEntryInfo, program); + if (currentTsEntrySymbol === null) { + return false; + } + return isSymbolTypeMatch(currentTsEntrySymbol, newTsEntryInfoSymbol, typeChecker); + }); + + if (matchedEntryIndex === -1) { + // No entry with a matching type was found, so append the new entry. + tsEntryInfos.push(newTsEntryInfo); + return; + } + + // An entry with a matching type was found at matchedEntryIndex. + const matchedEntry = tsEntryInfos[matchedEntryIndex]; + const matchedEntrySymbol = getSymbolFromTsEntryInfo(matchedEntry, program); + if (matchedEntrySymbol === null) { + // Should not happen based on the findIndex condition, but check defensively. + return; + } + + // Check if the `matchedEntrySymbol` is an alias of the `newTsEntryInfoSymbol`. + if (isSymbolAliasOf(matchedEntrySymbol, newTsEntryInfoSymbol, typeChecker)) { + // The first type-matching entry is an alias, so replace it. + tsEntryInfos[matchedEntryIndex] = newTsEntryInfo; + return; + } + + // The new entry's symbol is an alias of the existing entry's symbol. + // In this case, we prefer to keep the existing entry that was found first + // and do not replace it. + return; +} + +function getSymbolFromTsEntryInfo( + tsInfo: TsCompletionEntryInfo, + program: ts.Program, +): ts.Symbol | null { + const typeChecker = program.getTypeChecker(); + const sf = program.getSourceFile(tsInfo.tsCompletionEntrySymbolFileName); + if (sf === undefined) { + return null; + } + const sfSymbol = typeChecker.getSymbolAtLocation(sf); + if (sfSymbol === undefined) { + return null; + } + + return ( + typeChecker.tryGetMemberInModuleExports(tsInfo.tsCompletionEntrySymbolName, sfSymbol) ?? null + ); +} + +function getFirstTypeDeclarationOfSymbol( + symbol: ts.Symbol, + typeChecker: ts.TypeChecker, +): ts.Declaration | undefined { + const type = typeChecker.getTypeOfSymbol(symbol); + return type.getSymbol()?.declarations?.[0]; +} + +/** + * Check if the two symbols come from the same type node. For example: + * + * The `NewBarComponent`'s type node is the `BarComponent`. + * + * ``` + * // a.ts + * export class BarComponent + * + * // b.ts + * import {BarComponent} from "./a" + * const NewBarComponent = BarComponent; + * export {NewBarComponent} + * ``` + */ +function isSymbolTypeMatch( + first: ts.Symbol, + last: ts.Symbol, + typeChecker: ts.TypeChecker, +): boolean { + const firstTypeNode = getFirstTypeDeclarationOfSymbol(first, typeChecker); + const lastTypeNode = getFirstTypeDeclarationOfSymbol(last, typeChecker); + return firstTypeNode === lastTypeNode && firstTypeNode !== undefined; +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts index 5d5eba4a5f7..6e568010a21 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts @@ -216,3 +216,54 @@ export function isDirectiveDeclaration(node: ts.Node): node is ts.TypeNode | ts. hasExpressionIdentifier(sourceFile, node, ExpressionIdentifier.DIRECTIVE) ); } + +/** + * Check if the lastSymbol is an alias of the firstSymbol. For example: + * + * The NewBarComponent is an alias of BarComponent. + * + * But the NotAliasBarComponent is not an alias of BarComponent, because + * the NotAliasBarComponent is a new variable. + * + * This should work for most cases. + * + * https://github.com/microsoft/TypeScript/blob/9e20e032effad965567d4a1e1c30d5433b0a3332/src/compiler/checker.ts#L3638-L3652 + * + * ``` + * // a.ts + * export class BarComponent {}; + * // b.ts + * export {BarComponent as NewBarComponent} from "./a"; + * // c.ts + * import {BarComponent} from "./a" + * const NotAliasBarComponent = BarComponent; + * export {NotAliasBarComponent}; + * ``` + */ +export function isSymbolAliasOf( + firstSymbol: ts.Symbol, + lastSymbol: ts.Symbol, + typeChecker: ts.TypeChecker, +): boolean { + let currentSymbol: ts.Symbol | undefined = lastSymbol; + + const seenSymbol: Set = new Set(); + while ( + firstSymbol !== currentSymbol && + currentSymbol !== undefined && + currentSymbol.flags & ts.SymbolFlags.Alias + ) { + if (seenSymbol.has(currentSymbol)) { + break; + } + seenSymbol.add(currentSymbol); + + currentSymbol = typeChecker.getImmediateAliasedSymbol(currentSymbol); + + if (currentSymbol === firstSymbol) { + return true; + } + } + + return false; +} diff --git a/packages/language-service/src/utils/ts_utils.ts b/packages/language-service/src/utils/ts_utils.ts index fb50cdbf4fb..ac5880b19a0 100644 --- a/packages/language-service/src/utils/ts_utils.ts +++ b/packages/language-service/src/utils/ts_utils.ts @@ -7,6 +7,7 @@ */ import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core'; import { + DirectiveModuleExportDetails, PotentialDirective, PotentialDirectiveModuleSpecifierResolver, PotentialImportMode, @@ -821,19 +822,30 @@ class PotentialDirectiveModuleSpecifierResolverImpl private readonly includeCompletionsForModuleExports: boolean | undefined, ) {} - resolve(toImport: Reference, importOn: ts.Node | null): string | undefined { + resolve( + toImport: Reference, + importOn: ts.Node | null, + ): DirectiveModuleExportDetails | null { if (toImport.node.getSourceFile().fileName === importOn?.getSourceFile().fileName) { - return undefined; + return null; } - const moduleSpecifier = getModuleSpecifierIfExists(this.compiler, importOn, toImport.node); + const tsEntry = this.getMatchTsEntry(toImport); + const moduleSpecifier = getModuleSpecifierIfExists( + this.compiler, + importOn, + toImport.node, + tsEntry?.tsCompletionEntrySymbolName, + ); if (moduleSpecifier !== null) { - return moduleSpecifier; + return { + moduleSpecifier, + exportName: tsEntry?.tsCompletionEntrySymbolName ?? toImport.node.name.getText(), + }; } return getModuleSpecifierFromImportStatement( - this.directive.tsCompletionEntryInfos, - toImport, + tsEntry, importOn, this.templateTypeChecker, this.component, @@ -841,6 +853,18 @@ class PotentialDirectiveModuleSpecifierResolverImpl this.includeCompletionsForModuleExports, ); } + + private getMatchTsEntry(toImport: Reference): TsCompletionEntryInfo | null { + const program = this.tsLS.getProgram(); + if (program === undefined) { + return null; + } + return findTsCompletionEntryInfoForImport( + this.directive.tsCompletionEntryInfos, + toImport, + program, + ); + } } const importRegex = /\bimport\b[\s\S]*?\bfrom\b\s*(['"`])(.*?)\1/; @@ -854,28 +878,27 @@ const importRegex = /\bimport\b[\s\S]*?\bfrom\b\s*(['"`])(.*?)\1/; * like `import { FooComponent } from '@foo'`. The `@foo` will be returned by the function. */ function getModuleSpecifierFromImportStatement( - tsCompletionEntryInfos: TsCompletionEntryInfo[] | null, - toImport: Reference, + tsCompletionEntryInfo: TsCompletionEntryInfo | null, importOn: ts.Node | null, templateTypeChecker: TemplateTypeChecker, component: ts.ClassDeclaration, tsLS: ts.LanguageService, includeCompletionsForModuleExports: boolean | undefined, -): string | undefined { - const tsCompletionEntryInfo = findTsCompletionEntryInfoForImport( - tsCompletionEntryInfos, - toImport, - ); +): DirectiveModuleExportDetails | null { + const program = tsLS.getProgram(); + if (program === undefined) { + return null; + } - if (tsCompletionEntryInfo === undefined) { - return undefined; + if (tsCompletionEntryInfo === null) { + return null; } const tsEntryName = tsCompletionEntryInfo.tsCompletionEntrySymbolName; const globalContext = templateTypeChecker.getGlobalTsContext(component); if (globalContext === null) { - return undefined; + return null; } const completionListDetail = tsLS.getCompletionEntryDetails( @@ -892,7 +915,7 @@ function getModuleSpecifierFromImportStatement( const actions = completionListDetail?.codeActions; if (actions === undefined) { - return undefined; + return null; } const tcbDir = path.posix.dirname(globalContext.tcbPath); @@ -919,25 +942,41 @@ function getModuleSpecifierFromImportStatement( moduleSpecifier = `./${moduleSpecifier}`; } } - return moduleSpecifier; + return {moduleSpecifier, exportName: tsEntryName}; } } } } - return undefined; + return null; } function findTsCompletionEntryInfoForImport( tsCompletionEntryInfos: TsCompletionEntryInfo[] | null, toImport: Reference, -): TsCompletionEntryInfo | undefined { - const toImportSymbolName = toImport.node.name?.text; - const toImportSymbolFileName = toImport.node.getSourceFile().fileName; + program: ts.Program, +): TsCompletionEntryInfo | null { + const typeChecker = program.getTypeChecker(); - return tsCompletionEntryInfos?.find( - (entry) => - entry.tsCompletionEntrySymbolName === toImportSymbolName && - entry.tsCompletionEntrySymbolFileName === toImportSymbolFileName, + return ( + tsCompletionEntryInfos?.find((tsEntry) => { + const sf = program.getSourceFile(tsEntry.tsCompletionEntrySymbolFileName); + if (sf === undefined) { + return false; + } + const sfSymbol = typeChecker.getSymbolAtLocation(sf); + if (sfSymbol === undefined) { + return false; + } + const tsEntrySymbol = typeChecker.tryGetMemberInModuleExports( + tsEntry.tsCompletionEntrySymbolName, + sfSymbol, + ); + if (tsEntrySymbol === undefined) { + return false; + } + const tsEntryType = typeChecker.getTypeOfSymbol(tsEntrySymbol); + return tsEntryType.getSymbol()?.declarations?.[0] === toImport.node; + }) ?? null ); } @@ -972,6 +1011,7 @@ function getModuleSpecifierIfExists( compiler: NgCompiler, importOn: ts.Node | null, toImport: ClassDeclaration, + exportName: string | undefined, ): string | null { if (importOn === null) { return null; @@ -991,14 +1031,20 @@ function getModuleSpecifierIfExists( } const toImportSymbolFromModule = typeChecker.tryGetMemberInModuleExports( - toImport.name.getText(), + exportName ?? toImport.name.getText(), importSymbol, ); + if (toImportSymbolFromModule === undefined) { + continue; + } + + const symbolType = typeChecker.getTypeOfSymbol(toImportSymbolFromModule); + /** * Make sure these are the same node. */ - if (toImportSymbolFromModule?.declarations?.[0] === toImport) { + if (symbolType.getSymbol()?.declarations?.[0] === toImport) { return getStringLiteralText(importDecl.moduleSpecifier) ?? null; } } diff --git a/packages/language-service/test/code_fixes_spec.ts b/packages/language-service/test/code_fixes_spec.ts index 486cb1d02b3..22834222bad 100644 --- a/packages/language-service/test/code_fixes_spec.ts +++ b/packages/language-service/test/code_fixes_spec.ts @@ -1013,6 +1013,62 @@ describe('code fixes', () => { ]); }); + it('for a re-export symbol from the tsconfig path', () => { + const standaloneFiles = { + 'src/foo.ts': ` + import {Component} from '@angular/core'; + @Component({ + selector: 'foo', + template: '', + standalone: true + }) + export class FooComponent {} + `, + 'component/share/bar.ts': ` + import {Component} from '@angular/core'; + @Component({ + selector: 'bar', + template: '
bar
', + standalone: true + }) + export class BarComponent {} + `, + 'component/share/re_export.ts': ` + import {BarComponent as NewBarComponent1} from "./bar" + export {NewBarComponent1} + `, + 'component/share/public_api.ts': ` + export {NewBarComponent1 as NewBarComponent2} from "./re_export" + `, + 'component/share/index.ts': ` + export {NewBarComponent2 as NewBarComponent3} from "./public_api" + `, + }; + + const project = createModuleAndProjectWithDeclarations(env, 'test', {}, {}, standaloneFiles, { + paths: {'@app/*': ['./component/share/*.ts']}, + }); + const diags = project.getDiagnosticsForFile('src/foo.ts'); + const fixFile = project.openFile('src/foo.ts'); + fixFile.moveCursorToText('<¦bar>'); + + const codeActions = project.getCodeFixesAtPosition( + 'src/foo.ts', + fixFile.cursor, + fixFile.cursor, + [diags[0].code], + ); + const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions); + actionChangesMatch( + actionChanges, + `Import NewBarComponent3 from '@app/index' on FooComponent`, + [ + [``, `import { NewBarComponent3 } from "@app/index";`], + [``, `, imports: [NewBarComponent3]`], + ], + ); + }); + it('for a reusable path from the tsconfig', () => { const standaloneFiles = { 'src/foo.ts': ` @@ -1061,6 +1117,56 @@ describe('code fixes', () => { ]); }); + it('for a reusable path with name export from the tsconfig', () => { + const standaloneFiles = { + 'src/foo.ts': ` + import {Component} from '@angular/core'; + import {BazComponent} from '@app/bar'; + @Component({ + selector: 'foo', + template: '', + imports: [BazComponent] + }) + export class FooComponent {} + `, + 'component/share/bar.ts': ` + import {Component} from '@angular/core'; + @Component({ + selector: 'bar', + template: '
bar
', + }) + class BarComponent {} + + @Component({ + selector: 'baz', + template: '
baz
', + }) + class BazComponent {} + + export {BarComponent, BazComponent}; + `, + }; + + const project = createModuleAndProjectWithDeclarations(env, 'test', {}, {}, standaloneFiles, { + paths: {'@app/*': ['./component/share/*.ts']}, + }); + const diags = project.getDiagnosticsForFile('src/foo.ts'); + const fixFile = project.openFile('src/foo.ts'); + fixFile.moveCursorToText('<¦bar>'); + + const codeActions = project.getCodeFixesAtPosition( + 'src/foo.ts', + fixFile.cursor, + fixFile.cursor, + [diags[0].code], + ); + const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions); + actionChangesMatch(actionChanges, `Import BarComponent from '@app/bar' on FooComponent`, [ + [`{BazComponent}`, `{ BazComponent, BarComponent }`], + [`imports: [BazComponent]`, `imports: [BazComponent, BarComponent]`], + ]); + }); + it('for module specifier existing in the file', () => { const standaloneFiles = { 'src/foo.ts': `