Skip to content

Commit e10b3e2

Browse files
atscottjosephperrott
authored andcommitted
refactor(compiler): Add ngModule to directive symbol (angular#39099)
This is needed so that the Language Service can provide the module name in the quick info for a directive/component. To accomplish this, the compiler's `LocalModuleScope` is provided to the `TemplateTypeCheckerImpl`. This will also allow the `TemplateTypeChecker` to provide more completions in the future, giving it a way to determine all the directives/pipes/etc. available to a template. PR Close angular#39099
1 parent 57f8dd2 commit e10b3e2

File tree

8 files changed

+79
-11
lines changed

8 files changed

+79
-11
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, Inj
2121
import {ModuleWithProvidersScanner} from '../../modulewithproviders';
2222
import {PartialEvaluator} from '../../partial_evaluator';
2323
import {NOOP_PERF_RECORDER, PerfRecorder} from '../../perf';
24-
import {TypeScriptReflectionHost} from '../../reflection';
24+
import {ClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
2525
import {AdapterResourceLoader} from '../../resource';
2626
import {entryPointKeyFor, NgModuleRouteAnalyzer} from '../../routing';
2727
import {ComponentScopeReader, LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope';
@@ -780,7 +780,8 @@ export class NgCompiler {
780780

781781
const templateTypeChecker = new TemplateTypeCheckerImpl(
782782
this.tsProgram, this.typeCheckingProgramStrategy, traitCompiler,
783-
this.getTypeCheckingConfig(), refEmitter, reflector, this.adapter, this.incrementalDriver);
783+
this.getTypeCheckingConfig(), refEmitter, reflector, this.adapter, this.incrementalDriver,
784+
scopeRegistry);
784785

785786
return {
786787
isCore,

packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ ts_library(
1616
"//packages/compiler-cli/src/ngtsc/incremental:api",
1717
"//packages/compiler-cli/src/ngtsc/metadata",
1818
"//packages/compiler-cli/src/ngtsc/reflection",
19+
"//packages/compiler-cli/src/ngtsc/scope",
1920
"//packages/compiler-cli/src/ngtsc/shims",
2021
"//packages/compiler-cli/src/ngtsc/shims:api",
2122
"//packages/compiler-cli/src/ngtsc/translator",

packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {TmplAstElement, TmplAstReference, TmplAstTemplate, TmplAstVariable} from
1010
import * as ts from 'typescript';
1111

1212
import {AbsoluteFsPath} from '../../file_system';
13+
import {ClassDeclaration} from '../../reflection';
1314

1415
export enum SymbolKind {
1516
Input,
@@ -238,6 +239,9 @@ export interface DirectiveSymbol {
238239

239240
/** `true` if this `DirectiveSymbol` is for a @Component. */
240241
isComponent: boolean;
242+
243+
/** The `NgModule` that this directive is declared in or `null` if it could not be determined. */
244+
ngModule: ClassDeclaration|null;
241245
}
242246

243247
/**

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {absoluteFromSourceFile, AbsoluteFsPath, getSourceFileOrError} from '../.
1313
import {ReferenceEmitter} from '../../imports';
1414
import {IncrementalBuild} from '../../incremental/api';
1515
import {ReflectionHost} from '../../reflection';
16+
import {ComponentScopeReader} from '../../scope';
1617
import {isShim} from '../../shims';
1718
import {getSourceFileOrNull} from '../../util/src/typescript';
1819
import {OptimizeFor, ProgramTypeCheckAdapter, Symbol, TemplateId, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
@@ -38,7 +39,8 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
3839
private typeCheckAdapter: ProgramTypeCheckAdapter, private config: TypeCheckingConfig,
3940
private refEmitter: ReferenceEmitter, private reflector: ReflectionHost,
4041
private compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
41-
private priorBuild: IncrementalBuild<unknown, FileTypeCheckingData>) {}
42+
private priorBuild: IncrementalBuild<unknown, FileTypeCheckingData>,
43+
private readonly componentScopeReader: ComponentScopeReader) {}
4244

4345
resetOverrides(): void {
4446
for (const fileRecord of this.state.values()) {
@@ -372,7 +374,8 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
372374
return null;
373375
}
374376

375-
return new SymbolBuilder(typeChecker, shimPath, tcb, data).getSymbol(node);
377+
return new SymbolBuilder(typeChecker, shimPath, tcb, data, this.componentScopeReader)
378+
.getSymbol(node);
376379
}
377380
}
378381

packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {AST, ASTWithSource, BindingPipe, MethodCall, PropertyWrite, SafeMethodCa
1010
import * as ts from 'typescript';
1111

1212
import {AbsoluteFsPath} from '../../file_system';
13+
import {ClassDeclaration} from '../../reflection';
14+
import {ComponentScopeReader} from '../../scope';
1315
import {isAssignment} from '../../util/src/typescript';
1416
import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, ReferenceSymbol, Symbol, SymbolKind, TemplateSymbol, TsNodeSymbolInfo, TypeCheckableDirectiveMeta, VariableSymbol} from '../api';
1517

@@ -24,8 +26,12 @@ import {TcbDirectiveOutputsOp} from './type_check_block';
2426
*/
2527
export class SymbolBuilder {
2628
constructor(
27-
private readonly typeChecker: ts.TypeChecker, private readonly shimPath: AbsoluteFsPath,
28-
private readonly typeCheckBlock: ts.Node, private readonly templateData: TemplateData) {}
29+
private readonly typeChecker: ts.TypeChecker,
30+
private readonly shimPath: AbsoluteFsPath,
31+
private readonly typeCheckBlock: ts.Node,
32+
private readonly templateData: TemplateData,
33+
private readonly componentScopeReader: ComponentScopeReader,
34+
) {}
2935

3036
getSymbol(node: TmplAstTemplate|TmplAstElement): TemplateSymbol|ElementSymbol|null;
3137
getSymbol(node: TmplAstReference|TmplAstVariable): ReferenceSymbol|VariableSymbol|null;
@@ -99,21 +105,24 @@ export class SymbolBuilder {
99105
.map(node => {
100106
const symbol = this.getSymbolOfTsNode(node.parent);
101107
if (symbol === null || symbol.tsSymbol === null ||
102-
symbol.tsSymbol.declarations.length === 0) {
108+
symbol.tsSymbol.valueDeclaration === undefined ||
109+
!ts.isClassDeclaration(symbol.tsSymbol.valueDeclaration)) {
103110
return null;
104111
}
105-
const meta = this.getDirectiveMeta(element, symbol.tsSymbol.declarations[0]);
112+
const meta = this.getDirectiveMeta(element, symbol.tsSymbol.valueDeclaration);
106113
if (meta === null) {
107114
return null;
108115
}
109116

117+
const ngModule = this.getDirectiveModule(symbol.tsSymbol.valueDeclaration);
110118
const selector = meta.selector ?? null;
111119
const isComponent = meta.isComponent ?? null;
112120
const directiveSymbol: DirectiveSymbol = {
113121
...symbol,
114122
tsSymbol: symbol.tsSymbol,
115123
selector,
116124
isComponent,
125+
ngModule,
117126
kind: SymbolKind.Directive
118127
};
119128
return directiveSymbol;
@@ -132,6 +141,14 @@ export class SymbolBuilder {
132141
return directives.find(m => m.ref.node === directiveDeclaration) ?? null;
133142
}
134143

144+
private getDirectiveModule(declaration: ts.ClassDeclaration): ClassDeclaration|null {
145+
const scope = this.componentScopeReader.getScopeForComponent(declaration as ClassDeclaration);
146+
if (scope === null || scope === 'error') {
147+
return null;
148+
}
149+
return scope.ngModule;
150+
}
151+
135152
private getSymbolOfBoundEvent(eventBinding: TmplAstBoundEvent): OutputBindingSymbol|null {
136153
// Outputs are a `ts.CallExpression` that look like one of the two:
137154
// * _outputHelper(_t1["outputField"]).subscribe(handler);
@@ -239,17 +256,21 @@ export class SymbolBuilder {
239256
}
240257

241258
const symbol = this.getSymbolOfTsNode(declaration);
242-
if (symbol === null || symbol.tsSymbol === null) {
259+
if (symbol === null || symbol.tsSymbol === null ||
260+
symbol.tsSymbol.valueDeclaration === undefined ||
261+
!ts.isClassDeclaration(symbol.tsSymbol.valueDeclaration)) {
243262
return null;
244263
}
245264

265+
const ngModule = this.getDirectiveModule(symbol.tsSymbol.valueDeclaration);
246266
return {
247267
kind: SymbolKind.Directive,
248268
tsSymbol: symbol.tsSymbol,
249269
tsType: symbol.tsType,
250270
shimLocation: symbol.shimLocation,
251271
isComponent,
252272
selector,
273+
ngModule,
253274
};
254275
}
255276

packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ ts_library(
1818
"//packages/compiler-cli/src/ngtsc/incremental",
1919
"//packages/compiler-cli/src/ngtsc/metadata",
2020
"//packages/compiler-cli/src/ngtsc/reflection",
21+
"//packages/compiler-cli/src/ngtsc/scope",
2122
"//packages/compiler-cli/src/ngtsc/shims",
2223
"//packages/compiler-cli/src/ngtsc/testing",
2324
"//packages/compiler-cli/src/ngtsc/typecheck",

packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ import * as ts from 'typescript';
1111

1212
import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError, LogicalFileSystem} from '../../file_system';
1313
import {TestFile} from '../../file_system/testing';
14-
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
14+
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reexport, Reference, ReferenceEmitter} from '../../imports';
1515
import {NOOP_INCREMENTAL_BUILD} from '../../incremental';
1616
import {ClassPropertyMapping} from '../../metadata';
1717
import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
18+
import {ComponentScopeReader, ScopeData} from '../../scope';
1819
import {makeProgram} from '../../testing';
1920
import {getRootDirs} from '../../util/src/typescript';
2021
import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckContext} from '../api';
@@ -404,9 +405,32 @@ export function setup(targets: TypeCheckingTarget[], overrides: {
404405
(programStrategy as any).supportsInlineOperations = overrides.inlining;
405406
}
406407

408+
const fakeScopeReader = {
409+
getRequiresRemoteScope() {
410+
return null;
411+
},
412+
// If there is a module with [className] + 'Module' in the same source file, returns
413+
// `LocalModuleScope` with the ngModule class and empty arrays for everything else.
414+
getScopeForComponent(clazz: ClassDeclaration) {
415+
try {
416+
const ngModule = getClass(clazz.getSourceFile(), `${clazz.name.getText()}Module`);
417+
const stubScopeData = {directives: [], ngModules: [], pipes: []};
418+
return {
419+
ngModule,
420+
compilation: stubScopeData,
421+
reexports: [],
422+
schemas: [],
423+
exported: stubScopeData
424+
};
425+
} catch (e) {
426+
return null;
427+
}
428+
}
429+
};
430+
407431
const templateTypeChecker = new TemplateTypeCheckerImpl(
408432
program, programStrategy, checkAdapter, fullConfig, emitter, reflectionHost, host,
409-
NOOP_INCREMENTAL_BUILD);
433+
NOOP_INCREMENTAL_BUILD, fakeScopeReader);
410434
return {
411435
templateTypeChecker,
412436
program,

packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,7 +1304,11 @@ runInEachFileSystem(() => {
13041304
fileName: dirFile,
13051305
source: `
13061306
export class TestDir {}
1307+
// Allow the fake ComponentScopeReader to return a module for TestDir
1308+
export class TestDirModule {}
13071309
export class TestDir2 {}
1310+
// Allow the fake ComponentScopeReader to return a module for TestDir2
1311+
export class TestDir2Module {}
13081312
export class TestDirAllDivs {}
13091313
`,
13101314
templates: {},
@@ -1327,6 +1331,15 @@ runInEachFileSystem(() => {
13271331
const expectedSelectors = ['[dir]', '[dir2]', 'div'].sort();
13281332
const actualSelectors = symbol.directives.map(dir => dir.selector).sort();
13291333
expect(actualSelectors).toEqual(expectedSelectors);
1334+
1335+
// Testing this fully requires an integration test with a real `NgCompiler` (like in the
1336+
// Language Service, which uses the ngModule name for quick info). However, this path does
1337+
// assert that we are able to handle when the scope reader returns `null` or a class from
1338+
// the fake implementation.
1339+
const expectedModules = new Set([null, 'TestDirModule', 'TestDir2Module']);
1340+
const actualModules =
1341+
new Set(symbol.directives.map(dir => dir.ngModule?.name.getText() ?? null));
1342+
expect(actualModules).toEqual(expectedModules);
13301343
});
13311344
});
13321345

0 commit comments

Comments
 (0)