refactor(compiler): Add ngModule to directive symbol (#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 #39099
This commit is contained in:
Andrew Scott 2020-10-02 12:12:52 -07:00 committed by Joey Perrott
parent 57f8dd2978
commit e10b3e22ac
8 changed files with 79 additions and 11 deletions

View File

@ -21,7 +21,7 @@ import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, Inj
import {ModuleWithProvidersScanner} from '../../modulewithproviders';
import {PartialEvaluator} from '../../partial_evaluator';
import {NOOP_PERF_RECORDER, PerfRecorder} from '../../perf';
import {TypeScriptReflectionHost} from '../../reflection';
import {ClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {AdapterResourceLoader} from '../../resource';
import {entryPointKeyFor, NgModuleRouteAnalyzer} from '../../routing';
import {ComponentScopeReader, LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope';
@ -780,7 +780,8 @@ export class NgCompiler {
const templateTypeChecker = new TemplateTypeCheckerImpl(
this.tsProgram, this.typeCheckingProgramStrategy, traitCompiler,
this.getTypeCheckingConfig(), refEmitter, reflector, this.adapter, this.incrementalDriver);
this.getTypeCheckingConfig(), refEmitter, reflector, this.adapter, this.incrementalDriver,
scopeRegistry);
return {
isCore,

View File

@ -16,6 +16,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/incremental:api",
"//packages/compiler-cli/src/ngtsc/metadata",
"//packages/compiler-cli/src/ngtsc/reflection",
"//packages/compiler-cli/src/ngtsc/scope",
"//packages/compiler-cli/src/ngtsc/shims",
"//packages/compiler-cli/src/ngtsc/shims:api",
"//packages/compiler-cli/src/ngtsc/translator",

View File

@ -10,6 +10,7 @@ import {TmplAstElement, TmplAstReference, TmplAstTemplate, TmplAstVariable} from
import * as ts from 'typescript';
import {AbsoluteFsPath} from '../../file_system';
import {ClassDeclaration} from '../../reflection';
export enum SymbolKind {
Input,
@ -238,6 +239,9 @@ export interface DirectiveSymbol {
/** `true` if this `DirectiveSymbol` is for a @Component. */
isComponent: boolean;
/** The `NgModule` that this directive is declared in or `null` if it could not be determined. */
ngModule: ClassDeclaration|null;
}
/**

View File

@ -13,6 +13,7 @@ import {absoluteFromSourceFile, AbsoluteFsPath, getSourceFileOrError} from '../.
import {ReferenceEmitter} from '../../imports';
import {IncrementalBuild} from '../../incremental/api';
import {ReflectionHost} from '../../reflection';
import {ComponentScopeReader} from '../../scope';
import {isShim} from '../../shims';
import {getSourceFileOrNull} from '../../util/src/typescript';
import {OptimizeFor, ProgramTypeCheckAdapter, Symbol, TemplateId, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
@ -38,7 +39,8 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
private typeCheckAdapter: ProgramTypeCheckAdapter, private config: TypeCheckingConfig,
private refEmitter: ReferenceEmitter, private reflector: ReflectionHost,
private compilerHost: Pick<ts.CompilerHost, 'getCanonicalFileName'>,
private priorBuild: IncrementalBuild<unknown, FileTypeCheckingData>) {}
private priorBuild: IncrementalBuild<unknown, FileTypeCheckingData>,
private readonly componentScopeReader: ComponentScopeReader) {}
resetOverrides(): void {
for (const fileRecord of this.state.values()) {
@ -372,7 +374,8 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
return null;
}
return new SymbolBuilder(typeChecker, shimPath, tcb, data).getSymbol(node);
return new SymbolBuilder(typeChecker, shimPath, tcb, data, this.componentScopeReader)
.getSymbol(node);
}
}

View File

@ -10,6 +10,8 @@ import {AST, ASTWithSource, BindingPipe, MethodCall, PropertyWrite, SafeMethodCa
import * as ts from 'typescript';
import {AbsoluteFsPath} from '../../file_system';
import {ClassDeclaration} from '../../reflection';
import {ComponentScopeReader} from '../../scope';
import {isAssignment} from '../../util/src/typescript';
import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, ReferenceSymbol, Symbol, SymbolKind, TemplateSymbol, TsNodeSymbolInfo, TypeCheckableDirectiveMeta, VariableSymbol} from '../api';
@ -24,8 +26,12 @@ import {TcbDirectiveOutputsOp} from './type_check_block';
*/
export class SymbolBuilder {
constructor(
private readonly typeChecker: ts.TypeChecker, private readonly shimPath: AbsoluteFsPath,
private readonly typeCheckBlock: ts.Node, private readonly templateData: TemplateData) {}
private readonly typeChecker: ts.TypeChecker,
private readonly shimPath: AbsoluteFsPath,
private readonly typeCheckBlock: ts.Node,
private readonly templateData: TemplateData,
private readonly componentScopeReader: ComponentScopeReader,
) {}
getSymbol(node: TmplAstTemplate|TmplAstElement): TemplateSymbol|ElementSymbol|null;
getSymbol(node: TmplAstReference|TmplAstVariable): ReferenceSymbol|VariableSymbol|null;
@ -99,14 +105,16 @@ export class SymbolBuilder {
.map(node => {
const symbol = this.getSymbolOfTsNode(node.parent);
if (symbol === null || symbol.tsSymbol === null ||
symbol.tsSymbol.declarations.length === 0) {
symbol.tsSymbol.valueDeclaration === undefined ||
!ts.isClassDeclaration(symbol.tsSymbol.valueDeclaration)) {
return null;
}
const meta = this.getDirectiveMeta(element, symbol.tsSymbol.declarations[0]);
const meta = this.getDirectiveMeta(element, symbol.tsSymbol.valueDeclaration);
if (meta === null) {
return null;
}
const ngModule = this.getDirectiveModule(symbol.tsSymbol.valueDeclaration);
const selector = meta.selector ?? null;
const isComponent = meta.isComponent ?? null;
const directiveSymbol: DirectiveSymbol = {
@ -114,6 +122,7 @@ export class SymbolBuilder {
tsSymbol: symbol.tsSymbol,
selector,
isComponent,
ngModule,
kind: SymbolKind.Directive
};
return directiveSymbol;
@ -132,6 +141,14 @@ export class SymbolBuilder {
return directives.find(m => m.ref.node === directiveDeclaration) ?? null;
}
private getDirectiveModule(declaration: ts.ClassDeclaration): ClassDeclaration|null {
const scope = this.componentScopeReader.getScopeForComponent(declaration as ClassDeclaration);
if (scope === null || scope === 'error') {
return null;
}
return scope.ngModule;
}
private getSymbolOfBoundEvent(eventBinding: TmplAstBoundEvent): OutputBindingSymbol|null {
// Outputs are a `ts.CallExpression` that look like one of the two:
// * _outputHelper(_t1["outputField"]).subscribe(handler);
@ -239,10 +256,13 @@ export class SymbolBuilder {
}
const symbol = this.getSymbolOfTsNode(declaration);
if (symbol === null || symbol.tsSymbol === null) {
if (symbol === null || symbol.tsSymbol === null ||
symbol.tsSymbol.valueDeclaration === undefined ||
!ts.isClassDeclaration(symbol.tsSymbol.valueDeclaration)) {
return null;
}
const ngModule = this.getDirectiveModule(symbol.tsSymbol.valueDeclaration);
return {
kind: SymbolKind.Directive,
tsSymbol: symbol.tsSymbol,
@ -250,6 +270,7 @@ export class SymbolBuilder {
shimLocation: symbol.shimLocation,
isComponent,
selector,
ngModule,
};
}

View File

@ -18,6 +18,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/incremental",
"//packages/compiler-cli/src/ngtsc/metadata",
"//packages/compiler-cli/src/ngtsc/reflection",
"//packages/compiler-cli/src/ngtsc/scope",
"//packages/compiler-cli/src/ngtsc/shims",
"//packages/compiler-cli/src/ngtsc/testing",
"//packages/compiler-cli/src/ngtsc/typecheck",

View File

@ -11,10 +11,11 @@ import * as ts from 'typescript';
import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError, LogicalFileSystem} from '../../file_system';
import {TestFile} from '../../file_system/testing';
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reexport, Reference, ReferenceEmitter} from '../../imports';
import {NOOP_INCREMENTAL_BUILD} from '../../incremental';
import {ClassPropertyMapping} from '../../metadata';
import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {ComponentScopeReader, ScopeData} from '../../scope';
import {makeProgram} from '../../testing';
import {getRootDirs} from '../../util/src/typescript';
import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckContext} from '../api';
@ -404,9 +405,32 @@ export function setup(targets: TypeCheckingTarget[], overrides: {
(programStrategy as any).supportsInlineOperations = overrides.inlining;
}
const fakeScopeReader = {
getRequiresRemoteScope() {
return null;
},
// If there is a module with [className] + 'Module' in the same source file, returns
// `LocalModuleScope` with the ngModule class and empty arrays for everything else.
getScopeForComponent(clazz: ClassDeclaration) {
try {
const ngModule = getClass(clazz.getSourceFile(), `${clazz.name.getText()}Module`);
const stubScopeData = {directives: [], ngModules: [], pipes: []};
return {
ngModule,
compilation: stubScopeData,
reexports: [],
schemas: [],
exported: stubScopeData
};
} catch (e) {
return null;
}
}
};
const templateTypeChecker = new TemplateTypeCheckerImpl(
program, programStrategy, checkAdapter, fullConfig, emitter, reflectionHost, host,
NOOP_INCREMENTAL_BUILD);
NOOP_INCREMENTAL_BUILD, fakeScopeReader);
return {
templateTypeChecker,
program,

View File

@ -1304,7 +1304,11 @@ runInEachFileSystem(() => {
fileName: dirFile,
source: `
export class TestDir {}
// Allow the fake ComponentScopeReader to return a module for TestDir
export class TestDirModule {}
export class TestDir2 {}
// Allow the fake ComponentScopeReader to return a module for TestDir2
export class TestDir2Module {}
export class TestDirAllDivs {}
`,
templates: {},
@ -1327,6 +1331,15 @@ runInEachFileSystem(() => {
const expectedSelectors = ['[dir]', '[dir2]', 'div'].sort();
const actualSelectors = symbol.directives.map(dir => dir.selector).sort();
expect(actualSelectors).toEqual(expectedSelectors);
// Testing this fully requires an integration test with a real `NgCompiler` (like in the
// Language Service, which uses the ngModule name for quick info). However, this path does
// assert that we are able to handle when the scope reader returns `null` or a class from
// the fake implementation.
const expectedModules = new Set([null, 'TestDirModule', 'TestDir2Module']);
const actualModules =
new Set(symbol.directives.map(dir => dir.ngModule?.name.getText() ?? null));
expect(actualModules).toEqual(expectedModules);
});
});