Skip to content

Commit

Permalink
refactor(compiler-cli): implement DeclarationNode node type (#38959) (
Browse files Browse the repository at this point in the history
#39272)

Previously the `ConcreteDeclaration` and `InlineDeclaration` had
different properties for the underlying node type. And the `InlineDeclaration`
did not store a value that represented its declaration.

It turns out that a natural declaration node for an inline type is the
expression. For example in UMD/CommonJS this would be the `exports.<name>`
property access node.

So this expression is now used for the `node` of `InlineDeclaration` types
and the `expression` property is dropped.

To support this the codebase has been refactored to use a new `DeclarationNode`
type which is a union of `ts.Declaration|ts.Expression` instead of `ts.Declaration`
throughout.

PR Close #38959

PR Close #39272
  • Loading branch information
petebacondarwin authored and atscott committed Oct 14, 2020
1 parent 6acf117 commit 26988f0
Show file tree
Hide file tree
Showing 53 changed files with 279 additions and 254 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as ts from 'typescript';
import {ReferencesRegistry} from '../../../src/ngtsc/annotations';
import {Reference} from '../../../src/ngtsc/imports';
import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator';
import {ClassDeclaration, isNamedClassDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {ClassDeclaration, DeclarationNode, isNamedClassDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {NgccReflectionHost} from '../host/ngcc_host';
import {hasNameIdentifier, isDefined} from '../utils';

Expand All @@ -30,7 +30,7 @@ export interface ModuleWithProvidersInfo {
/**
* Declaration of the containing class (if this is a method)
*/
container: ts.Declaration|null;
container: DeclarationNode|null;
/**
* The declaration of the class that the `ngModule` property on the `ModuleWithProviders` object
* refers to.
Expand Down Expand Up @@ -125,7 +125,7 @@ export class ModuleWithProvidersAnalyzer {
*/
private parseForModuleWithProviders(
name: string, node: ts.Node|null, implementation: ts.Node|null = node,
container: ts.Declaration|null = null): ModuleWithProvidersInfo|null {
container: DeclarationNode|null = null): ModuleWithProvidersInfo|null {
if (implementation === null ||
(!ts.isFunctionDeclaration(implementation) && !ts.isMethodDeclaration(implementation) &&
!ts.isFunctionExpression(implementation))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import * as ts from 'typescript';
import {ReferencesRegistry} from '../../../src/ngtsc/annotations';
import {Reference} from '../../../src/ngtsc/imports';
import {ConcreteDeclaration, ReflectionHost} from '../../../src/ngtsc/reflection';
import {Declaration, DeclarationNode, ReflectionHost} from '../../../src/ngtsc/reflection';
import {hasNameIdentifier} from '../utils';

/**
Expand All @@ -20,7 +20,7 @@ import {hasNameIdentifier} from '../utils';
* from libraries that are compiled by ngcc.
*/
export class NgccReferencesRegistry implements ReferencesRegistry {
private map = new Map<ts.Identifier, ConcreteDeclaration>();
private map = new Map<ts.Identifier, Declaration>();

constructor(private host: ReflectionHost) {}

Expand All @@ -29,12 +29,12 @@ export class NgccReferencesRegistry implements ReferencesRegistry {
* Only `ResolveReference` references are stored. Other types are ignored.
* @param references A collection of references to register.
*/
add(source: ts.Declaration, ...references: Reference<ts.Declaration>[]): void {
add(source: DeclarationNode, ...references: Reference<DeclarationNode>[]): void {
references.forEach(ref => {
// Only store relative references. We are not interested in literals.
if (ref.bestGuessOwningModule === null && hasNameIdentifier(ref.node)) {
const declaration = this.host.getDeclarationOfIdentifier(ref.node.name);
if (declaration && declaration.node !== null && hasNameIdentifier(declaration.node)) {
if (declaration && hasNameIdentifier(declaration.node)) {
this.map.set(declaration.node.name, declaration);
}
}
Expand All @@ -45,7 +45,7 @@ export class NgccReferencesRegistry implements ReferencesRegistry {
* Create and return a mapping for the registered resolved references.
* @returns A map of reference identifiers to reference declarations.
*/
getDeclarationMap(): Map<ts.Identifier, ConcreteDeclaration> {
getDeclarationMap(): Map<ts.Identifier, Declaration> {
return this.map;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import * as ts from 'typescript';

import {absoluteFromSourceFile, AbsoluteFsPath} from '../../../src/ngtsc/file_system';
import {ConcreteDeclaration} from '../../../src/ngtsc/reflection';
import {Declaration} from '../../../src/ngtsc/reflection';
import {NgccReflectionHost} from '../host/ngcc_host';
import {hasNameIdentifier, isDefined} from '../utils';

Expand Down Expand Up @@ -40,8 +40,8 @@ export class PrivateDeclarationsAnalyzer {

private getPrivateDeclarations(
rootFiles: ts.SourceFile[],
declarations: Map<ts.Identifier, ConcreteDeclaration>): PrivateDeclarationsAnalyses {
const privateDeclarations: Map<ts.Identifier, ConcreteDeclaration> = new Map(declarations);
declarations: Map<ts.Identifier, Declaration>): PrivateDeclarationsAnalyses {
const privateDeclarations: Map<ts.Identifier, Declaration> = new Map(declarations);

rootFiles.forEach(f => {
const exports = this.host.getExportsOfModule(f);
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/ngcc/src/host/commonjs_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as ts from 'typescript';

import {absoluteFrom} from '../../../src/ngtsc/file_system';
import {Logger} from '../../../src/ngtsc/logging';
import {Declaration, Import} from '../../../src/ngtsc/reflection';
import {Declaration, DeclarationKind, Import} from '../../../src/ngtsc/reflection';
import {BundleProgram} from '../packages/bundle_program';
import {FactoryMap, isDefined} from '../utils';

Expand Down Expand Up @@ -181,7 +181,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
} else {
return {
name,
declaration: {node: null, known: null, expression, viaModule: null},
declaration: {node: expression, known: null, kind: DeclarationKind.Inline, viaModule: null},
};
}
}
Expand All @@ -204,7 +204,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
return null;
}
const viaModule = isExternalImport(importPath) ? importPath : null;
return {node: module, known: null, viaModule, identity: null};
return {node: module, known: null, viaModule, identity: null, kind: DeclarationKind.Concrete};
}

private resolveModuleName(moduleName: string, containingFile: ts.SourceFile): ts.SourceFile
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/ngcc/src/host/delegating_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import * as ts from 'typescript';

import {ClassDeclaration, ClassMember, CtorParameter, Declaration, Decorator, FunctionDefinition, Import, ReflectionHost} from '../../../src/ngtsc/reflection';
import {ClassDeclaration, ClassMember, CtorParameter, Declaration, DeclarationNode, Decorator, FunctionDefinition, Import, ReflectionHost} from '../../../src/ngtsc/reflection';
import {isFromDtsFile} from '../../../src/ngtsc/util/src/typescript';

import {NgccClassSymbol, NgccReflectionHost, SwitchableVariableDeclaration} from './ngcc_host';
Expand Down Expand Up @@ -38,7 +38,7 @@ export class DelegatingReflectionHost implements NgccReflectionHost {
return this.ngccHost.getDeclarationOfIdentifier(id);
}

getDecoratorsOfDeclaration(declaration: ts.Declaration): Decorator[]|null {
getDecoratorsOfDeclaration(declaration: DeclarationNode): Decorator[]|null {
if (isFromDtsFile(declaration)) {
return this.tsHost.getDecoratorsOfDeclaration(declaration);
}
Expand All @@ -52,7 +52,7 @@ export class DelegatingReflectionHost implements NgccReflectionHost {
return this.ngccHost.getDefinitionOfFunction(fn);
}

getDtsDeclaration(declaration: ts.Declaration): ts.Declaration|null {
getDtsDeclaration(declaration: DeclarationNode): ts.Declaration|null {
if (isFromDtsFile(declaration)) {
return this.tsHost.getDtsDeclaration(declaration);
}
Expand Down
64 changes: 32 additions & 32 deletions packages/compiler-cli/ngcc/src/host/esm2015_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as ts from 'typescript';

import {absoluteFromSourceFile} from '../../../src/ngtsc/file_system';
import {Logger} from '../../../src/ngtsc/logging';
import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, EnumMember, Import, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference, TypeValueReferenceKind, ValueUnavailableKind} from '../../../src/ngtsc/reflection';
import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, DeclarationNode, Decorator, EnumMember, Import, isConcreteDeclaration, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference, TypeValueReferenceKind, ValueUnavailableKind} from '../../../src/ngtsc/reflection';
import {isWithinPackage} from '../analysis/util';
import {BundleProgram} from '../packages/bundle_program';
import {findAll, getNameText, hasNameIdentifier, isDefined, stripDollarSuffix} from '../utils';
Expand Down Expand Up @@ -58,15 +58,15 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
* tree. Note that by definition the key and value declarations will not be in the same TS
* program.
*/
protected publicDtsDeclarationMap: Map<ts.Declaration, ts.Declaration>|null = null;
protected publicDtsDeclarationMap: Map<DeclarationNode, ts.Declaration>|null = null;
/**
* A mapping from source declarations to typings declarations, which are not publicly exported.
*
* This mapping is a best guess between declarations that happen to be exported from their file by
* the same name in both the source and the dts file. Note that by definition the key and value
* declarations will not be in the same TS program.
*/
protected privateDtsDeclarationMap: Map<ts.Declaration, ts.Declaration>|null = null;
protected privateDtsDeclarationMap: Map<DeclarationNode, ts.Declaration>|null = null;

/**
* The set of source files that have already been preprocessed.
Expand All @@ -88,7 +88,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
*
* This map is populated during the preprocessing of each source file.
*/
protected aliasedClassDeclarations = new Map<ts.Declaration, ts.Identifier>();
protected aliasedClassDeclarations = new Map<DeclarationNode, ts.Identifier>();

/**
* Caches the information of the decorators on a class, as the work involved with extracting
Expand Down Expand Up @@ -140,16 +140,17 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
* Examine a declaration (for example, of a class or function) and return metadata about any
* decorators present on the declaration.
*
* @param declaration a TypeScript `ts.Declaration` node representing the class or function over
* which to reflect. For example, if the intent is to reflect the decorators of a class and the
* source is in ES6 format, this will be a `ts.ClassDeclaration` node. If the source is in ES5
* format, this might be a `ts.VariableDeclaration` as classes in ES5 are represented as the
* result of an IIFE execution.
* @param declaration a TypeScript node representing the class or function over which to reflect.
* For example, if the intent is to reflect the decorators of a class and the source is in ES6
* format, this will be a `ts.ClassDeclaration` node. If the source is in ES5 format, this
* might be a `ts.VariableDeclaration` as classes in ES5 are represented as the result of an
* IIFE execution.
*
* @returns an array of `Decorator` metadata if decorators are present on the declaration, or
* `null` if either no decorators were present or if the declaration is not of a decoratable type.
* `null` if either no decorators were present or if the declaration is not of a decoratable
* type.
*/
getDecoratorsOfDeclaration(declaration: ts.Declaration): Decorator[]|null {
getDecoratorsOfDeclaration(declaration: DeclarationNode): Decorator[]|null {
const symbol = this.getClassSymbol(declaration);
if (!symbol) {
return null;
Expand Down Expand Up @@ -280,13 +281,14 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null {
const superDeclaration = super.getDeclarationOfIdentifier(id);

// If no declaration was found or it's an inline declaration, return as is.
if (superDeclaration === null || superDeclaration.node === null) {
// If no declaration was found, return.
if (superDeclaration === null) {
return superDeclaration;
}

// If the declaration already has traits assigned to it, return as is.
if (superDeclaration.known !== null || superDeclaration.identity !== null) {
if (superDeclaration.known !== null ||
isConcreteDeclaration(superDeclaration) && superDeclaration.identity !== null) {
return superDeclaration;
}

Expand All @@ -302,7 +304,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
const declaration = outerNode !== null && isNamedVariableDeclaration(outerNode) ?
this.getDeclarationOfIdentifier(outerNode.name) :
superDeclaration;
if (declaration === null || declaration.node === null || declaration.known !== null) {
if (declaration === null || declaration.known !== null ||
isConcreteDeclaration(declaration) && declaration.identity !== null) {
return declaration;
}

Expand All @@ -314,7 +317,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
}

// Variable declarations may represent an enum declaration, so attempt to resolve its members.
if (ts.isVariableDeclaration(declaration.node)) {
if (isConcreteDeclaration(declaration) && ts.isVariableDeclaration(declaration.node)) {
const enumMembers = this.resolveEnumMembers(declaration.node);
if (enumMembers !== null) {
declaration.identity = {kind: SpecialDeclarationKind.DownleveledEnum, enumMembers};
Expand Down Expand Up @@ -450,7 +453,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
* Note that the `ts.ClassDeclaration` returned from this function may not be from the same
* `ts.Program` as the input declaration.
*/
getDtsDeclaration(declaration: ts.Declaration): ts.Declaration|null {
getDtsDeclaration(declaration: DeclarationNode): ts.Declaration|null {
if (this.dts === null) {
return null;
}
Expand Down Expand Up @@ -777,7 +780,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
* @returns The original identifier that the given class declaration resolves to, or `undefined`
* if the declaration does not represent an aliased class.
*/
protected resolveAliasedClassIdentifier(declaration: ts.Declaration): ts.Identifier|null {
protected resolveAliasedClassIdentifier(declaration: DeclarationNode): ts.Identifier|null {
this.ensurePreprocessed(declaration.getSourceFile());
return this.aliasedClassDeclarations.has(declaration) ?
this.aliasedClassDeclarations.get(declaration)! :
Expand Down Expand Up @@ -829,7 +832,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
const aliasedIdentifier = initializer.left;

const aliasedDeclaration = this.getDeclarationOfIdentifier(aliasedIdentifier);
if (aliasedDeclaration === null || aliasedDeclaration.node === null) {
if (aliasedDeclaration === null) {
throw new Error(
`Unable to locate declaration of ${aliasedIdentifier.text} in "${statement.getText()}"`);
}
Expand Down Expand Up @@ -1628,7 +1631,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N

const imp = this.getImportOfExpression(typeExpression);
const decl = this.getDeclarationOfExpression(typeExpression);
if (imp === null || decl === null || decl.node === null) {
if (imp === null || decl === null) {
return {
kind: TypeValueReferenceKind.LOCAL,
expression: typeExpression,
Expand Down Expand Up @@ -1788,8 +1791,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
* @returns a map of source declarations to typings declarations.
*/
protected computePublicDtsDeclarationMap(src: BundleProgram, dts: BundleProgram):
Map<ts.Declaration, ts.Declaration> {
const declarationMap = new Map<ts.Declaration, ts.Declaration>();
Map<DeclarationNode, ts.Declaration> {
const declarationMap = new Map<DeclarationNode, ts.Declaration>();
const dtsDeclarationMap = new Map<string, ts.Declaration>();
const rootDts = getRootFileOrFail(dts);
this.collectDtsExportedDeclarations(dtsDeclarationMap, rootDts, dts.program.getTypeChecker());
Expand All @@ -1812,8 +1815,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
* @returns a map of source declarations to typings declarations.
*/
protected computePrivateDtsDeclarationMap(src: BundleProgram, dts: BundleProgram):
Map<ts.Declaration, ts.Declaration> {
const declarationMap = new Map<ts.Declaration, ts.Declaration>();
Map<DeclarationNode, ts.Declaration> {
const declarationMap = new Map<DeclarationNode, ts.Declaration>();
const dtsDeclarationMap = new Map<string, ts.Declaration>();
const typeChecker = dts.program.getTypeChecker();

Expand Down Expand Up @@ -1855,13 +1858,13 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N


protected collectSrcExportedDeclarations(
declarationMap: Map<ts.Declaration, ts.Declaration>,
declarationMap: Map<DeclarationNode, ts.Declaration>,
dtsDeclarationMap: Map<string, ts.Declaration>, srcFile: ts.SourceFile): void {
const fileExports = this.getExportsOfModule(srcFile);
if (fileExports !== null) {
for (const [exportName, {node: declaration}] of fileExports) {
if (declaration !== null && dtsDeclarationMap.has(exportName)) {
declarationMap.set(declaration, dtsDeclarationMap.get(exportName)!);
for (const [exportName, {node: declarationNode}] of fileExports) {
if (dtsDeclarationMap.has(exportName)) {
declarationMap.set(declarationNode, dtsDeclarationMap.get(exportName)!);
}
}
}
Expand All @@ -1877,7 +1880,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
}

const namespaceDecl = this.getDeclarationOfIdentifier(expression.expression);
if (!namespaceDecl || namespaceDecl.node === null || !ts.isSourceFile(namespaceDecl.node)) {
if (!namespaceDecl || !ts.isSourceFile(namespaceDecl.node)) {
return null;
}

Expand All @@ -1896,9 +1899,6 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N

/** Checks if the specified declaration resolves to the known JavaScript global `Object`. */
protected isJavaScriptObjectDeclaration(decl: Declaration): boolean {
if (decl.node === null) {
return false;
}
const node = decl.node;
// The default TypeScript library types the global `Object` variable through
// a variable declaration with a type reference resolving to `ObjectConstructor`.
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/ngcc/src/host/esm5_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import * as ts from 'typescript';

import {ClassDeclaration, ClassMember, ClassMemberKind, Declaration, Decorator, FunctionDefinition, isNamedFunctionDeclaration, KnownDeclaration, Parameter, reflectObjectLiteral} from '../../../src/ngtsc/reflection';
import {ClassDeclaration, ClassMember, ClassMemberKind, Declaration, DeclarationKind, Decorator, FunctionDefinition, isNamedFunctionDeclaration, KnownDeclaration, Parameter, reflectObjectLiteral} from '../../../src/ngtsc/reflection';
import {getTsHelperFnFromDeclaration, getTsHelperFnFromIdentifier, hasNameIdentifier} from '../utils';

import {Esm2015ReflectionHost, getOuterNodeFromInnerDeclaration, getPropertyValueFromSymbol, isAssignmentStatement, ParamInfo} from './esm2015_host';
Expand Down Expand Up @@ -82,9 +82,9 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
// `importHelpers: false` (the default). This is, for example, the case with
// `@nativescript/[email protected]`.
return {
expression: id,
kind: DeclarationKind.Inline,
node: id,
known: nonEmittedNorImportedTsHelperDeclaration,
node: null,
viaModule: null,
};
}
Expand Down
Loading

0 comments on commit 26988f0

Please sign in to comment.