From 1c39ad38d3afbca3b5489d4b5874bc5acd554181 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Tue, 18 Dec 2018 11:09:21 -0800 Subject: [PATCH] feat(ivy): reference external classes by their exported name (#27743) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, ngtsc would assume that a given directive/pipe being imported from an external package was importable using the same name by which it was declared. This isn't always true; sometimes a package will export a directive under a different name. For example, Angular frequently prefixes directive names with the 'ɵ' character to indicate that they're part of the package's private API, and not for public consumption. This commit introduces the TsReferenceResolver class which, given a declaration to import and a module name to import it from, can determine the exported name of the declared class within the module. This allows ngtsc to pick the correct name by which to import the class instead of making assumptions about how it was exported. This resolver is used to select a correct symbol name when creating an AbsoluteReference. FW-517 #resolve FW-536 #resolve PR Close #27743 --- .../ngcc/src/analysis/decoration_analyzer.ts | 6 +- .../test/analysis/references_registry_spec.ts | 11 +- .../ngtsc/annotations/src/selector_scope.ts | 61 +++++---- .../ngtsc/annotations/test/component_spec.ts | 15 +-- .../annotations/test/selector_scope_spec.ts | 18 +-- .../src/ngtsc/imports/BUILD.bazel | 1 + .../compiler-cli/src/ngtsc/imports/index.ts | 1 + .../src/ngtsc/imports/src/resolver.ts | 116 ++++++++++++++++++ .../ngtsc/partial_evaluator/src/interface.ts | 9 +- .../partial_evaluator/src/interpreter.ts | 85 +++++++------ .../partial_evaluator/test/evaluator_spec.ts | 17 +-- packages/compiler-cli/src/ngtsc/program.ts | 7 +- .../src/ngtsc/util/src/typescript.ts | 10 ++ .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 103 +++++++++++----- 14 files changed, 332 insertions(+), 128 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/imports/src/resolver.ts diff --git a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts index c24f698fc9..f24b0a7bbc 100644 --- a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import {ConstantPool} from '@angular/compiler'; +import {TsReferenceResolver} from '@angular/compiler-cli/src/ngtsc/imports'; import {PartialEvaluator} from '@angular/compiler-cli/src/ngtsc/partial_evaluator'; import * as path from 'canonical-path'; import * as fs from 'fs'; @@ -59,8 +60,9 @@ export class FileResourceLoader implements ResourceLoader { */ export class DecorationAnalyzer { resourceLoader = new FileResourceLoader(); - scopeRegistry = new SelectorScopeRegistry(this.typeChecker, this.reflectionHost); - evaluator = new PartialEvaluator(this.reflectionHost, this.typeChecker); + resolver = new TsReferenceResolver(this.program, this.typeChecker, this.options, this.host); + scopeRegistry = new SelectorScopeRegistry(this.typeChecker, this.reflectionHost, this.resolver); + evaluator = new PartialEvaluator(this.reflectionHost, this.typeChecker, this.resolver); handlers: DecoratorHandler[] = [ new BaseDefDecoratorHandler(this.reflectionHost, this.evaluator), new ComponentDecoratorHandler( diff --git a/packages/compiler-cli/src/ngcc/test/analysis/references_registry_spec.ts b/packages/compiler-cli/src/ngcc/test/analysis/references_registry_spec.ts index 46183d93ce..230ed7f8c5 100644 --- a/packages/compiler-cli/src/ngcc/test/analysis/references_registry_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/analysis/references_registry_spec.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {Reference} from '../../../ngtsc/imports'; +import {Reference, TsReferenceResolver} from '../../../ngtsc/imports'; import {PartialEvaluator} from '../../../ngtsc/partial_evaluator'; import {TypeScriptReflectionHost} from '../../../ngtsc/reflection'; import {getDeclaration, makeProgram} from '../../../ngtsc/testing/in_memory_typescript'; @@ -16,7 +16,7 @@ import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registr describe('NgccReferencesRegistry', () => { it('should return a mapping from resolved reference identifiers to their declarations', () => { - const {program} = makeProgram([{ + const {program, options, host} = makeProgram([{ name: 'index.ts', contents: ` export class SomeClass {} @@ -38,9 +38,10 @@ describe('NgccReferencesRegistry', () => { getDeclaration(program, 'index.ts', 'someVariable', ts.isVariableDeclaration); const testArrayExpression = testArrayDeclaration.initializer !; - const host = new TypeScriptReflectionHost(checker); - const evaluator = new PartialEvaluator(host, checker); - const registry = new NgccReferencesRegistry(host); + const reflectionHost = new TypeScriptReflectionHost(checker); + const resolver = new TsReferenceResolver(program, checker, options, host); + const evaluator = new PartialEvaluator(reflectionHost, checker, resolver); + const registry = new NgccReferencesRegistry(reflectionHost); const references = evaluator.evaluate(testArrayExpression) as Reference[]; registry.add(null !, ...references); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/selector_scope.ts b/packages/compiler-cli/src/ngtsc/annotations/src/selector_scope.ts index 8fc435000a..928b499461 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/selector_scope.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/selector_scope.ts @@ -9,7 +9,7 @@ import {Expression, WrappedNodeExpr} from '@angular/compiler'; import * as ts from 'typescript'; -import {AbsoluteReference, Reference, ResolvedReference} from '../../imports'; +import {AbsoluteReference, Reference, ReferenceResolver, ResolvedReference} from '../../imports'; import {ReflectionHost, reflectIdentifierOfDeclaration, reflectNameOfDeclaration, reflectTypeEntityToDeclaration} from '../../reflection'; import {TypeCheckableDirectiveMeta} from '../../typecheck'; @@ -89,7 +89,9 @@ export class SelectorScopeRegistry { */ private _declararedTypeToModule = new Map(); - constructor(private checker: ts.TypeChecker, private reflector: ReflectionHost) {} + constructor( + private checker: ts.TypeChecker, private reflector: ReflectionHost, + private resolver: ReferenceResolver) {} /** * Register a module's metadata with the registry. @@ -161,7 +163,9 @@ export class SelectorScopeRegistry { // Process the declaration scope of the module, and lookup the selector of every declared type. // The initial value of ngModuleImportedFrom is 'null' which signifies that the NgModule // was not imported from a .d.ts source. - for (const ref of this.lookupScopesOrDie(module !, /* ngModuleImportedFrom */ null) + for (const ref of this + .lookupScopesOrDie( + module !, /* ngModuleImportedFrom */ null, node.getSourceFile().fileName) .compilation) { const node = ts.getOriginalNode(ref.node) as ts.Declaration; @@ -203,9 +207,10 @@ export class SelectorScopeRegistry { return scope !== null ? convertScopeToExpressions(scope, node) : null; } - private lookupScopesOrDie(node: ts.Declaration, ngModuleImportedFrom: string|null): - SelectorScopes { - const result = this.lookupScopes(node, ngModuleImportedFrom); + private lookupScopesOrDie( + node: ts.Declaration, ngModuleImportedFrom: string|null, + resolutionContext: string): SelectorScopes { + const result = this.lookupScopes(node, ngModuleImportedFrom, resolutionContext); if (result === null) { throw new Error(`Module not found: ${reflectNameOfDeclaration(node)}`); } @@ -219,8 +224,9 @@ export class SelectorScopeRegistry { * (`ngModuleImportedFrom`) then all of its declarations are exported at that same path, as well * as imports and exports from other modules that are relatively imported. */ - private lookupScopes(node: ts.Declaration, ngModuleImportedFrom: string|null): SelectorScopes - |null { + private lookupScopes( + node: ts.Declaration, ngModuleImportedFrom: string|null, + resolutionContext: string): SelectorScopes|null { let data: ModuleData|null = null; // Either this module was analyzed directly, or has a precompiled ngModuleDef. @@ -230,7 +236,7 @@ export class SelectorScopeRegistry { } else { // The module wasn't analyzed before, and probably has a precompiled ngModuleDef with a type // annotation that specifies the needed metadata. - data = this._readModuleDataFromCompiledClass(node, ngModuleImportedFrom); + data = this._readModuleDataFromCompiledClass(node, ngModuleImportedFrom, resolutionContext); // Note that data here could still be null, if the class didn't have a precompiled // ngModuleDef. } @@ -239,22 +245,28 @@ export class SelectorScopeRegistry { return null; } + const context = node.getSourceFile().fileName; + return { compilation: [ ...data.declarations, // Expand imports to the exported scope of those imports. ...flatten(data.imports.map( - ref => this.lookupScopesOrDie(ref.node as ts.Declaration, absoluteModuleName(ref)) - .exported)), + ref => + this.lookupScopesOrDie(ref.node as ts.Declaration, absoluteModuleName(ref), context) + .exported)), // And include the compilation scope of exported modules. ...flatten( data.exports - .map(ref => this.lookupScopes(ref.node as ts.Declaration, absoluteModuleName(ref))) + .map( + ref => this.lookupScopes( + ref.node as ts.Declaration, absoluteModuleName(ref), context)) .filter((scope: SelectorScopes | null): scope is SelectorScopes => scope !== null) .map(scope => scope.exported)) ], exported: flatten(data.exports.map(ref => { - const scope = this.lookupScopes(ref.node as ts.Declaration, absoluteModuleName(ref)); + const scope = + this.lookupScopes(ref.node as ts.Declaration, absoluteModuleName(ref), context); if (scope !== null) { return scope.exported; } else { @@ -297,7 +309,8 @@ export class SelectorScopeRegistry { * stemming from this module. */ private _readModuleDataFromCompiledClass( - clazz: ts.Declaration, ngModuleImportedFrom: string|null): ModuleData|null { + clazz: ts.Declaration, ngModuleImportedFrom: string|null, + resolutionContext: string): ModuleData|null { // This operation is explicitly not memoized, as it depends on `ngModuleImportedFrom`. // TODO(alxhub): investigate caching of .d.ts module metadata. const ngModuleDef = this.reflector.getMembersOfClass(clazz).find( @@ -315,9 +328,12 @@ export class SelectorScopeRegistry { // Read the ModuleData out of the type arguments. const [_, declarationMetadata, importMetadata, exportMetadata] = ngModuleDef.type.typeArguments; return { - declarations: this._extractReferencesFromType(declarationMetadata, ngModuleImportedFrom), - exports: this._extractReferencesFromType(exportMetadata, ngModuleImportedFrom), - imports: this._extractReferencesFromType(importMetadata, ngModuleImportedFrom), + declarations: this._extractReferencesFromType( + declarationMetadata, ngModuleImportedFrom, resolutionContext), + exports: + this._extractReferencesFromType(exportMetadata, ngModuleImportedFrom, resolutionContext), + imports: + this._extractReferencesFromType(importMetadata, ngModuleImportedFrom, resolutionContext), }; } @@ -389,8 +405,9 @@ export class SelectorScopeRegistry { * This operation assumes that these types should be imported from `ngModuleImportedFrom` unless * they themselves were imported from another absolute path. */ - private _extractReferencesFromType(def: ts.TypeNode, ngModuleImportedFrom: string|null): - Reference[] { + private _extractReferencesFromType( + def: ts.TypeNode, ngModuleImportedFrom: string|null, + resolutionContext: string): Reference[] { if (!ts.isTupleTypeNode(def)) { return []; } @@ -402,12 +419,10 @@ export class SelectorScopeRegistry { if (ngModuleImportedFrom !== null) { const {node, from} = reflectTypeEntityToDeclaration(type, this.checker); const moduleName = (from !== null && !from.startsWith('.') ? from : ngModuleImportedFrom); - const id = reflectIdentifierOfDeclaration(node); - return new AbsoluteReference(node, id !, moduleName, id !.text); + return this.resolver.resolve(node, moduleName, resolutionContext); } else { const {node} = reflectTypeEntityToDeclaration(type, this.checker); - const id = reflectIdentifierOfDeclaration(node); - return new ResolvedReference(node, id !); + return this.resolver.resolve(node, null, resolutionContext); } }); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts index cad3096519..05a702c433 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts @@ -9,9 +9,9 @@ import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; +import {TsReferenceResolver} from '../../imports'; import {PartialEvaluator} from '../../partial_evaluator'; import {TypeScriptReflectionHost} from '../../reflection'; - import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; import {ResourceLoader} from '../src/api'; import {ComponentDecoratorHandler} from '../src/component'; @@ -23,7 +23,7 @@ export class NoopResourceLoader implements ResourceLoader { describe('ComponentDecoratorHandler', () => { it('should produce a diagnostic when @Component has non-literal argument', () => { - const {program} = makeProgram([ + const {program, options, host} = makeProgram([ { name: 'node_modules/@angular/core/index.d.ts', contents: 'export const Component: any;', @@ -39,13 +39,14 @@ describe('ComponentDecoratorHandler', () => { }, ]); const checker = program.getTypeChecker(); - const host = new TypeScriptReflectionHost(checker); - const evaluator = new PartialEvaluator(host, checker); + const reflectionHost = new TypeScriptReflectionHost(checker); + const resolver = new TsReferenceResolver(program, checker, options, host); + const evaluator = new PartialEvaluator(reflectionHost, checker, resolver); const handler = new ComponentDecoratorHandler( - host, evaluator, new SelectorScopeRegistry(checker, host), false, new NoopResourceLoader(), - [''], false, true); + reflectionHost, evaluator, new SelectorScopeRegistry(checker, reflectionHost, resolver), + false, new NoopResourceLoader(), [''], false, true); const TestCmp = getDeclaration(program, 'entry.ts', 'TestCmp', ts.isClassDeclaration); - const detected = handler.detect(TestCmp, host.getDecoratorsOfDeclaration(TestCmp)); + const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp)); if (detected === undefined) { return fail('Failed to recognize @Component'); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/selector_scope_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/selector_scope_spec.ts index c3228f21f7..56785a28c8 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/selector_scope_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/selector_scope_spec.ts @@ -8,15 +8,14 @@ import * as ts from 'typescript'; -import {AbsoluteReference, ResolvedReference} from '../../imports'; +import {AbsoluteReference, ResolvedReference, TsReferenceResolver} from '../../imports'; import {TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; - import {SelectorScopeRegistry} from '../src/selector_scope'; describe('SelectorScopeRegistry', () => { it('absolute imports work', () => { - const {program} = makeProgram([ + const {program, options, host} = makeProgram([ { name: 'node_modules/@angular/core/index.d.ts', contents: ` @@ -29,6 +28,7 @@ describe('SelectorScopeRegistry', () => { contents: ` import {NgModuleDef} from '@angular/core'; import * as i0 from './component'; + export {SomeCmp} from './component'; export declare class SomeModule { static ngModuleDef: NgModuleDef; @@ -54,7 +54,7 @@ describe('SelectorScopeRegistry', () => { }, ]); const checker = program.getTypeChecker(); - const host = new TypeScriptReflectionHost(checker); + const reflectionHost = new TypeScriptReflectionHost(checker); const ProgramModule = getDeclaration(program, 'entry.ts', 'ProgramModule', ts.isClassDeclaration); const ProgramCmp = getDeclaration(program, 'entry.ts', 'ProgramCmp', ts.isClassDeclaration); @@ -65,7 +65,8 @@ describe('SelectorScopeRegistry', () => { const ProgramCmpRef = new ResolvedReference(ProgramCmp, ProgramCmp.name !); - const registry = new SelectorScopeRegistry(checker, host); + const resolver = new TsReferenceResolver(program, checker, options, host); + const registry = new SelectorScopeRegistry(checker, reflectionHost, resolver); registry.registerModule(ProgramModule, { declarations: [new ResolvedReference(ProgramCmp, ProgramCmp.name !)], @@ -95,7 +96,7 @@ describe('SelectorScopeRegistry', () => { }); it('exports of third-party libs work', () => { - const {program} = makeProgram([ + const {program, options, host} = makeProgram([ { name: 'node_modules/@angular/core/index.d.ts', contents: ` @@ -126,7 +127,7 @@ describe('SelectorScopeRegistry', () => { }, ]); const checker = program.getTypeChecker(); - const host = new TypeScriptReflectionHost(checker); + const reflectionHost = new TypeScriptReflectionHost(checker); const ProgramModule = getDeclaration(program, 'entry.ts', 'ProgramModule', ts.isClassDeclaration); const ProgramCmp = getDeclaration(program, 'entry.ts', 'ProgramCmp', ts.isClassDeclaration); @@ -137,7 +138,8 @@ describe('SelectorScopeRegistry', () => { const ProgramCmpRef = new ResolvedReference(ProgramCmp, ProgramCmp.name !); - const registry = new SelectorScopeRegistry(checker, host); + const resolver = new TsReferenceResolver(program, checker, options, host); + const registry = new SelectorScopeRegistry(checker, reflectionHost, resolver); registry.registerModule(ProgramModule, { declarations: [new ResolvedReference(ProgramCmp, ProgramCmp.name !)], diff --git a/packages/compiler-cli/src/ngtsc/imports/BUILD.bazel b/packages/compiler-cli/src/ngtsc/imports/BUILD.bazel index 816a1f4f9f..16e733b198 100644 --- a/packages/compiler-cli/src/ngtsc/imports/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/imports/BUILD.bazel @@ -12,6 +12,7 @@ ts_library( deps = [ "//packages:types", "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/util", "@ngdeps//@types/node", "@ngdeps//typescript", ], diff --git a/packages/compiler-cli/src/ngtsc/imports/index.ts b/packages/compiler-cli/src/ngtsc/imports/index.ts index 94db07ea1a..e7c07af3a6 100644 --- a/packages/compiler-cli/src/ngtsc/imports/index.ts +++ b/packages/compiler-cli/src/ngtsc/imports/index.ts @@ -7,3 +7,4 @@ */ export {AbsoluteReference, ImportMode, NodeReference, Reference, ResolvedReference} from './src/references'; +export {ReferenceResolver, TsReferenceResolver} from './src/resolver'; diff --git a/packages/compiler-cli/src/ngtsc/imports/src/resolver.ts b/packages/compiler-cli/src/ngtsc/imports/src/resolver.ts new file mode 100644 index 0000000000..a4ec2bfecb --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/imports/src/resolver.ts @@ -0,0 +1,116 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as ts from 'typescript'; + +import {isFromDtsFile} from '../../util/src/typescript'; + +import {AbsoluteReference, Reference, ResolvedReference} from './references'; + +export interface ReferenceResolver { + resolve(decl: ts.Declaration, importFromHint: string|null, fromFile: string): + Reference; +} + +export class TsReferenceResolver implements ReferenceResolver { + private moduleExportsCache = new Map|null>(); + + constructor( + private program: ts.Program, private checker: ts.TypeChecker, + private options: ts.CompilerOptions, private host: ts.CompilerHost) {} + + resolve(decl: ts.Declaration, importFromHint: string|null, fromFile: string): + Reference { + const id = identifierOfDeclaration(decl); + if (id === undefined) { + throw new Error(`Internal error: don't know how to refer to ${ts.SyntaxKind[decl.kind]}`); + } + + if (!isFromDtsFile(decl) || importFromHint === null) { + return new ResolvedReference(decl, id); + } else { + const publicName = this.resolveImportName(importFromHint, decl, fromFile); + if (publicName !== null) { + return new AbsoluteReference(decl, id, importFromHint, publicName); + } else { + throw new Error(`Internal error: Symbol ${id.text} is not exported from ${importFromHint}`); + } + } + } + + private resolveImportName(moduleName: string, target: ts.Declaration, fromFile: string): string + |null { + const exports = this.getExportsOfModule(moduleName, fromFile); + if (exports !== null && exports.has(target)) { + return exports.get(target) !; + } else { + return null; + } + } + + private getExportsOfModule(moduleName: string, fromFile: string): + Map|null { + if (!this.moduleExportsCache.has(moduleName)) { + this.moduleExportsCache.set(moduleName, this.enumerateExportsOfModule(moduleName, fromFile)); + } + return this.moduleExportsCache.get(moduleName) !; + } + + private enumerateExportsOfModule(moduleName: string, fromFile: string): + Map|null { + const resolved = ts.resolveModuleName(moduleName, fromFile, this.options, this.host); + if (resolved.resolvedModule === undefined) { + return null; + } + + const indexFile = this.program.getSourceFile(resolved.resolvedModule.resolvedFileName); + if (indexFile === undefined) { + return null; + } + + const indexSymbol = this.checker.getSymbolAtLocation(indexFile); + if (indexSymbol === undefined) { + return null; + } + + const exportMap = new Map(); + + const exports = this.checker.getExportsOfModule(indexSymbol); + for (const expSymbol of exports) { + const declSymbol = expSymbol.flags & ts.SymbolFlags.Alias ? + this.checker.getAliasedSymbol(expSymbol) : + expSymbol; + const decl = declSymbol.valueDeclaration; + if (decl === undefined) { + continue; + } + + if (declSymbol.name === expSymbol.name || !exportMap.has(decl)) { + exportMap.set(decl, expSymbol.name); + } + } + + return exportMap; + } +} + +function identifierOfDeclaration(decl: ts.Declaration): ts.Identifier|undefined { + if (ts.isClassDeclaration(decl)) { + return decl.name; + } else if (ts.isEnumDeclaration(decl)) { + return decl.name; + } else if (ts.isFunctionDeclaration(decl)) { + return decl.name; + } else if (ts.isVariableDeclaration(decl) && ts.isIdentifier(decl.name)) { + return decl.name; + } else if (ts.isShorthandPropertyAssignment(decl)) { + return decl.name; + } else { + return undefined; + } +} diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interface.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interface.ts index 695f536765..957172d789 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interface.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interface.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {Reference} from '../../imports'; +import {Reference, ReferenceResolver} from '../../imports'; import {ReflectionHost} from '../../reflection'; import {StaticInterpreter} from './interpreter'; @@ -19,12 +19,15 @@ export type ForeignFunctionResolver = ts.Expression | null; export class PartialEvaluator { - constructor(private host: ReflectionHost, private checker: ts.TypeChecker) {} + constructor( + private host: ReflectionHost, private checker: ts.TypeChecker, + private refResolver: ReferenceResolver) {} evaluate(expr: ts.Expression, foreignFunctionResolver?: ForeignFunctionResolver): ResolvedValue { - const interpreter = new StaticInterpreter(this.host, this.checker); + const interpreter = new StaticInterpreter(this.host, this.checker, this.refResolver); return interpreter.visit(expr, { absoluteModuleName: null, + resolutionContext: expr.getSourceFile().fileName, scope: new Map(), foreignFunctionResolver, }); } diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts index 4fb0fca483..70769e80fb 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts @@ -8,8 +8,8 @@ import * as ts from 'typescript'; -import {AbsoluteReference, NodeReference, Reference, ResolvedReference} from '../../imports'; -import {ReflectionHost} from '../../reflection'; +import {AbsoluteReference, NodeReference, Reference, ReferenceResolver, ResolvedReference} from '../../imports'; +import {Declaration, ReflectionHost} from '../../reflection'; import {ArraySliceBuiltinFn} from './builtin'; import {BuiltinFn, DYNAMIC_VALUE, EnumValue, ResolvedValue, ResolvedValueArray, ResolvedValueMap, isDynamicValue} from './result'; @@ -61,7 +61,16 @@ const UNARY_OPERATORS = new Map any>([ ]); interface Context { + /** + * The module name (if any) which was used to reach the currently resolving symbols. + */ absoluteModuleName: string|null; + + /** + * A file name representing the context in which the current `absoluteModuleName`, if any, was + * resolved. + */ + resolutionContext: string; scope: Scope; foreignFunctionResolver? (ref: Reference, @@ -69,7 +78,9 @@ interface Context { } export class StaticInterpreter { - constructor(private host: ReflectionHost, private checker: ts.TypeChecker) {} + constructor( + private host: ReflectionHost, private checker: ts.TypeChecker, + private refResolver: ReferenceResolver) {} visit(node: ts.Expression, context: Context): ResolvedValue { return this.visitExpression(node, context); @@ -203,8 +214,8 @@ export class StaticInterpreter { if (decl === null) { return DYNAMIC_VALUE; } - const result = this.visitDeclaration( - decl.node, {...context, absoluteModuleName: decl.viaModule || context.absoluteModuleName}); + const result = + this.visitDeclaration(decl.node, {...context, ...joinModuleContext(context, node, decl)}); if (result instanceof Reference) { result.addIdentifier(node); } @@ -292,10 +303,10 @@ export class StaticInterpreter { } const map = new Map(); declarations.forEach((decl, name) => { - const value = this.visitDeclaration(decl.node, { - ...context, - absoluteModuleName: decl.viaModule || context.absoluteModuleName, - }); + const value = this.visitDeclaration( + decl.node, { + ...context, ...joinModuleContext(context, node, decl), + }); map.set(name, value); }); return map; @@ -381,12 +392,16 @@ export class StaticInterpreter { // If the function is declared in a different file, resolve the foreign function expression // using the absolute module name of that file (if any). - let absoluteModuleName: string|null = context.absoluteModuleName; - if (lhs instanceof NodeReference || lhs instanceof AbsoluteReference) { - absoluteModuleName = lhs.moduleName || absoluteModuleName; + if ((lhs instanceof NodeReference || lhs instanceof AbsoluteReference) && + lhs.moduleName !== null) { + context = { + ...context, + absoluteModuleName: lhs.moduleName, + resolutionContext: node.getSourceFile().fileName, + }; } - return this.visitExpression(expr, {...context, absoluteModuleName}); + return this.visitExpression(expr, context); } const body = fn.body; @@ -473,17 +488,7 @@ export class StaticInterpreter { } private getReference(node: ts.Declaration, context: Context): Reference { - const id = identifierOfDeclaration(node); - if (id === undefined) { - throw new Error(`Don't know how to refer to ${ts.SyntaxKind[node.kind]}`); - } - if (context.absoluteModuleName !== null) { - // TODO(alxhub): investigate whether this can get symbol names wrong in the event of - // re-exports under different names. - return new AbsoluteReference(node, id, context.absoluteModuleName, id.text); - } else { - return new ResolvedReference(node, id); - } + return this.refResolver.resolve(node, context.absoluteModuleName, context.resolutionContext); } } @@ -504,22 +509,6 @@ function literal(value: ResolvedValue): any { throw new Error(`Value ${value} is not literal and cannot be used in this context.`); } -function identifierOfDeclaration(decl: ts.Declaration): ts.Identifier|undefined { - if (ts.isClassDeclaration(decl)) { - return decl.name; - } else if (ts.isEnumDeclaration(decl)) { - return decl.name; - } else if (ts.isFunctionDeclaration(decl)) { - return decl.name; - } else if (ts.isVariableDeclaration(decl) && ts.isIdentifier(decl.name)) { - return decl.name; - } else if (ts.isShorthandPropertyAssignment(decl)) { - return decl.name; - } else { - return undefined; - } -} - function isVariableDeclarationDeclared(node: ts.VariableDeclaration): boolean { if (node.parent === undefined || !ts.isVariableDeclarationList(node.parent)) { return false; @@ -532,3 +521,19 @@ function isVariableDeclarationDeclared(node: ts.VariableDeclaration): boolean { return varStmt.modifiers !== undefined && varStmt.modifiers.some(mod => mod.kind === ts.SyntaxKind.DeclareKeyword); } + +const EMPTY = {}; + +function joinModuleContext(existing: Context, node: ts.Node, decl: Declaration): { + absoluteModuleName?: string, + resolutionContext?: string, +} { + if (decl.viaModule !== null && decl.viaModule !== existing.absoluteModuleName) { + return { + absoluteModuleName: decl.viaModule, + resolutionContext: node.getSourceFile().fileName, + }; + } else { + return EMPTY; + } +} \ No newline at end of file diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts index 0e380e8cef..e45057aba9 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts @@ -9,7 +9,7 @@ import {WrappedNodeExpr} from '@angular/compiler'; import * as ts from 'typescript'; -import {AbsoluteReference, Reference} from '../../imports'; +import {AbsoluteReference, Reference, TsReferenceResolver} from '../../imports'; import {TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; import {PartialEvaluator} from '../src/interface'; @@ -42,9 +42,10 @@ function makeExpression( function evaluate( code: string, expr: string, supportingFiles: {name: string, contents: string}[] = []): T { - const {expression, checker} = makeExpression(code, expr, supportingFiles); + const {expression, checker, program, options, host} = makeExpression(code, expr, supportingFiles); const reflectionHost = new TypeScriptReflectionHost(checker); - const evaluator = new PartialEvaluator(reflectionHost, checker); + const resolver = new TsReferenceResolver(program, checker, options, host); + const evaluator = new PartialEvaluator(reflectionHost, checker, resolver); return evaluator.evaluate(expression) as T; } @@ -135,7 +136,7 @@ describe('ngtsc metadata', () => { }); it('imports work', () => { - const {program} = makeProgram([ + const {program, options, host} = makeProgram([ {name: 'second.ts', contents: 'export function foo(bar) { return bar; }'}, { name: 'entry.ts', @@ -149,7 +150,8 @@ describe('ngtsc metadata', () => { const reflectionHost = new TypeScriptReflectionHost(checker); const result = getDeclaration(program, 'entry.ts', 'target$', ts.isVariableDeclaration); const expr = result.initializer !; - const evaluator = new PartialEvaluator(reflectionHost, checker); + const resolver = new TsReferenceResolver(program, checker, options, host); + const evaluator = new PartialEvaluator(reflectionHost, checker, resolver); const resolved = evaluator.evaluate(expr); if (!(resolved instanceof Reference)) { return fail('Expected expression to resolve to a reference'); @@ -167,7 +169,7 @@ describe('ngtsc metadata', () => { }); it('absolute imports work', () => { - const {program} = makeProgram([ + const {program, options, host} = makeProgram([ {name: 'node_modules/some_library/index.d.ts', contents: 'export declare function foo(bar);'}, { name: 'entry.ts', @@ -181,7 +183,8 @@ describe('ngtsc metadata', () => { const reflectionHost = new TypeScriptReflectionHost(checker); const result = getDeclaration(program, 'entry.ts', 'target$', ts.isVariableDeclaration); const expr = result.initializer !; - const evaluator = new PartialEvaluator(reflectionHost, checker); + const resolver = new TsReferenceResolver(program, checker, options, host); + const evaluator = new PartialEvaluator(reflectionHost, checker, resolver); const resolved = evaluator.evaluate(expr); if (!(resolved instanceof AbsoluteReference)) { return fail('Expected expression to resolve to an absolute reference'); diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index ae54925907..a1b213d535 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -16,7 +16,7 @@ import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecorato import {BaseDefDecoratorHandler} from './annotations/src/base_def'; import {ErrorCode, ngErrorCode} from './diagnostics'; import {FlatIndexGenerator, ReferenceGraph, checkForPrivateExports, findFlatIndexEntryPoint} from './entry_point'; -import {Reference} from './imports'; +import {Reference, TsReferenceResolver} from './imports'; import {PartialEvaluator} from './partial_evaluator'; import {TypeScriptReflectionHost} from './reflection'; import {FileResourceLoader, HostResourceLoader} from './resource_loader'; @@ -274,8 +274,9 @@ export class NgtscProgram implements api.Program { private makeCompilation(): IvyCompilation { const checker = this.tsProgram.getTypeChecker(); - const evaluator = new PartialEvaluator(this.reflector, checker); - const scopeRegistry = new SelectorScopeRegistry(checker, this.reflector); + const refResolver = new TsReferenceResolver(this.tsProgram, checker, this.options, this.host); + const evaluator = new PartialEvaluator(this.reflector, checker, refResolver); + const scopeRegistry = new SelectorScopeRegistry(checker, this.reflector, refResolver); // If a flat module entrypoint was specified, then track references via a `ReferenceGraph` in // order to produce proper diagnostics for incorrectly exported directives/pipes/etc. If there diff --git a/packages/compiler-cli/src/ngtsc/util/src/typescript.ts b/packages/compiler-cli/src/ngtsc/util/src/typescript.ts index e557d36ae1..e0816ef3b1 100644 --- a/packages/compiler-cli/src/ngtsc/util/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/util/src/typescript.ts @@ -9,6 +9,8 @@ const TS = /\.tsx?$/i; const D_TS = /\.d\.ts$/i; +import * as ts from 'typescript'; + export function isDtsPath(filePath: string): boolean { return D_TS.test(filePath); } @@ -16,3 +18,11 @@ export function isDtsPath(filePath: string): boolean { export function isNonDeclarationTsPath(filePath: string): boolean { return TS.test(filePath) && !D_TS.test(filePath); } + +export function isFromDtsFile(node: ts.Node): boolean { + let sf: ts.SourceFile|undefined = node.getSourceFile(); + if (sf === undefined) { + sf = ts.getOriginalNode(node).getSourceFile(); + } + return sf !== undefined && D_TS.test(sf.fileName); +} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 8fe06755e3..30ff6066b0 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -420,10 +420,12 @@ describe('ngtsc behavioral tests', () => { env.write('node_modules/router/index.d.ts', ` import {ModuleWithProviders} from '@angular/core'; import * as internal from './internal'; + export {InternalRouterModule} from './internal'; - declare class RouterModule { + declare export class RouterModule { static forRoot(): ModuleWithProviders; } + `); env.write('node_modules/router/internal.d.ts', ` @@ -1195,40 +1197,81 @@ describe('ngtsc behavioral tests', () => { // Success is enough to indicate that this passes. }); - it('should not emit multiple references to the same directive', () => { - env.tsconfig(); - env.write('node_modules/external/index.d.ts', ` - import {ɵDirectiveDefWithMeta, ɵNgModuleDefWithMeta} from '@angular/core'; + describe('when processing external directives', () => { + it('should not emit multiple references to the same directive', () => { + env.tsconfig(); + env.write('node_modules/external/index.d.ts', ` + import {ɵDirectiveDefWithMeta, ɵNgModuleDefWithMeta} from '@angular/core'; + + export declare class ExternalDir { + static ngDirectiveDef: ɵDirectiveDefWithMeta; + } + + export declare class ExternalModule { + static ngModuleDef: ɵNgModuleDefWithMeta; + } + `); + env.write('test.ts', ` + import {Component, Directive, NgModule} from '@angular/core'; + import {ExternalModule} from 'external'; + + @Component({ + template: '
', + }) + class Cmp {} + + @NgModule({ + declarations: [Cmp], + // Multiple imports of the same module used to result in duplicate directive references + // in the output. + imports: [ExternalModule, ExternalModule], + }) + class Module {} + `); - export declare class ExternalDir { - static ngDirectiveDef: ɵDirectiveDefWithMeta; - } + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents).toMatch(/directives: \[i1\.ExternalDir\]/); + }); - export declare class ExternalModule { - static ngModuleDef: ɵNgModuleDefWithMeta; - } - `); - env.write('test.ts', ` - import {Component, Directive, NgModule} from '@angular/core'; - import {ExternalModule} from 'external'; + it('should import directives by their external name', () => { + env.tsconfig(); + env.write('node_modules/external/index.d.ts', ` + import {ɵDirectiveDefWithMeta, ɵNgModuleDefWithMeta} from '@angular/core'; + import {InternalDir} from './internal'; - @Component({ - template: '
', - }) - class Cmp {} + export {InternalDir as ExternalDir} from './internal'; - @NgModule({ - declarations: [Cmp], - // Multiple imports of the same module used to result in duplicate directive references - // in the output. - imports: [ExternalModule, ExternalModule], - }) - class Module {} - `); + export declare class ExternalModule { + static ngModuleDef: ɵNgModuleDefWithMeta; + } + `); + env.write('node_modules/external/internal.d.ts', ` - env.driveMain(); - const jsContents = env.getContents('test.js'); - expect(jsContents).toMatch(/directives: \[i1\.ExternalDir\]/); + export declare class InternalDir { + static ngDirectiveDef: ɵDirectiveDefWithMeta; + } + `); + env.write('test.ts', ` + import {Component, Directive, NgModule} from '@angular/core'; + import {ExternalModule} from 'external'; + + @Component({ + template: '
', + }) + class Cmp {} + + @NgModule({ + declarations: [Cmp], + imports: [ExternalModule], + }) + class Module {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents).toMatch(/directives: \[i1\.ExternalDir\]/); + }); }); describe('flat module indices', () => {