Skip to content

Commit 1413334

Browse files
dylhunnAndrewKushnir
dylhunn
authored and
AndrewKushnir
committed
feat(language-service): Introduce a new NgModuleIndex, and use it to suggest re-exports. (#48354)
NgModules can re-export other NgModules, which transitively includes all traits exported by the re-exported NgModule. We want to be able to suggest *all* re-exports of a component when trying to auto-import it. Previously, we used an approximation when importing from NgModules: we looked at a Component's metadata, and just imported the declaring NgModule. However, this is not technically correct -- the declaring NgModule it is not necessarily the same one that exports it for the current scope. (Indeed, there could be multiple re-exports!) As a replacement, I have implemented a more general solution. This PR introduces a new class on the template type checker, called `NgModuleIndex`. When queried, it conducts a depth-first-search over the NgModule import/export graph, in order to find all NgModules anywhere in the current dependency graph, as well as all exports of those NgModules. This allows the language service to suggest all of the re-exports, in addition to the original export. PR Close #48354
1 parent ea8b339 commit 1413334

File tree

4 files changed

+178
-37
lines changed

4 files changed

+178
-37
lines changed

‎packages/compiler-cli/src/ngtsc/core/src/compiler.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {IncrementalBuildStrategy, IncrementalCompilation, IncrementalState} from
1919
import {SemanticSymbol} from '../../incremental/semantic_graph';
2020
import {generateAnalysis, IndexedComponent, IndexingContext} from '../../indexer';
2121
import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, HostDirectivesResolver, LocalMetadataRegistry, MetadataReader, MetadataReaderWithIndex, PipeMeta, ResourceRegistry} from '../../metadata';
22+
import {NgModuleIndexImpl} from '../../metadata/src/ng_module_index';
2223
import {PartialEvaluator} from '../../partial_evaluator';
2324
import {ActivePerfRecorder, DelegatingPerfRecorder, PerfCheckpoint, PerfEvent, PerfPhase} from '../../perf';
2425
import {FileUpdate, ProgramDriver, UpdateMode} from '../../program_driver';
@@ -971,6 +972,7 @@ export class NgCompiler {
971972
const localMetaReader: MetadataReaderWithIndex = localMetaRegistry;
972973
const depScopeReader = new MetadataDtsModuleScopeResolver(dtsReader, aliasingHost);
973974
const metaReader = new CompoundMetadataReader([localMetaReader, dtsReader]);
975+
const ngModuleIndex = new NgModuleIndexImpl(metaReader, localMetaReader);
974976
const ngModuleScopeRegistry = new LocalModuleScopeRegistry(
975977
localMetaReader, metaReader, depScopeReader, refEmitter, aliasingHost);
976978
const standaloneScopeReader =
@@ -1072,7 +1074,7 @@ export class NgCompiler {
10721074
const templateTypeChecker = new TemplateTypeCheckerImpl(
10731075
this.inputProgram, notifyingDriver, traitCompiler, this.getTypeCheckingConfig(), refEmitter,
10741076
reflector, this.adapter, this.incrementalCompilation, metaReader, localMetaReader,
1075-
scopeReader, typeCheckScopeRegistry, this.delegatingPerfRecorder);
1077+
ngModuleIndex, scopeReader, typeCheckScopeRegistry, this.delegatingPerfRecorder);
10761078

10771079
// Only construct the extended template checker if the configuration is valid and usable.
10781080
const extendedTemplateChecker = this.constructionDiagnostics.length === 0 ?

‎packages/compiler-cli/src/ngtsc/metadata/src/ng_module_index.ts‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Reference} from '../../imports';
10+
import {ClassDeclaration} from '../../reflection';
11+
12+
import {MetadataReader, MetadataReaderWithIndex, MetaKind, NgModuleIndex} from './api';
13+
14+
/**
15+
* An index of all NgModules that export or re-export a given trait.
16+
*/
17+
export class NgModuleIndexImpl implements NgModuleIndex {
18+
constructor(private metaReader: MetadataReader, private localReader: MetadataReaderWithIndex) {}
19+
20+
// A map from an NgModule's Class Declaration to the "main" reference to that module, aka the one
21+
// present in the reader metadata object
22+
private ngModuleAuthoritativeReference = new Map<ClassDeclaration, Reference<ClassDeclaration>>();
23+
// A map from a Directive/Pipe's class declaration to the class declarations of all re-exporting
24+
// NgModules
25+
private typeToExportingModules = new Map<ClassDeclaration, Set<ClassDeclaration>>();
26+
27+
private indexed = false;
28+
29+
private updateWith<K, V>(cache: Map<K, Set<V>>, key: K, elem: V) {
30+
if (cache.has(key)) {
31+
cache.get(key)!.add(elem);
32+
} else {
33+
const set = new Set<V>();
34+
set.add(elem);
35+
cache.set(key, set);
36+
}
37+
}
38+
39+
private index(): void {
40+
const seenTypesWithReexports = new Map<ClassDeclaration, Set<ClassDeclaration>>();
41+
const locallyDeclaredDirsAndNgModules = [
42+
...this.localReader.getKnown(MetaKind.NgModule),
43+
...this.localReader.getKnown(MetaKind.Directive),
44+
];
45+
for (const decl of locallyDeclaredDirsAndNgModules) {
46+
// Here it's safe to create a new Reference because these are known local types.
47+
this.indexTrait(new Reference(decl), seenTypesWithReexports);
48+
}
49+
this.indexed = true;
50+
}
51+
52+
private indexTrait(
53+
ref: Reference<ClassDeclaration>,
54+
seenTypesWithReexports: Map<ClassDeclaration, Set<ClassDeclaration>>): void {
55+
if (seenTypesWithReexports.has(ref.node)) {
56+
// We've processed this type before.
57+
return;
58+
}
59+
seenTypesWithReexports.set(ref.node, new Set());
60+
61+
const meta =
62+
this.metaReader.getDirectiveMetadata(ref) ?? this.metaReader.getNgModuleMetadata(ref);
63+
if (meta === null) {
64+
return;
65+
}
66+
67+
// Component + NgModule: recurse into imports
68+
if (meta.imports !== null) {
69+
for (const childRef of meta.imports) {
70+
this.indexTrait(childRef, seenTypesWithReexports);
71+
}
72+
}
73+
74+
if (meta.kind === MetaKind.NgModule) {
75+
if (!this.ngModuleAuthoritativeReference.has(ref.node)) {
76+
this.ngModuleAuthoritativeReference.set(ref.node, ref);
77+
}
78+
79+
for (const childRef of meta.exports) {
80+
this.indexTrait(childRef, seenTypesWithReexports);
81+
82+
const childMeta = this.metaReader.getDirectiveMetadata(childRef) ??
83+
this.metaReader.getPipeMetadata(childRef) ??
84+
this.metaReader.getNgModuleMetadata(childRef);
85+
if (childMeta === null) {
86+
continue;
87+
}
88+
89+
switch (childMeta.kind) {
90+
case MetaKind.Directive:
91+
case MetaKind.Pipe:
92+
this.updateWith(this.typeToExportingModules, childRef.node, ref.node);
93+
this.updateWith(seenTypesWithReexports, ref.node, childRef.node);
94+
break;
95+
case MetaKind.NgModule:
96+
if (seenTypesWithReexports.has(childRef.node)) {
97+
for (const reexported of seenTypesWithReexports.get(childRef.node)!) {
98+
this.updateWith(this.typeToExportingModules, reexported, ref.node);
99+
this.updateWith(seenTypesWithReexports, ref.node, reexported);
100+
}
101+
}
102+
break;
103+
}
104+
}
105+
}
106+
}
107+
108+
getNgModulesExporting(directiveOrPipe: ClassDeclaration): Array<Reference<ClassDeclaration>> {
109+
if (!this.indexed) {
110+
this.index();
111+
}
112+
113+
if (!this.typeToExportingModules.has(directiveOrPipe)) {
114+
return [];
115+
}
116+
117+
const refs: Array<Reference<ClassDeclaration>> = [];
118+
for (const ngModule of this.typeToExportingModules.get(directiveOrPipe)!) {
119+
if (this.ngModuleAuthoritativeReference.has(ngModule)) {
120+
refs.push(this.ngModuleAuthoritativeReference.get(ngModule)!);
121+
}
122+
}
123+
return refs;
124+
}
125+
}

