feat(ivy): produce diagnostics for missing exports, incorrect entrypoint (#27743)

This commit adds tracking of modules, directives, and pipes which are made
visible to consumers through NgModules exported from the package entrypoint.
ngtsc will now produce a diagnostic if such classes are not themselves
exported via the entrypoint (as this is a requirement for downstream
consumers to use them with Ivy).

To accomplish this, a graph of references is created and populated via the
ReferencesRegistry. Symbols exported via the package entrypoint are compared
against the graph to determine if any publicly visible symbols are not
properly exported. Diagnostics are produced for each one which also show the
path by which they become visible.

This commit also introduces a diagnostic (instead of a hard compiler crash)
if an entrypoint file cannot be correctly determined.

PR Close #27743
This commit is contained in:
Alex Rickabaugh 2018-12-13 11:52:20 -08:00 committed by Kara Erickson
parent ac157170c8
commit 0b9094ec63
22 changed files with 597 additions and 66 deletions

View File

@ -27,12 +27,14 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/annotations", "//packages/compiler-cli/src/ngtsc/annotations",
"//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/entry_point", "//packages/compiler-cli/src/ngtsc/entry_point",
"//packages/compiler-cli/src/ngtsc/imports",
"//packages/compiler-cli/src/ngtsc/partial_evaluator", "//packages/compiler-cli/src/ngtsc/partial_evaluator",
"//packages/compiler-cli/src/ngtsc/reflection", "//packages/compiler-cli/src/ngtsc/reflection",
"//packages/compiler-cli/src/ngtsc/shims", "//packages/compiler-cli/src/ngtsc/shims",
"//packages/compiler-cli/src/ngtsc/switch", "//packages/compiler-cli/src/ngtsc/switch",
"//packages/compiler-cli/src/ngtsc/transform", "//packages/compiler-cli/src/ngtsc/transform",
"//packages/compiler-cli/src/ngtsc/typecheck", "//packages/compiler-cli/src/ngtsc/typecheck",
"//packages/compiler-cli/src/ngtsc/util",
"@ngdeps//@bazel/typescript", "@ngdeps//@bazel/typescript",
"@ngdeps//@types", "@ngdeps//@types",
"@ngdeps//tsickle", "@ngdeps//tsickle",

View File

@ -62,7 +62,8 @@ export class ModuleWithProvidersAnalyzer {
`The referenced NgModule in ${fn.declaration.getText()} is not a class declaration in the typings program; instead we get ${dtsNgModule.getText()}`); `The referenced NgModule in ${fn.declaration.getText()} is not a class declaration in the typings program; instead we get ${dtsNgModule.getText()}`);
} }
// Record the usage of the internal module as it needs to become an exported symbol // Record the usage of the internal module as it needs to become an exported symbol
this.referencesRegistry.add(new ResolvedReference(ngModule.node, fn.ngModule)); this.referencesRegistry.add(
ngModule.node, new ResolvedReference(ngModule.node, fn.ngModule));
ngModule = {node: dtsNgModule, viaModule: null}; ngModule = {node: dtsNgModule, viaModule: null};
} }

View File