‎packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,9 @@ export interface TemplateTypeChecker {
149149
/**
150150
* In the context of an Angular trait, generate potential imports for a directive.
151151
*/
152-
getPotentialImportsFor(directive: PotentialDirective, inComponent: ts.ClassDeclaration):
153-
ReadonlyArray<PotentialImport>;
152+
getPotentialImportsFor(
153+
toImport: PotentialDirective|PotentialPipe,
154+
inComponent: ts.ClassDeclaration): ReadonlyArray<PotentialImport>;
154155

155156
/**
156157
* Get the primary decorator for an Angular class (such as @Component). This does not work for

‎packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts‎

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {ErrorCode, ngErrorCode} from '../../diagnostics';
1313
import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath, getSourceFileOrError} from '../../file_system';
1414
import {Reference, ReferenceEmitKind, ReferenceEmitter} from '../../imports';
1515
import {IncrementalBuild} from '../../incremental/api';
16-
import {DirectiveMeta, MetadataReader, MetadataReaderWithIndex, MetaKind, NgModuleMeta, PipeMeta} from '../../metadata';
16+
import {DirectiveMeta, MetadataReader, MetadataReaderWithIndex, MetaKind, NgModuleIndex, PipeMeta} from '../../metadata';
1717
import {PerfCheckpoint, PerfEvent, PerfPhase, PerfRecorder} from '../../perf';
1818
import {ProgramDriver, UpdateMode} from '../../program_driver';
1919
import {ClassDeclaration, DeclarationNode, isNamedClassDeclaration, ReflectionHost} from '../../reflection';
@@ -86,6 +86,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
8686
private priorBuild: IncrementalBuild<unknown, FileTypeCheckingData>,
8787
private readonly metaReader: MetadataReader,
8888
private readonly localMetaReader: MetadataReaderWithIndex,
89+
private readonly ngModuleIndex: NgModuleIndex,
8990
private readonly componentScopeReader: ComponentScopeReader,
9091
private readonly typeCheckScopeRegistry: TypeCheckScopeRegistry,
9192
private readonly perf: PerfRecorder) {}
@@ -687,46 +688,58 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
687688
return scope.ngModule;
688689
}
689690

690-
getPotentialImportsFor(toImport: PotentialDirective, inContext: ts.ClassDeclaration):
691-
ReadonlyArray<PotentialImport> {
692-
// Look up the original reference associated with the trait's ngModule, so we don't lose the
693-
// Reference context (such as identifiers). If the trait is standalone, this will be
694-
// `undefined`.
695-
let ngModuleRef: Reference<ClassDeclaration<DeclarationNode>>|undefined;
696-
if (toImport.ngModule !== null) {
697-
ngModuleRef = this.metaReader.getNgModuleMetadata(new Reference(toImport.ngModule))?.ref;
691+
private emit(
692+
kind: PotentialImportKind, refTo: Reference<ClassDeclaration>,
693+
inContext: ts.ClassDeclaration): PotentialImport|null {
694+
const emittedRef = this.refEmitter.emit(refTo, inContext.getSourceFile());
695+
if (emittedRef.kind === ReferenceEmitKind.Failed) {
696+
return null;
698697
}
699-
const kind = ngModuleRef ? PotentialImportKind.NgModule : PotentialImportKind.Standalone;
700-
701-
// Import the ngModule if one exists. Otherwise, import the standalone trait directly.
702-
const importTarget = ngModuleRef ?? toImport.ref;
698+
const emitted = emittedRef.expression;
699+
if (emitted instanceof WrappedNodeExpr) {
700+
// An appropriate identifier is already in scope.
701+
return {kind, symbolName: emitted.node.text};
702+
} else if (
703+
emitted instanceof ExternalExpr && emitted.value.moduleName !== null &&
704+
emitted.value.name !== null) {
705+
return {
706+
kind,
707+
moduleSpecifier: emitted.value.moduleName,
708+
symbolName: emitted.value.name,
709+
};
710+
}
711+
return null;
712+
}
703713

704-
// Using the compiler's ReferenceEmitter, try to emit a reference to the trait.
705-
// TODO(dylhunn): In the future, we can use a more sophisticated strategy for generating and
706-
// ranking references, such as keeping a record of import specifiers used in existing code.
707-
const emittedRef = this.refEmitter.emit(importTarget, inContext.getSourceFile());
708-
if (emittedRef.kind === ReferenceEmitKind.Failed) return [];
709-
const emittedExpression = emittedRef.expression;
714+
getPotentialImportsFor(
715+
toImport: PotentialDirective|PotentialPipe,
716+
inContext: ts.ClassDeclaration): ReadonlyArray<PotentialImport> {
717+
const imports: PotentialImport[] = [];
710718

711-
// This is not be a true import if an appropriate identifier is already in scope.
712-
if (emittedExpression instanceof WrappedNodeExpr) {
713-
return [{kind, symbolName: emittedExpression.node.getText()}];
719+
const meta = this.metaReader.getDirectiveMetadata(toImport.ref) ??
720+
this.metaReader.getPipeMetadata(toImport.ref);
721+
if (meta === null) {
722+
return imports;
714723
}
715-
// Otherwise, it must be a genuine external expression.
716-
if (!(emittedExpression instanceof ExternalExpr)) {
717-
return [];
724+
725+
if (meta.isStandalone) {
726+
const emitted = this.emit(PotentialImportKind.Standalone, toImport.ref, inContext);
727+
if (emitted !== null) {
728+
imports.push(emitted);
729+
}
718730
}
719731

720-
if (emittedExpression.value.moduleName === null || emittedExpression.value.name === null)
721-
return [];
732+
const exportingNgModules = this.ngModuleIndex.getNgModulesExporting(meta.ref.node);
733+
if (exportingNgModules !== null) {
734+
for (const exporter of exportingNgModules) {
735+
const emittedRef = this.emit(PotentialImportKind.Standalone, exporter, inContext);
736+
if (emittedRef !== null) {
737+
imports.push(emittedRef);
738+
}
739+
}
740+
}
722741

723-
// Extract and return the TS module and identifier names.
724-
const preferredImport: PotentialImport = {
725-
kind: ngModuleRef ? PotentialImportKind.NgModule : PotentialImportKind.Standalone,
726-
moduleSpecifier: emittedExpression.value.moduleName,
727-
symbolName: emittedExpression.value.name,
728-
};
729-
return [preferredImport];
742+
return imports;
730743
}
731744

732745
private getScopeData(component: ts.ClassDeclaration): ScopeData|null {

0 commit comments

Comments
 (0)