@ -29,7 +29,7 @@ export class NgccReferencesRegistry implements ReferencesRegistry {
* Only `ResolveReference` references are stored. Other types are ignored. * Only `ResolveReference` references are stored. Other types are ignored.
* @param references A collection of references to register. * @param references A collection of references to register.
*/ */
add(...references: Reference<ts.Declaration>[]): void { add(source: ts.Declaration, ...references: Reference<ts.Declaration>[]): void {
references.forEach(ref => { references.forEach(ref => {
// Only store resolved references. We are not interested in literals. // Only store resolved references. We are not interested in literals.
if (ref instanceof ResolvedReference && hasNameIdentifier(ref.node)) { if (ref instanceof ResolvedReference && hasNameIdentifier(ref.node)) {

View File

@ -7,10 +7,10 @@
*/ */
import * as ts from 'typescript'; import * as ts from 'typescript';
import {ReferencesRegistry} from '../../../ngtsc/annotations';
import {Declaration} from '../../../ngtsc/reflection'; import {Declaration} from '../../../ngtsc/reflection';
import {NgccReflectionHost} from '../host/ngcc_host'; import {NgccReflectionHost} from '../host/ngcc_host';
import {hasNameIdentifier, isDefined} from '../utils'; import {hasNameIdentifier, isDefined} from '../utils';
import {NgccReferencesRegistry} from './ngcc_references_registry';
export interface ExportInfo { export interface ExportInfo {
identifier: string; identifier: string;
@ -24,7 +24,8 @@ export type PrivateDeclarationsAnalyses = ExportInfo[];
* (i.e. on an NgModule) that are not publicly exported via an entry-point. * (i.e. on an NgModule) that are not publicly exported via an entry-point.
*/ */
export class PrivateDeclarationsAnalyzer { export class PrivateDeclarationsAnalyzer {
constructor(private host: NgccReflectionHost, private referencesRegistry: ReferencesRegistry) {} constructor(
private host: NgccReflectionHost, private referencesRegistry: NgccReferencesRegistry) {}
analyzeProgram(program: ts.Program): PrivateDeclarationsAnalyses { analyzeProgram(program: ts.Program): PrivateDeclarationsAnalyses {
const rootFiles = this.getRootFiles(program); const rootFiles = this.getRootFiles(program);

View File

@ -137,15 +137,18 @@ describe('PrivateDeclarationsAnalyzer', () => {
const publicComponentDeclaration = const publicComponentDeclaration =
getDeclaration(program, '/src/a.js', 'PublicComponent', ts.isClassDeclaration); getDeclaration(program, '/src/a.js', 'PublicComponent', ts.isClassDeclaration);
referencesRegistry.add( referencesRegistry.add(
null !,
new ResolvedReference(publicComponentDeclaration, publicComponentDeclaration.name !)); new ResolvedReference(publicComponentDeclaration, publicComponentDeclaration.name !));
const privateComponentDeclaration = const privateComponentDeclaration =
getDeclaration(program, '/src/b.js', 'PrivateComponent', ts.isClassDeclaration); getDeclaration(program, '/src/b.js', 'PrivateComponent', ts.isClassDeclaration);
referencesRegistry.add(new ResolvedReference( referencesRegistry.add(
privateComponentDeclaration, privateComponentDeclaration.name !)); null !, new ResolvedReference(
privateComponentDeclaration, privateComponentDeclaration.name !));
const internalComponentDeclaration = const internalComponentDeclaration =
getDeclaration(program, '/src/c.js', 'InternalComponent', ts.isClassDeclaration); getDeclaration(program, '/src/c.js', 'InternalComponent', ts.isClassDeclaration);
referencesRegistry.add(new ResolvedReference( referencesRegistry.add(
internalComponentDeclaration, internalComponentDeclaration.name !)); null !, new ResolvedReference(
internalComponentDeclaration, internalComponentDeclaration.name !));
const analyses = analyzer.analyzeProgram(program); const analyses = analyzer.analyzeProgram(program);
expect(analyses.length).toEqual(2); expect(analyses.length).toEqual(2);

View File

@ -43,7 +43,7 @@ describe('NgccReferencesRegistry', () => {
const registry = new NgccReferencesRegistry(host); const registry = new NgccReferencesRegistry(host);
const references = evaluator.evaluate(testArrayExpression) as Reference<ts.Declaration>[]; const references = evaluator.evaluate(testArrayExpression) as Reference<ts.Declaration>[];
registry.add(...references); registry.add(null !, ...references);
const map = registry.getDeclarationMap(); const map = registry.getDeclarationMap();
expect(map.size).toEqual(2); expect(map.size).toEqual(2);

View File

@ -109,7 +109,8 @@ export interface LazyRoute {
export interface Program { export interface Program {
getTsProgram(): ts.Program; getTsProgram(): ts.Program;
getTsOptionDiagnostics(cancellationToken?: ts.CancellationToken): ReadonlyArray<ts.Diagnostic>; getTsOptionDiagnostics(cancellationToken?: ts.CancellationToken): ReadonlyArray<ts.Diagnostic>;
getNgOptionDiagnostics(cancellationToken?: ts.CancellationToken): ReadonlyArray<Diagnostic>; getNgOptionDiagnostics(cancellationToken?: ts.CancellationToken):
ReadonlyArray<ts.Diagnostic|Diagnostic>;
getTsSyntacticDiagnostics(sourceFile?: ts.SourceFile, cancellationToken?: ts.CancellationToken): getTsSyntacticDiagnostics(sourceFile?: ts.SourceFile, cancellationToken?: ts.CancellationToken):
ReadonlyArray<ts.Diagnostic>; ReadonlyArray<ts.Diagnostic>;
getNgStructuralDiagnostics(cancellationToken?: ts.CancellationToken): ReadonlyArray<Diagnostic>; getNgStructuralDiagnostics(cancellationToken?: ts.CancellationToken): ReadonlyArray<Diagnostic>;

View File

@ -75,7 +75,6 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
const expr = ngModule.get('declarations') !; const expr = ngModule.get('declarations') !;
const declarationMeta = this.evaluator.evaluate(expr); const declarationMeta = this.evaluator.evaluate(expr);
declarations = this.resolveTypeList(expr, declarationMeta, 'declarations'); declarations = this.resolveTypeList(expr, declarationMeta, 'declarations');
this.referencesRegistry.add(...declarations);
} }
let imports: Reference<ts.Declaration>[] = []; let imports: Reference<ts.Declaration>[] = [];
if (ngModule.has('imports')) { if (ngModule.has('imports')) {
@ -83,7 +82,6 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
const importsMeta = this.evaluator.evaluate( const importsMeta = this.evaluator.evaluate(
expr, ref => this._extractModuleFromModuleWithProvidersFn(ref.node)); expr, ref => this._extractModuleFromModuleWithProvidersFn(ref.node));
imports = this.resolveTypeList(expr, importsMeta, 'imports'); imports = this.resolveTypeList(expr, importsMeta, 'imports');
this.referencesRegistry.add(...imports);
} }
let exports: Reference<ts.Declaration>[] = []; let exports: Reference<ts.Declaration>[] = [];
if (ngModule.has('exports')) { if (ngModule.has('exports')) {
@ -91,14 +89,13 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
const exportsMeta = this.evaluator.evaluate( const exportsMeta = this.evaluator.evaluate(
expr, ref => this._extractModuleFromModuleWithProvidersFn(ref.node)); expr, ref => this._extractModuleFromModuleWithProvidersFn(ref.node));
exports = this.resolveTypeList(expr, exportsMeta, 'exports'); exports = this.resolveTypeList(expr, exportsMeta, 'exports');
this.referencesRegistry.add(...exports); this.referencesRegistry.add(node, ...exports);
} }
let bootstrap: Reference<ts.Declaration>[] = []; let bootstrap: Reference<ts.Declaration>[] = [];
if (ngModule.has('bootstrap')) { if (ngModule.has('bootstrap')) {
const expr = ngModule.get('bootstrap') !; const expr = ngModule.get('bootstrap') !;
const bootstrapMeta = this.evaluator.evaluate(expr); const bootstrapMeta = this.evaluator.evaluate(expr);
bootstrap = this.resolveTypeList(expr, bootstrapMeta, 'bootstrap'); bootstrap = this.resolveTypeList(expr, bootstrapMeta, 'bootstrap');
this.referencesRegistry.add(...bootstrap);
} }
// Register this module's information with the SelectorScopeRegistry. This ensures that during // Register this module's information with the SelectorScopeRegistry. This ensures that during

View File

@ -17,15 +17,9 @@ import {Declaration} from '../../reflection';
export interface ReferencesRegistry { export interface ReferencesRegistry {
/** /**
* Register one or more references in the registry. * Register one or more references in the registry.
* Only `ResolveReference` references are stored. Other types are ignored.
* @param references A collection of references to register. * @param references A collection of references to register.
*/ */
add(...references: Reference<ts.Declaration>[]): void; add(source: ts.Declaration, ...references: Reference<ts.Declaration>[]): void;
/**
* Create and return a mapping for the registered resolved references.
* @returns A map of reference identifiers to reference declarations.
*/
getDeclarationMap(): Map<ts.Identifier, Declaration>;
} }
/** /**
@ -34,6 +28,5 @@ export interface ReferencesRegistry {
* The ngcc tool implements a working version for its purposes. * The ngcc tool implements a working version for its purposes.
*/ */
export class NoopReferencesRegistry implements ReferencesRegistry { export class NoopReferencesRegistry implements ReferencesRegistry {
add(...references: Reference<ts.Declaration>[]): void {} add(source: ts.Declaration, ...references: Reference<ts.Declaration>[]): void {}
getDeclarationMap(): Map<ts.Identifier, Declaration> { return new Map(); }
} }

View File

@ -6,6 +6,6 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
export {ErrorCode} from './src/code'; export {ErrorCode, ngErrorCode} from './src/code';
export {FatalDiagnosticError, isFatalDiagnosticError} from './src/error'; export {FatalDiagnosticError, isFatalDiagnosticError} from './src/error';
export {replaceTsWithNgInErrors} from './src/util'; export {replaceTsWithNgInErrors} from './src/util';

View File

@ -19,4 +19,13 @@ export enum ErrorCode {
COMPONENT_MISSING_TEMPLATE = 2001, COMPONENT_MISSING_TEMPLATE = 2001,
PIPE_MISSING_NAME = 2002, PIPE_MISSING_NAME = 2002,
PARAM_MISSING_TOKEN = 2003, PARAM_MISSING_TOKEN = 2003,
SYMBOL_NOT_EXPORTED = 3001,
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,
CONFIG_FLAT_MODULE_NO_INDEX = 4001,
}
export function ngErrorCode(code: ErrorCode): number {
return parseInt('-99' + code);
} }

View File

@ -10,6 +10,7 @@ ts_library(
]), ]),
module_name = "@angular/compiler-cli/src/ngtsc/entry_point", module_name = "@angular/compiler-cli/src/ngtsc/entry_point",
deps = [ deps = [
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/shims", "//packages/compiler-cli/src/ngtsc/shims",
"//packages/compiler-cli/src/ngtsc/util", "//packages/compiler-cli/src/ngtsc/util",
"@ngdeps//@types/node", "@ngdeps//@types/node",

View File

@ -7,3 +7,6 @@
*/ */
export {FlatIndexGenerator} from './src/generator'; export {FlatIndexGenerator} from './src/generator';
export {findFlatIndexEntryPoint} from './src/logic';
export {checkForPrivateExports} from './src/private_export_checker';
export {ReferenceGraph} from './src/reference_graph';

View File

@ -12,43 +12,18 @@ import * as path from 'path';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {ShimGenerator} from '../../shims'; import {ShimGenerator} from '../../shims';
import {isNonDeclarationTsPath} from '../../util/src/typescript';
export class FlatIndexGenerator implements ShimGenerator { export class FlatIndexGenerator implements ShimGenerator {
readonly flatIndexPath: string; readonly flatIndexPath: string;
private constructor( constructor(
relativeFlatIndexPath: string, readonly entryPoint: string, readonly entryPoint: string, relativeFlatIndexPath: string,
readonly moduleName: string|null) { readonly moduleName: string|null) {
this.flatIndexPath = path.posix.join(path.posix.dirname(entryPoint), relativeFlatIndexPath) this.flatIndexPath = path.posix.join(path.posix.dirname(entryPoint), relativeFlatIndexPath)
.replace(/\.js$/, '') + .replace(/\.js$/, '') +
'.ts'; '.ts';
} }
static forRootFiles(flatIndexPath: string, files: ReadonlyArray<string>, moduleName: string|null):
FlatIndexGenerator|null {
// If there's only one .ts file in the program, it's the entry. Otherwise, look for the shortest
// (in terms of characters in the filename) file that ends in /index.ts. The second behavior is
// deprecated; users should always explicitly specify a single .ts entrypoint.
const tsFiles = files.filter(file => isNonDeclarationTsPath(file));
if (tsFiles.length === 1) {
return new FlatIndexGenerator(flatIndexPath, tsFiles[0], moduleName);
} else {
let indexFile: string|null = null;
for (const tsFile of tsFiles) {
if (tsFile.endsWith('/index.ts') &&
(indexFile === null || tsFile.length <= indexFile.length)) {
indexFile = tsFile;
}
}
if (indexFile !== null) {
return new FlatIndexGenerator(flatIndexPath, indexFile, moduleName);
} else {
return null;
}
}
}
recognize(fileName: string): boolean { return fileName === this.flatIndexPath; } recognize(fileName: string): boolean { return fileName === this.flatIndexPath; }
generate(): ts.SourceFile { generate(): ts.SourceFile {

View File

@ -0,0 +1,34 @@
/**
* @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 {isNonDeclarationTsPath} from '../../util/src/typescript';
export function findFlatIndexEntryPoint(rootFiles: ReadonlyArray<string>): string|null {
// There are two ways for a file to be recognized as the flat module index:
// 1) if it's the only file!!!!!!
// 2) (deprecated) if it's named 'index.ts' and has the shortest path of all such files.
const tsFiles = rootFiles.filter(file => isNonDeclarationTsPath(file));
if (tsFiles.length === 1) {
// There's only one file - this is the flat module index.
return tsFiles[0];
} else {
// In the event there's more than one TS file, one of them can still be selected as the
// flat module index if it's named 'index.ts'. If there's more than one 'index.ts', the one
// with the shortest path wins.
//
// This behavior is DEPRECATED and only exists to support existing usages.
let indexFile: string|null = null;
for (const tsFile of tsFiles) {
if (tsFile.endsWith('/index.ts') &&
(indexFile === null || tsFile.length <= indexFile.length)) {
indexFile = tsFile;
}
}
return indexFile;
}
}

View File

@ -0,0 +1,147 @@
/**
* @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 {ErrorCode, ngErrorCode} from '../../diagnostics';
import {ReferenceGraph} from './reference_graph';
/**
* Produce `ts.Diagnostic`s for classes that are visible from exported types (e.g. directives
* exposed by exported `NgModule`s) that are not themselves exported.
*
* This function reconciles two concepts:
*
* A class is Exported if it's exported from the main library `entryPoint` file.
* A class is Visible if, via Angular semantics, a downstream consumer can import an Exported class
* and be affected by the class in question. For example, an Exported NgModule may expose a
* directive class to its consumers. Consumers that import the NgModule may have the directive
* applied to elements in their templates. In this case, the directive is considered Visible.
*
* `checkForPrivateExports` attempts to verify that all Visible classes are Exported, and report
* `ts.Diagnostic`s for those that aren't.
*
* @param entryPoint `ts.SourceFile` of the library's entrypoint, which should export the library's
* public API.
* @param checker `ts.TypeChecker` for the current program.
* @param refGraph `ReferenceGraph` tracking the visibility of Angular types.
* @returns an array of `ts.Diagnostic`s representing errors when visible classes are not exported
* properly.
*/
export function checkForPrivateExports(
entryPoint: ts.SourceFile, checker: ts.TypeChecker, refGraph: ReferenceGraph): ts.Diagnostic[] {
const diagnostics: ts.Diagnostic[] = [];
// Firstly, compute the exports of the entry point. These are all the Exported classes.
const topLevelExports = new Set<ts.Declaration>();
// Do this via `ts.TypeChecker.getExportsOfModule`.
const moduleSymbol = checker.getSymbolAtLocation(entryPoint);
if (moduleSymbol === undefined) {
throw new Error(`Internal error: failed to get symbol for entrypoint`);
}
const exportedSymbols = checker.getExportsOfModule(moduleSymbol);
// Loop through the exported symbols, de-alias if needed, and add them to `topLevelExports`.
// TODO(alxhub): use proper iteration when build.sh is removed. (#27762)
exportedSymbols.forEach(symbol => {
if (symbol.flags & ts.SymbolFlags.Alias) {
symbol = checker.getAliasedSymbol(symbol);
}
const decl = symbol.valueDeclaration;
if (decl !== undefined) {
topLevelExports.add(decl);
}
});
// Next, go through each exported class and expand it to the set of classes it makes Visible,
// using the `ReferenceGraph`. For each Visible class, verify that it's also Exported, and queue
// an error if it isn't. `checkedSet` ensures only one error is queued per class.
const checkedSet = new Set<ts.Declaration>();
// Loop through each Exported class.
// TODO(alxhub): use proper iteration when the legacy build is removed. (#27762)
topLevelExports.forEach(mainExport => {
// Loop through each class made Visible by the Exported class.
refGraph.transitiveReferencesOf(mainExport).forEach(transitiveReference => {
// Skip classes which have already been checked.
if (checkedSet.has(transitiveReference)) {
return;
}
checkedSet.add(transitiveReference);
// Verify that the Visible class is also Exported.
if (!topLevelExports.has(transitiveReference)) {
// This is an error, `mainExport` makes `transitiveReference` Visible, but
// `transitiveReference` is not Exported from the entrypoint. Construct a diagnostic to
// give to the user explaining the situation.
const descriptor = getDescriptorOfDeclaration(transitiveReference);
const name = getNameOfDeclaration(transitiveReference);
// Construct the path of visibility, from `mainExport` to `transitiveReference`.
let visibleVia = 'NgModule exports';
const transitivePath = refGraph.pathFrom(mainExport, transitiveReference);
if (transitivePath !== null) {
visibleVia = transitivePath.map(seg => getNameOfDeclaration(seg)).join(' -> ');
}
const diagnostic: ts.Diagnostic = {
category: ts.DiagnosticCategory.Error,
code: ngErrorCode(ErrorCode.SYMBOL_NOT_EXPORTED),
file: transitiveReference.getSourceFile(), ...getPosOfDeclaration(transitiveReference),
messageText:
`Unsupported private ${descriptor} ${name}. This ${descriptor} is visible to consumers via ${visibleVia}, but is not exported from the top-level library entrypoint.`,
};
diagnostics.push(diagnostic);
}
});
});
return diagnostics;
}
function getPosOfDeclaration(decl: ts.Declaration): {start: number, length: number} {
const node: ts.Node = getIdentifierOfDeclaration(decl) || decl;
return {
start: node.getStart(),
length: node.getEnd() + 1 - node.getStart(),
};
}
function getIdentifierOfDeclaration(decl: ts.Declaration): ts.Identifier|null {
if ((ts.isClassDeclaration(decl) || ts.isVariableDeclaration(decl) ||
ts.isFunctionDeclaration(decl)) &&
decl.name !== undefined && ts.isIdentifier(decl.name)) {
return decl.name;
} else {
return null;
}
}
function getNameOfDeclaration(decl: ts.Declaration): string {
const id = getIdentifierOfDeclaration(decl);
return id !== null ? id.text : '(unnamed)';
}
function getDescriptorOfDeclaration(decl: ts.Declaration): string {
switch (decl.kind) {
case ts.SyntaxKind.ClassDeclaration:
return 'class';
case ts.SyntaxKind.FunctionDeclaration:
return 'function';
case ts.SyntaxKind.VariableDeclaration:
return 'variable';
case ts.SyntaxKind.EnumDeclaration:
return 'enum';
default:
return 'declaration';
}
}

View File

@ -0,0 +1,78 @@
/**
* @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';
export class ReferenceGraph<T = ts.Declaration> {
private references = new Map<T, Set<T>>();
add(from: T, to: T): void {
if (!this.references.has(from)) {
this.references.set(from, new Set());
}
this.references.get(from) !.add(to);
}
transitiveReferencesOf(target: T): Set<T> {
const set = new Set<T>();
this.collectTransitiveReferences(set, target);
return set;
}
pathFrom(source: T, target: T): T[]|null {
return this.collectPathFrom(source, target, new Set());
}
private collectPathFrom(source: T, target: T, seen: Set<T>): T[]|null {
if (source === target) {
// Looking for a path from the target to itself - that path is just the target. This is the
// "base case" of the search.
return [target];
} else if (seen.has(source)) {
// The search has already looked through this source before.
return null;
}
// Consider outgoing edges from `source`.
seen.add(source);
if (!this.references.has(source)) {
// There are no outgoing edges from `source`.
return null;
} else {
// Look through the outgoing edges of `source`.
// TODO(alxhub): use proper iteration when the legacy build is removed. (#27762)
let candidatePath: T[]|null = null;
this.references.get(source) !.forEach(edge => {
// Early exit if a path has already been found.
if (candidatePath !== null) {
return;
}
// Look for a path from this outgoing edge to `target`.
const partialPath = this.collectPathFrom(edge, target, seen);
if (partialPath !== null) {
// A path exists from `edge` to `target`. Insert `source` at the beginning.
candidatePath = [source, ...partialPath];
}
});
return candidatePath;
}
}
private collectTransitiveReferences(set: Set<T>, decl: T): void {
if (this.references.has(decl)) {
// TODO(alxhub): use proper iteration when the legacy build is removed. (#27762)
this.references.get(decl) !.forEach(ref => {
if (!set.has(ref)) {
set.add(ref);
this.collectTransitiveReferences(set, ref);
}
});
}
}
}

View File

@ -0,0 +1,25 @@
package(default_visibility = ["//visibility:public"])
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
ts_library(
name = "test_lib",
testonly = True,
srcs = glob([
"**/*.ts",
]),
deps = [
"//packages:types",
"//packages/compiler-cli/src/ngtsc/entry_point",
"@ngdeps//typescript",
],
)
jasmine_node_test(
name = "test",
bootstrap = ["angular/tools/testing/init_node_no_angular_spec.js"],
deps = [
":test_lib",
"//tools/testing:node_no_angular",
],
)

View File

@ -0,0 +1,50 @@
/**
* @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 {ReferenceGraph} from '../src/reference_graph';
describe('entry_point reference graph', () => {
let graph: ReferenceGraph<string>;
const refs =
(target: string) => { return Array.from(graph.transitiveReferencesOf(target)).sort(); };
beforeEach(() => {
graph = new ReferenceGraph();
graph.add('origin', 'alpha');
graph.add('alpha', 'beta');
graph.add('beta', 'gamma');
});
it('should track a simple chain of references', () => {
// origin -> alpha -> beta -> gamma
expect(refs('origin')).toEqual(['alpha', 'beta', 'gamma']);
expect(refs('beta')).toEqual(['gamma']);
});
it('should not crash on a cycle', () => {
// origin -> alpha -> beta -> gamma
// ^---------------/
graph.add('beta', 'origin');
expect(refs('origin')).toEqual(['alpha', 'beta', 'gamma', 'origin']);
});
it('should report a path between two nodes in the graph', () => {
// ,------------------------\
// origin -> alpha -> beta -> gamma -> delta
// \----------------^
graph.add('beta', 'delta');
graph.add('delta', 'alpha');
expect(graph.pathFrom('origin', 'gamma')).toEqual(['origin', 'alpha', 'beta', 'gamma']);
expect(graph.pathFrom('beta', 'alpha')).toEqual(['beta', 'delta', 'alpha']);
});
it('should not report a path that doesn\'t exist',
() => { expect(graph.pathFrom('gamma', 'beta')).toBeNull(); });
});

View File

@ -12,9 +12,11 @@ import * as ts from 'typescript';
import * as api from '../transformers/api'; import * as api from '../transformers/api';
import {nocollapseHack} from '../transformers/nocollapse_hack'; import {nocollapseHack} from '../transformers/nocollapse_hack';
import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, NoopReferencesRegistry, PipeDecoratorHandler, ResourceLoader, SelectorScopeRegistry} from './annotations'; import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, NoopReferencesRegistry, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader, SelectorScopeRegistry} from './annotations';
import {BaseDefDecoratorHandler} from './annotations/src/base_def'; import {BaseDefDecoratorHandler} from './annotations/src/base_def';
import {FlatIndexGenerator} from './entry_point'; import {ErrorCode, ngErrorCode} from './diagnostics';
import {FlatIndexGenerator, ReferenceGraph, checkForPrivateExports, findFlatIndexEntryPoint} from './entry_point';
import {Reference} from './imports';
import {PartialEvaluator} from './partial_evaluator'; import {PartialEvaluator} from './partial_evaluator';
import {TypeScriptReflectionHost} from './reflection'; import {TypeScriptReflectionHost} from './reflection';
import {FileResourceLoader, HostResourceLoader} from './resource_loader'; import {FileResourceLoader, HostResourceLoader} from './resource_loader';
@ -22,6 +24,7 @@ import {FactoryGenerator, FactoryInfo, GeneratedShimsHostWrapper, ShimGenerator,
import {ivySwitchTransform} from './switch'; import {ivySwitchTransform} from './switch';
import {IvyCompilation, ivyTransformFactory} from './transform'; import {IvyCompilation, ivyTransformFactory} from './transform';
import {TypeCheckContext, TypeCheckProgramHost} from './typecheck'; import {TypeCheckContext, TypeCheckProgramHost} from './typecheck';
import {isDtsPath} from './util/src/typescript';
export class NgtscProgram implements api.Program { export class NgtscProgram implements api.Program {
private tsProgram: ts.Program; private tsProgram: ts.Program;
@ -35,6 +38,11 @@ export class NgtscProgram implements api.Program {
private _isCore: boolean|undefined = undefined; private _isCore: boolean|undefined = undefined;
private rootDirs: string[]; private rootDirs: string[];
private closureCompilerEnabled: boolean; private closureCompilerEnabled: boolean;
private entryPoint: ts.SourceFile|null;
private exportReferenceGraph: ReferenceGraph|null = null;
private flatIndexGenerator: FlatIndexGenerator|null = null;
private constructionDiagnostics: ts.Diagnostic[] = [];
constructor( constructor(
@ -79,14 +87,10 @@ export class NgtscProgram implements api.Program {
generators.push(summaryGenerator, factoryGenerator); generators.push(summaryGenerator, factoryGenerator);
} }
let entryPoint: string|null = null;
if (options.flatModuleOutFile !== undefined) { if (options.flatModuleOutFile !== undefined) {
const flatModuleId = options.flatModuleId || null; entryPoint = findFlatIndexEntryPoint(rootNames);
const flatIndexGenerator = if (entryPoint === null) {
FlatIndexGenerator.forRootFiles(options.flatModuleOutFile, rootNames, flatModuleId);
if (flatIndexGenerator !== null) {
generators.push(flatIndexGenerator);
rootFiles.push(flatIndexGenerator.flatIndexPath);
} else {
// This error message talks specifically about having a single .ts file in "files". However // This error message talks specifically about having a single .ts file in "files". However
// the actual logic is a bit more permissive. If a single file exists, that will be taken, // the actual logic is a bit more permissive. If a single file exists, that will be taken,
// otherwise the highest level (shortest path) "index.ts" file will be used as the flat // otherwise the highest level (shortest path) "index.ts" file will be used as the flat
@ -95,8 +99,21 @@ export class NgtscProgram implements api.Program {
// //
// The user is not informed about the "index.ts" option as this behavior is deprecated - // The user is not informed about the "index.ts" option as this behavior is deprecated -
// an explicit entrypoint should always be specified. // an explicit entrypoint should always be specified.
throw new Error( this.constructionDiagnostics.push({
'Angular compiler option "flatModuleIndex" requires one and only one .ts file in the "files" field.'); category: ts.DiagnosticCategory.Error,
code: ngErrorCode(ErrorCode.CONFIG_FLAT_MODULE_NO_INDEX),
file: undefined,
start: undefined,
length: undefined,
messageText:
'Angular compiler option "flatModuleOutFile" requires one and only one .ts file in the "files" field.',
});
} else {
const flatModuleId = options.flatModuleId || null;
this.flatIndexGenerator =
new FlatIndexGenerator(entryPoint, options.flatModuleOutFile, flatModuleId);
generators.push(this.flatIndexGenerator);
rootFiles.push(this.flatIndexGenerator.flatIndexPath);
} }
} }
@ -106,6 +123,8 @@ export class NgtscProgram implements api.Program {
this.tsProgram = this.tsProgram =
ts.createProgram(rootFiles, options, this.host, oldProgram && oldProgram.getTsProgram()); ts.createProgram(rootFiles, options, this.host, oldProgram && oldProgram.getTsProgram());
this.entryPoint = entryPoint !== null ? this.tsProgram.getSourceFile(entryPoint) || null : null;
} }
getTsProgram(): ts.Program { return this.tsProgram; } getTsProgram(): ts.Program { return this.tsProgram; }
@ -116,8 +135,8 @@ export class NgtscProgram implements api.Program {
} }
getNgOptionDiagnostics(cancellationToken?: ts.CancellationToken| getNgOptionDiagnostics(cancellationToken?: ts.CancellationToken|
undefined): ReadonlyArray<api.Diagnostic> { undefined): ReadonlyArray<ts.Diagnostic|api.Diagnostic> {
return []; return this.constructionDiagnostics;
} }
getTsSyntacticDiagnostics( getTsSyntacticDiagnostics(
@ -147,6 +166,10 @@ export class NgtscProgram implements api.Program {
compilation.typeCheck(ctx); compilation.typeCheck(ctx);
diagnostics.push(...this.compileTypeCheckProgram(ctx)); diagnostics.push(...this.compileTypeCheckProgram(ctx));
} }
if (this.entryPoint !== null && this.exportReferenceGraph !== null) {
diagnostics.push(...checkForPrivateExports(
this.entryPoint, this.tsProgram.getTypeChecker(), this.exportReferenceGraph));
}
return diagnostics; return diagnostics;
} }
@ -253,7 +276,17 @@ export class NgtscProgram implements api.Program {
const checker = this.tsProgram.getTypeChecker(); const checker = this.tsProgram.getTypeChecker();
const evaluator = new PartialEvaluator(this.reflector, checker); const evaluator = new PartialEvaluator(this.reflector, checker);
const scopeRegistry = new SelectorScopeRegistry(checker, this.reflector); const scopeRegistry = new SelectorScopeRegistry(checker, this.reflector);
const referencesRegistry = new NoopReferencesRegistry();
// 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
// is no flat module entrypoint then don't pay the cost of tracking references.
let referencesRegistry: ReferencesRegistry;
if (this.entryPoint !== null) {
this.exportReferenceGraph = new ReferenceGraph();
referencesRegistry = new ReferenceGraphAdapter(this.exportReferenceGraph);
} else {
referencesRegistry = new NoopReferencesRegistry();
}
// Set up the IvyCompilation, which manages state for the Ivy transformer. // Set up the IvyCompilation, which manages state for the Ivy transformer.
const handlers = [ const handlers = [
@ -355,3 +388,21 @@ function isAngularCorePackage(program: ts.Program): boolean {
}); });
}); });
} }
export class ReferenceGraphAdapter implements ReferencesRegistry {
constructor(private graph: ReferenceGraph) {}
add(source: ts.Declaration, ...references: Reference<ts.Declaration>[]): void {
for (const {node} of references) {
let sourceFile = node.getSourceFile();
if (sourceFile === undefined) {
sourceFile = ts.getOriginalNode(node).getSourceFile();
}
// Only record local references (not references into .d.ts files).
if (sourceFile === undefined || !isDtsPath(sourceFile.fileName)) {
this.graph.add(source, node);
}
}
}
}

View File

@ -307,7 +307,8 @@ export interface Program {
/** /**
* Retrieve options diagnostics for the Angular options used to create the program. * Retrieve options diagnostics for the Angular options used to create the program.
*/ */
getNgOptionDiagnostics(cancellationToken?: ts.CancellationToken): ReadonlyArray<Diagnostic>; getNgOptionDiagnostics(cancellationToken?: ts.CancellationToken):
ReadonlyArray<ts.Diagnostic|Diagnostic>;
/** /**
* Retrieve the syntax diagnostics from TypeScript. This is faster than calling * Retrieve the syntax diagnostics from TypeScript. This is faster than calling

View File

@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import * as ts from 'typescript';
import {NgtscTestEnvironment} from './env'; import {NgtscTestEnvironment} from './env';
const trim = (input: string): string => input.replace(/\s+/g, ' ').trim(); const trim = (input: string): string => input.replace(/\s+/g, ' ').trim();
@ -1252,6 +1254,155 @@ describe('ngtsc behavioral tests', () => {
const dtsContents = env.getContents('flat.d.ts'); const dtsContents = env.getContents('flat.d.ts');
expect(dtsContents).toContain('/// <amd-module name="@mymodule" />'); expect(dtsContents).toContain('/// <amd-module name="@mymodule" />');
}); });
it('should report an error when a flat module index is requested but no entrypoint can be determined',
() => {
env.tsconfig({'flatModuleOutFile': 'flat.js'});
env.write('test.ts', 'export class Foo {}');
env.write('test2.ts', 'export class Bar {}');
const errors = env.driveDiagnostics();
expect(errors.length).toBe(1);
expect(errors[0].messageText)
.toBe(
'Angular compiler option "flatModuleOutFile" requires one and only one .ts file in the "files" field.');
});
it('should report an error when a visible directive is not exported', () => {
env.tsconfig({'flatModuleOutFile': 'flat.js'});
env.write('test.ts', `
import {Directive, NgModule} from '@angular/core';
// The directive is not exported.
@Directive({selector: 'test'})
class Dir {}
// The module is, which makes the directive visible.
@NgModule({declarations: [Dir], exports: [Dir]})
export class Module {}
`);
const errors = env.driveDiagnostics();
expect(errors.length).toBe(1);
expect(errors[0].messageText)
.toBe(
'Unsupported private class Dir. This class is visible ' +
'to consumers via Module -> Dir, but is not exported from the top-level library ' +
'entrypoint.');
// Verify that the error is for the correct class.
const id = expectTokenAtPosition(errors[0].file !, errors[0].start !, ts.isIdentifier);
expect(id.text).toBe('Dir');
expect(ts.isClassDeclaration(id.parent)).toBe(true);
});
it('should report an error when a deeply visible directive is not exported', () => {
env.tsconfig({'flatModuleOutFile': 'flat.js'});
env.write('test.ts', `
import {Directive, NgModule} from '@angular/core';
// The directive is not exported.
@Directive({selector: 'test'})
class Dir {}
// Neither is the module which declares it - meaning the directive is not visible here.
@NgModule({declarations: [Dir], exports: [Dir]})
class DirModule {}
// The module is, which makes the directive visible.
@NgModule({exports: [DirModule]})
export class Module {}
`);
const errors = env.driveDiagnostics();
expect(errors.length).toBe(2);
expect(errors[0].messageText)
.toBe(
'Unsupported private class DirModule. This class is ' +
'visible to consumers via Module -> DirModule, but is not exported from the top-level ' +
'library entrypoint.');
expect(errors[1].messageText)
.toBe(
'Unsupported private class Dir. This class is visible ' +
'to consumers via Module -> DirModule -> Dir, but is not exported from the top-level ' +
'library entrypoint.');
});
it('should report an error when a deeply visible module is not exported', () => {
env.tsconfig({'flatModuleOutFile': 'flat.js'});
env.write('test.ts', `
import {Directive, NgModule} from '@angular/core';
// The directive is exported.
@Directive({selector: 'test'})
export class Dir {}
// The module which declares it is not.
@NgModule({declarations: [Dir], exports: [Dir]})
class DirModule {}
// The module is, which makes the module and directive visible.
@NgModule({exports: [DirModule]})
export class Module {}
`);
const errors = env.driveDiagnostics();
expect(errors.length).toBe(1);
expect(errors[0].messageText)
.toBe(
'Unsupported private class DirModule. This class is ' +
'visible to consumers via Module -> DirModule, but is not exported from the top-level ' +
'library entrypoint.');
});
it('should not report an error when a non-exported module is imported by a visible one', () => {
env.tsconfig({'flatModuleOutFile': 'flat.js'});
env.write('test.ts', `
import {Directive, NgModule} from '@angular/core';
// The directive is not exported.
@Directive({selector: 'test'})
class Dir {}
// Neither is the module which declares it.
@NgModule({declarations: [Dir], exports: [Dir]})
class DirModule {}
// This module is, but it doesn't re-export the module, so it doesn't make the module and
// directive visible.
@NgModule({imports: [DirModule]})
export class Module {}
`);
const errors = env.driveDiagnostics();
expect(errors.length).toBe(0);
});
it('should not report an error when re-exporting an external symbol', () => {
env.tsconfig({'flatModuleOutFile': 'flat.js'});
env.write('test.ts', `
import {Directive, NgModule} from '@angular/core';
import {ExternalModule} from 'external';
// This module makes ExternalModule and ExternalDir visible.
@NgModule({exports: [ExternalModule]})
export class Module {}
`);
env.write('node_modules/external/index.d.ts', `
import {ɵDirectiveDefWithMeta, ɵNgModuleDefWithMeta} from '@angular/core';
export declare class ExternalDir {
static ngDirectiveDef: ɵDirectiveDefWithMeta<ExternalDir, '[test]', never, never, never, never>;
}
export declare class ExternalModule {
static ngModuleDef: ɵNgModuleDefWithMeta<ExternalModule, [typeof ExternalDir], never, [typeof ExternalDir]>;
}
`);
const errors = env.driveDiagnostics();
expect(errors.length).toBe(0);
});
}); });
it('should execute custom transformers', () => { it('should execute custom transformers', () => {
@ -1282,3 +1433,11 @@ describe('ngtsc behavioral tests', () => {
}); });
}); });
function expectTokenAtPosition<T extends ts.Node>(
sf: ts.SourceFile, pos: number, guard: (node: ts.Node) => node is T): T {
// getTokenAtPosition is part of TypeScript's private API.
const node = (ts as any).getTokenAtPosition(sf, pos) as ts.Node;
expect(guard(node)).toBe(true);
return node as T;
}