diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md index 52aba50642c..0fba1f0e373 100644 --- a/goldens/public-api/compiler-cli/error_code.md +++ b/goldens/public-api/compiler-cli/error_code.md @@ -31,6 +31,7 @@ export enum ErrorCode { DUPLICATE_VARIABLE_DECLARATION = 8006, HOST_BINDING_PARSE_ERROR = 5001, IMPORT_CYCLE_DETECTED = 3003, + IMPORT_GENERATION_FAILURE = 3004, INJECTABLE_DUPLICATE_PROV = 9001, INLINE_TCB_REQUIRED = 8900, INLINE_TYPE_CTOR_REQUIRED = 8901, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 4e114696580..036a04da3e2 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -12,7 +12,7 @@ import ts from 'typescript'; import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; import {absoluteFrom, relative} from '../../file_system'; -import {ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; +import {assertSuccessfulReferenceEmit, ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {DependencyTracker} from '../../incremental/api'; import {extractSemanticTypeParameters, isArrayEqual, isReferenceEqual, SemanticDepGraphUpdater, SemanticReference, SemanticSymbol} from '../../incremental/semantic_graph'; import {IndexingContext} from '../../indexer'; @@ -690,6 +690,8 @@ export class ComponentDecoratorHandler implements R3UsedDirectiveMetadata&{ref: Reference, importedFile: ImportedFile}; const usedDirectives: UsedDirective[] = bound.getUsedDirectives().map(directive => { const type = this.refEmitter.emit(directive.ref, context); + assertSuccessfulReferenceEmit( + type, node.name, directive.isComponent ? 'component' : 'directive'); return { ref: directive.ref, type: type.expression, @@ -715,6 +717,7 @@ export class ComponentDecoratorHandler implements } const pipe = pipes.get(pipeName)!; const type = this.refEmitter.emit(pipe, context); + assertSuccessfulReferenceEmit(type, node.name, 'pipe'); usedPipes.push({ ref: pipe, pipeName, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index f6e0a46cc18..216b118b763 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -10,7 +10,7 @@ import {compileClassMetadata, compileDeclareClassMetadata, compileDeclareInjecto import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; -import {Reference, ReferenceEmitter} from '../../imports'; +import {assertSuccessfulReferenceEmit, Reference, ReferenceEmitter} from '../../imports'; import {isArrayEqual, isReferenceEqual, isSymbolEqual, SemanticReference, SemanticSymbol} from '../../incremental/semantic_graph'; import {InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator'; @@ -280,12 +280,19 @@ export class NgModuleDecoratorHandler implements typeContext = typeNode.getSourceFile(); } - const bootstrap = - bootstrapRefs.map(bootstrap => this._toR3Reference(bootstrap, valueContext, typeContext)); - const declarations = - declarationRefs.map(decl => this._toR3Reference(decl, valueContext, typeContext)); - const imports = importRefs.map(imp => this._toR3Reference(imp, valueContext, typeContext)); - const exports = exportRefs.map(exp => this._toR3Reference(exp, valueContext, typeContext)); + const bootstrap = bootstrapRefs.map( + bootstrap => this._toR3Reference( + bootstrap.getOriginForDiagnostics(meta, node.name), bootstrap, valueContext, + typeContext)); + const declarations = declarationRefs.map( + decl => this._toR3Reference( + decl.getOriginForDiagnostics(meta, node.name), decl, valueContext, typeContext)); + const imports = importRefs.map( + imp => this._toR3Reference( + imp.getOriginForDiagnostics(meta, node.name), imp, valueContext, typeContext)); + const exports = exportRefs.map( + exp => this._toR3Reference( + exp.getOriginForDiagnostics(meta, node.name), exp, valueContext, typeContext)); const isForwardReference = (ref: R3Reference) => isExpressionForwardReference(ref.value, node.name!, valueContext); @@ -419,7 +426,9 @@ export class NgModuleDecoratorHandler implements const context = getSourceFile(node); for (const exportRef of analysis.exports) { if (isNgModule(exportRef.node, scope.compilation)) { - data.injectorImports.push(this.refEmitter.emit(exportRef, context).expression); + const type = this.refEmitter.emit(exportRef, context); + assertSuccessfulReferenceEmit(type, node, 'NgModule'); + data.injectorImports.push(type.expression); } } @@ -506,12 +515,21 @@ export class NgModuleDecoratorHandler implements for (const decl of declarations) { const remoteScope = this.scopeRegistry.getRemoteScope(decl.node); if (remoteScope !== null) { - const directives = remoteScope.directives.map( - directive => this.refEmitter.emit(directive, context).expression); - const pipes = remoteScope.pipes.map(pipe => this.refEmitter.emit(pipe, context).expression); + const directives = remoteScope.directives.map(directive => { + const type = this.refEmitter.emit(directive, context); + assertSuccessfulReferenceEmit(type, node, 'directive'); + return type.expression; + }); + const pipes = remoteScope.pipes.map(pipe => { + const type = this.refEmitter.emit(pipe, context); + assertSuccessfulReferenceEmit(type, node, 'pipe'); + return type.expression; + }); const directiveArray = new LiteralArrayExpr(directives); const pipesArray = new LiteralArrayExpr(pipes); - const declExpr = this.refEmitter.emit(decl, context).expression; + const componentType = this.refEmitter.emit(decl, context); + assertSuccessfulReferenceEmit(componentType, node, 'component'); + const declExpr = componentType.expression; const setComponentScope = new ExternalExpr(R3Identifiers.setComponentScope); const callExpr = new InvokeFunctionExpr(setComponentScope, [declExpr, directiveArray, pipesArray]); @@ -543,17 +561,17 @@ export class NgModuleDecoratorHandler implements } private _toR3Reference( - valueRef: Reference, valueContext: ts.SourceFile, + origin: ts.Node, valueRef: Reference, valueContext: ts.SourceFile, typeContext: ts.SourceFile): R3Reference { if (valueRef.hasOwningModuleGuess) { - return toR3Reference(valueRef, valueRef, valueContext, valueContext, this.refEmitter); + return toR3Reference(origin, valueRef, valueRef, valueContext, valueContext, this.refEmitter); } else { let typeRef = valueRef; let typeNode = this.reflector.getDtsDeclaration(typeRef.node); if (typeNode !== null && isNamedClassDeclaration(typeNode)) { typeRef = new Reference(typeNode); } - return toR3Reference(valueRef, typeRef, valueContext, typeContext, this.refEmitter); + return toR3Reference(origin, valueRef, typeRef, valueContext, typeContext, this.refEmitter); } } diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts index 2b4dca1afcf..56d38ccace2 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts @@ -10,7 +10,7 @@ import {Expression, ExternalExpr, FactoryTarget, LiteralExpr, ParseLocation, Par import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; -import {ImportFlags, Reference, ReferenceEmitter} from '../../imports'; +import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../imports'; import {attachDefaultImportDeclaration} from '../../imports/src/default'; import {ForeignFunctionResolver, PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, CtorParameter, Decorator, Import, ImportedTypeValueReference, isNamedClassDeclaration, LocalTypeValueReference, ReflectionHost, TypeValueReference, TypeValueReferenceKind, UnavailableValue, ValueUnavailableKind} from '../../reflection'; @@ -257,13 +257,18 @@ function createUnsuitableInjectionTokenError( } export function toR3Reference( - valueRef: Reference, typeRef: Reference, valueContext: ts.SourceFile, + origin: ts.Node, valueRef: Reference, typeRef: Reference, valueContext: ts.SourceFile, typeContext: ts.SourceFile, refEmitter: ReferenceEmitter): R3Reference { + const emittedValueRef = refEmitter.emit(valueRef, valueContext); + assertSuccessfulReferenceEmit(emittedValueRef, origin, 'class'); + + const emittedTypeRef = refEmitter.emit( + typeRef, typeContext, ImportFlags.ForceNewImport | ImportFlags.AllowTypeImports); + assertSuccessfulReferenceEmit(emittedTypeRef, origin, 'class'); + return { - value: refEmitter.emit(valueRef, valueContext).expression, - type: refEmitter - .emit(typeRef, typeContext, ImportFlags.ForceNewImport | ImportFlags.AllowTypeImports) - .expression, + value: emittedValueRef.expression, + type: emittedTypeRef.expression, }; } diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/index.ts b/packages/compiler-cli/src/ngtsc/diagnostics/index.ts index 4ff58490a23..25a283f8781 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/index.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/index.ts @@ -7,7 +7,7 @@ */ export {COMPILER_ERRORS_WITH_GUIDES} from './src/docs'; -export {FatalDiagnosticError, isFatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from './src/error'; +export {FatalDiagnosticError, isFatalDiagnosticError, makeDiagnostic, makeDiagnosticChain, makeRelatedInformation} from './src/error'; export {ErrorCode} from './src/error_code'; export {ERROR_DETAILS_PAGE_BASE_URL} from './src/error_details_base_url'; export {ngErrorCode, replaceTsWithNgInErrors} from './src/util'; diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts index 9ef6d8665c1..bd16c149591 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts @@ -42,6 +42,16 @@ export function makeDiagnostic( }; } +export function makeDiagnosticChain( + messageText: string, next?: ts.DiagnosticMessageChain[]): ts.DiagnosticMessageChain { + return { + category: ts.DiagnosticCategory.Message, + code: 0, + messageText, + next, + }; +} + export function makeRelatedInformation( node: ts.Node, messageText: string): ts.DiagnosticRelatedInformation { node = ts.getOriginalNode(node); diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index f791145f786..b41686eee3a 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -64,6 +64,11 @@ export enum ErrorCode { */ IMPORT_CYCLE_DETECTED = 3003, + /** + * Raised when the compiler is unable to generate an import statement for a reference. + */ + IMPORT_GENERATION_FAILURE = 3004, + CONFIG_FLAT_MODULE_NO_INDEX = 4001, CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002, diff --git a/packages/compiler-cli/src/ngtsc/imports/BUILD.bazel b/packages/compiler-cli/src/ngtsc/imports/BUILD.bazel index 026cfb7b0be..02288da57d2 100644 --- a/packages/compiler-cli/src/ngtsc/imports/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/imports/BUILD.bazel @@ -11,6 +11,7 @@ ts_library( "//packages:types", "//packages/compiler", "//packages/compiler-cli/src/ngtsc/core:api", + "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/reflection", "//packages/compiler-cli/src/ngtsc/util", diff --git a/packages/compiler-cli/src/ngtsc/imports/index.ts b/packages/compiler-cli/src/ngtsc/imports/index.ts index 523bdffe6ba..e042588d6eb 100644 --- a/packages/compiler-cli/src/ngtsc/imports/index.ts +++ b/packages/compiler-cli/src/ngtsc/imports/index.ts @@ -9,7 +9,7 @@ export {AliasingHost, AliasStrategy, PrivateExportAliasingHost, UnifiedModulesAliasingHost} from './src/alias'; export {ImportRewriter, NoopImportRewriter, R3SymbolsImportRewriter, validateAndRewriteCoreSymbol} from './src/core'; export {DefaultImportTracker} from './src/default'; -export {AbsoluteModuleStrategy, EmittedReference, ImportedFile, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesStrategy} from './src/emitter'; +export {AbsoluteModuleStrategy, assertSuccessfulReferenceEmit, EmittedReference, FailedEmitResult, ImportedFile, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitKind, ReferenceEmitResult, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesStrategy} from './src/emitter'; export {Reexport} from './src/reexport'; export {OwningModule, Reference} from './src/references'; export {ModuleResolver} from './src/resolver'; diff --git a/packages/compiler-cli/src/ngtsc/imports/src/alias.ts b/packages/compiler-cli/src/ngtsc/imports/src/alias.ts index 98b7ef703c6..4c41e3cc4a3 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/alias.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/alias.ts @@ -12,7 +12,7 @@ import ts from 'typescript'; import {UnifiedModulesHost} from '../../core/api'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {EmittedReference, ImportFlags, ReferenceEmitStrategy} from './emitter'; +import {EmittedReference, ImportFlags, ReferenceEmitKind, ReferenceEmitStrategy} from './emitter'; import {Reference} from './references'; @@ -217,6 +217,10 @@ export class AliasStrategy implements ReferenceEmitStrategy { return null; } - return {expression: ref.alias, importedFile: 'unknown'}; + return { + kind: ReferenceEmitKind.Success, + expression: ref.alias, + importedFile: 'unknown', + }; } } diff --git a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts index b1b86b19e34..d272a5113b2 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts @@ -9,6 +9,7 @@ import {Expression, ExternalExpr, ExternalReference, WrappedNodeExpr} from '@ang import ts from 'typescript'; import {UnifiedModulesHost} from '../../core/api'; +import {ErrorCode, FatalDiagnosticError, makeDiagnosticChain, makeRelatedInformation} from '../../diagnostics'; import {absoluteFromSourceFile, dirname, LogicalFileSystem, LogicalProjectPath, relative, toRelativeImport} from '../../file_system'; import {stripExtension} from '../../file_system/src/util'; import {DeclarationNode, ReflectionHost} from '../../reflection'; @@ -62,11 +63,18 @@ export enum ImportFlags { */ export type ImportedFile = ts.SourceFile|'unknown'|null; +export const enum ReferenceEmitKind { + Success, + Failed, +} + /** * Represents the emitted expression of a `Reference` that is valid in the source file it was * emitted from. */ export interface EmittedReference { + kind: ReferenceEmitKind.Success; + /** * The expression that refers to `Reference`. */ @@ -81,6 +89,52 @@ export interface EmittedReference { importedFile: ImportedFile; } +/** + * Represents a failure to emit a `Reference` into a different source file. + */ +export interface FailedEmitResult { + kind: ReferenceEmitKind.Failed; + + /** + * The reference that could not be emitted. + */ + ref: Reference; + + /** + * The source file into which the reference was requested to be emitted. + */ + context: ts.SourceFile; + + /** + * Describes why the reference could not be emitted. This may be shown in a diagnostic. + */ + reason: string; +} + +export type ReferenceEmitResult = EmittedReference|FailedEmitResult; + +/** + * Verifies that a reference was emitted successfully, or raises a `FatalDiagnosticError` otherwise. + * @param result The emit result that should have been successful. + * @param origin The node that is used to report the failure diagnostic. + * @param typeKind The kind of the symbol that the reference represents, e.g. 'component' or + * 'class'. + */ +export function assertSuccessfulReferenceEmit( + result: ReferenceEmitResult, origin: ts.Node, + typeKind: string): asserts result is EmittedReference { + if (result.kind === ReferenceEmitKind.Success) { + return; + } + + const message = makeDiagnosticChain( + `Unable to import ${typeKind} ${nodeNameForError(result.ref.node)}.`, + [makeDiagnosticChain(result.reason)]); + throw new FatalDiagnosticError( + ErrorCode.IMPORT_GENERATION_FAILURE, origin, message, + [makeRelatedInformation(result.ref.node, `The ${typeKind} is declared here.`)]); +} + /** * A particular strategy for generating an expression which refers to a `Reference`. * @@ -104,7 +158,7 @@ export interface ReferenceEmitStrategy { * @returns an `EmittedReference` which refers to the `Reference`, or `null` if none can be * generated */ - emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): EmittedReference|null; + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): ReferenceEmitResult|null; } /** @@ -117,15 +171,20 @@ export class ReferenceEmitter { constructor(private strategies: ReferenceEmitStrategy[]) {} emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags = ImportFlags.None): - EmittedReference { + ReferenceEmitResult { for (const strategy of this.strategies) { const emitted = strategy.emit(ref, context, importFlags); if (emitted !== null) { return emitted; } } - throw new Error(`Unable to write a reference to ${nodeNameForError(ref.node)} in ${ - ref.node.getSourceFile().fileName} from ${context.fileName}`); + + return { + kind: ReferenceEmitKind.Failed, + ref, + context, + reason: `Unable to write a reference to ${nodeNameForError(ref.node)}.`, + }; } } @@ -150,6 +209,7 @@ export class LocalIdentifierStrategy implements ReferenceEmitStrategy { // invalid emission of a free-standing `foo` identifier, rather than `exports.foo`. if (!isDeclaration(ref.node) && refSf === context) { return { + kind: ReferenceEmitKind.Success, expression: new WrappedNodeExpr(ref.node), importedFile: null, }; @@ -160,6 +220,7 @@ export class LocalIdentifierStrategy implements ReferenceEmitStrategy { const identifier = ref.getIdentityIn(context); if (identifier !== null) { return { + kind: ReferenceEmitKind.Success, expression: new WrappedNodeExpr(identifier), importedFile: null, }; @@ -176,12 +237,12 @@ interface ModuleExports { /** * The source file of the module. */ - module: ts.SourceFile; + module: ts.SourceFile|null; /** * The map of declarations to their exported name. */ - exportMap: Map; + exportMap: Map|null; } /** @@ -198,13 +259,13 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { * A cache of the exports of specific modules, because resolving a module to its exports is a * costly operation. */ - private moduleExportsCache = new Map(); + private moduleExportsCache = new Map(); constructor( protected program: ts.Program, protected checker: ts.TypeChecker, protected moduleResolver: ModuleResolver, private reflectionHost: ReflectionHost) {} - emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): EmittedReference|null { + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): ReferenceEmitResult|null { if (ref.bestGuessOwningModule === null) { // There is no module name available for this Reference, meaning it was arrived at via a // relative path. @@ -221,38 +282,48 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { // Try to find the exported name of the declaration, if one is available. const {specifier, resolutionContext} = ref.bestGuessOwningModule; const exports = this.getExportsOfModule(specifier, resolutionContext); - if (exports === null || !exports.exportMap.has(ref.node)) { - // TODO(alxhub): make this error a ts.Diagnostic pointing at whatever caused this import to be - // triggered. - throw new Error(`Symbol ${ref.debugName} declared in ${ - getSourceFile(ref.node).fileName} is not exported from ${specifier} (import into ${ - context.fileName})`); + if (exports.module === null) { + return { + kind: ReferenceEmitKind.Failed, + ref, + context, + reason: `The module '${specifier}' could not be found.`, + }; + } else if (exports.exportMap === null || !exports.exportMap.has(ref.node)) { + return { + kind: ReferenceEmitKind.Failed, + ref, + context, + reason: + `The symbol is not exported from ${exports.module.fileName} (module '${specifier}').`, + }; } const symbolName = exports.exportMap.get(ref.node)!; return { + kind: ReferenceEmitKind.Success, expression: new ExternalExpr(new ExternalReference(specifier, symbolName)), importedFile: exports.module, }; } - private getExportsOfModule(moduleName: string, fromFile: string): ModuleExports|null { + private getExportsOfModule(moduleName: string, fromFile: string): ModuleExports { if (!this.moduleExportsCache.has(moduleName)) { this.moduleExportsCache.set(moduleName, this.enumerateExportsOfModule(moduleName, fromFile)); } return this.moduleExportsCache.get(moduleName)!; } - protected enumerateExportsOfModule(specifier: string, fromFile: string): ModuleExports|null { + protected enumerateExportsOfModule(specifier: string, fromFile: string): ModuleExports { // First, resolve the module specifier to its entry point, and get the ts.Symbol for it. const entryPointFile = this.moduleResolver.resolveModule(specifier, fromFile); if (entryPointFile === null) { - return null; + return {module: null, exportMap: null}; } const exports = this.reflectionHost.getExportsOfModule(entryPointFile); if (exports === null) { - return null; + return {module: entryPointFile, exportMap: null}; } const exportMap = new Map(); for (const [name, declaration] of exports) { @@ -284,7 +355,7 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { export class LogicalProjectStrategy implements ReferenceEmitStrategy { constructor(private reflector: ReflectionHost, private logicalFs: LogicalFileSystem) {} - emit(ref: Reference, context: ts.SourceFile): EmittedReference|null { + emit(ref: Reference, context: ts.SourceFile): ReferenceEmitResult|null { const destSf = getSourceFile(ref.node); // Compute the relative path from the importing file to the file being imported. This is done @@ -292,7 +363,14 @@ export class LogicalProjectStrategy implements ReferenceEmitStrategy { const destPath = this.logicalFs.logicalPathOfSf(destSf); if (destPath === null) { // The imported file is not within the logical project filesystem. - return null; + // Note: this error is analogous to `TS6059: File is not under 'rootDir'` that TypeScript + // reports. + return { + kind: ReferenceEmitKind.Failed, + ref, + context, + reason: `The file ${destSf.fileName} is outside of the configured 'rootDir'.`, + }; } const originPath = this.logicalFs.logicalPathOfSf(context); @@ -309,13 +387,19 @@ export class LogicalProjectStrategy implements ReferenceEmitStrategy { const name = findExportedNameOfNode(ref.node, destSf, this.reflector); if (name === null) { // The target declaration isn't exported from the file it's declared in. This is an issue! - return null; + return { + kind: ReferenceEmitKind.Failed, + ref, + context, + reason: `The symbol is not exported from ${destSf.fileName}.`, + }; } // With both files expressed as LogicalProjectPaths, getting the module specifier as a relative // path is now straightforward. const moduleName = LogicalProjectPath.relativePathBetween(originPath, destPath); return { + kind: ReferenceEmitKind.Success, expression: new ExternalExpr({moduleName, name}), importedFile: destSf, }; @@ -331,14 +415,26 @@ export class LogicalProjectStrategy implements ReferenceEmitStrategy { export class RelativePathStrategy implements ReferenceEmitStrategy { constructor(private reflector: ReflectionHost) {} - emit(ref: Reference, context: ts.SourceFile): EmittedReference|null { + emit(ref: Reference, context: ts.SourceFile): ReferenceEmitResult|null { const destSf = getSourceFile(ref.node); const relativePath = relative(dirname(absoluteFromSourceFile(context)), absoluteFromSourceFile(destSf)); const moduleName = toRelativeImport(stripExtension(relativePath)); const name = findExportedNameOfNode(ref.node, destSf, this.reflector); - return {expression: new ExternalExpr({moduleName, name}), importedFile: destSf}; + if (name === null) { + return { + kind: ReferenceEmitKind.Failed, + ref, + context, + reason: `The symbol is not exported from ${destSf.fileName}.`, + }; + } + return { + kind: ReferenceEmitKind.Success, + expression: new ExternalExpr({moduleName, name}), + importedFile: destSf, + }; } } @@ -360,6 +456,7 @@ export class UnifiedModulesStrategy implements ReferenceEmitStrategy { this.unifiedModulesHost.fileNameToModuleName(destSf.fileName, context.fileName); return { + kind: ReferenceEmitKind.Success, expression: new ExternalExpr({moduleName, name}), importedFile: destSf, }; diff --git a/packages/compiler-cli/src/ngtsc/imports/src/find_export.ts b/packages/compiler-cli/src/ngtsc/imports/src/find_export.ts index 65e0f88a8bf..61214cee43e 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/find_export.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/find_export.ts @@ -7,6 +7,7 @@ */ import ts from 'typescript'; + import {ReflectionHost} from '../../reflection'; import {isNamedDeclaration} from '../../util/src/typescript'; @@ -36,32 +37,5 @@ export function findExportedNameOfNode( foundExportName = exportName; } - - if (foundExportName === null) { - throw new Error( - `Failed to find exported name of node (${target.getText()}) in '${file.fileName}'.`); - } return foundExportName; } - -/** - * Check whether a given `ts.Symbol` represents a declaration of a given node. - * - * This is not quite as trivial as just checking the declarations, as some nodes are - * `ts.ExportSpecifier`s and need to be unwrapped. - */ -function symbolDeclaresNode(sym: ts.Symbol, node: ts.Node, checker: ts.TypeChecker): boolean { - if (sym.declarations === undefined) { - return false; - } - - return sym.declarations.some(decl => { - if (ts.isExportSpecifier(decl)) { - const exportedSymbol = checker.getExportSpecifierLocalTargetSymbol(decl); - if (exportedSymbol !== undefined) { - return symbolDeclaresNode(exportedSymbol, node, checker); - } - } - return decl === node; - }); -} diff --git a/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts b/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts index 04e1737302c..fc58e5a6ace 100644 --- a/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts +++ b/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts @@ -7,13 +7,13 @@ */ import {ExternalExpr} from '@angular/compiler'; import ts from 'typescript'; -import {UnifiedModulesHost} from '../../core/api'; +import {UnifiedModulesHost} from '../../core/api'; import {absoluteFrom as _, basename, LogicalFileSystem} from '../../file_system'; import {runInEachFileSystem, TestFile} from '../../file_system/testing'; import {Declaration, TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing'; -import {AbsoluteModuleStrategy, ImportFlags, LogicalProjectStrategy, RelativePathStrategy, UnifiedModulesStrategy} from '../src/emitter'; +import {AbsoluteModuleStrategy, ImportFlags, LogicalProjectStrategy, ReferenceEmitKind, RelativePathStrategy, UnifiedModulesStrategy} from '../src/emitter'; import {Reference} from '../src/references'; import {ModuleResolver} from '../src/resolver'; @@ -75,7 +75,7 @@ runInEachFileSystem(() => { resolutionContext: context.fileName, }); const emitted = strategy.emit(reference, context, ImportFlags.None); - if (emitted === null) { + if (emitted === null || emitted.kind !== ReferenceEmitKind.Success) { return fail('Reference should be emitted'); } if (!(emitted.expression instanceof ExternalExpr)) { @@ -108,7 +108,7 @@ runInEachFileSystem(() => { resolutionContext: context.fileName, }); const emitted = strategy.emit(reference, context, ImportFlags.None); - if (emitted === null) { + if (emitted === null || emitted.kind !== ReferenceEmitKind.Success) { return fail('Reference should be emitted'); } if (!(emitted.expression instanceof ExternalExpr)) { @@ -160,7 +160,7 @@ runInEachFileSystem(() => { const reference = new Reference(decl, {specifier: 'external', resolutionContext: context.fileName}); const emitted = strategy.emit(reference, context, ImportFlags.AllowTypeImports); - if (emitted === null) { + if (emitted === null || emitted.kind !== ReferenceEmitKind.Success) { return fail('Reference should be emitted'); } if (!(emitted.expression instanceof ExternalExpr)) { @@ -204,7 +204,9 @@ runInEachFileSystem(() => { const decl = getDeclaration(program, _('/index.ts'), 'Foo', ts.isClassDeclaration); const context = program.getSourceFile(_('/context.ts'))!; const ref = strategy.emit(new Reference(decl), context); - expect(ref).not.toBeNull(); + if (ref === null || ref.kind !== ReferenceEmitKind.Success) { + return fail('Reference should be emitted'); + } // Expect the prefixed name from the TestHost. expect((ref!.expression as ExternalExpr).value.name).toEqual('testFoo'); @@ -232,7 +234,7 @@ runInEachFileSystem(() => { const decl = getDeclaration(program, _('/index.ts'), 'Foo', ts.isClassDeclaration); const context = program.getSourceFile(_('/context.ts'))!; const emitted = strategy.emit(new Reference(decl), context); - if (emitted === null) { + if (emitted === null || emitted.kind !== ReferenceEmitKind.Success) { return fail('Reference should be emitted'); } if (!(emitted.expression instanceof ExternalExpr)) { @@ -265,7 +267,7 @@ runInEachFileSystem(() => { const decl = getDeclaration(program, _('/index.ts'), 'Foo', ts.isClassDeclaration); const context = program.getSourceFile(_('/context.ts'))!; const emitted = strategy.emit(new Reference(decl), context); - if (emitted === null) { + if (emitted === null || emitted.kind !== ReferenceEmitKind.Success) { return fail('Reference should be emitted'); } if (!(emitted.expression instanceof ExternalExpr)) { diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index c1fd10c7784..3bc5cdda232 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -10,7 +10,7 @@ import {ExternalExpr, SchemaMetadata} from '@angular/compiler'; import ts from 'typescript'; import {ErrorCode, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; -import {AliasingHost, Reexport, Reference, ReferenceEmitter} from '../../imports'; +import {AliasingHost, assertSuccessfulReferenceEmit, Reexport, Reference, ReferenceEmitter} from '../../imports'; import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata'; import {ClassDeclaration, DeclarationNode} from '../../reflection'; import {identifierOfNode, nodeNameForError} from '../../util/src/typescript'; @@ -511,8 +511,9 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop asAlias: exportName, }); } else { - const expr = - this.refEmitter.emit(exportRef.cloneWithNoIdentifiers(), sourceFile).expression; + const emittedRef = this.refEmitter.emit(exportRef.cloneWithNoIdentifiers(), sourceFile); + assertSuccessfulReferenceEmit(emittedRef, ngModuleRef.node.name, 'class'); + const expr = emittedRef.expression; if (!(expr instanceof ExternalExpr) || expr.value.moduleName === null || expr.value.name === null) { throw new Error('Expected ExternalExpr'); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index c8975b22e27..0995cffe968 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -9,7 +9,7 @@ import {ExpressionType, ExternalExpr, Type, WrappedNodeExpr} from '@angular/compiler'; import ts from 'typescript'; -import {ImportFlags, Reference, ReferenceEmitter} from '../../imports'; +import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../imports'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; import {ImportManager, translateExpression, translateType} from '../../translator'; import {TypeCheckableDirectiveMeta, TypeCheckingConfig, TypeCtorMetadata} from '../api'; @@ -121,6 +121,7 @@ export class Environment { // in these cases as there is no strict dependency checking during the template type-checking // pass. const ngExpr = this.refEmitter.emit(ref, this.contextFile, ImportFlags.NoAliasing); + assertSuccessfulReferenceEmit(ngExpr, this.contextFile, 'class'); // Use `translateExpression` to convert the `Expression` into a `ts.Expression`. return translateExpression(ngExpr.expression, this.importManager); @@ -134,6 +135,7 @@ export class Environment { referenceType(ref: Reference): ts.TypeNode { const ngExpr = this.refEmitter.emit( ref, this.contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports); + assertSuccessfulReferenceEmit(ngExpr, this.contextFile, 'symbol'); // Create an `ExpressionType` from the `Expression` and translate it via `translateType`. // TODO(alxhub): support references to types with generic arguments in a clean way. diff --git a/packages/compiler-cli/test/ngtsc/imports_spec.ts b/packages/compiler-cli/test/ngtsc/imports_spec.ts new file mode 100644 index 00000000000..cc3d7cbc428 --- /dev/null +++ b/packages/compiler-cli/test/ngtsc/imports_spec.ts @@ -0,0 +1,119 @@ +/** + * @license + * Copyright Google LLC 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 ts from 'typescript'; + +import {absoluteFrom} from '../../src/ngtsc/file_system'; +import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing'; +import {loadStandardTestFiles} from '../../src/ngtsc/testing'; + +import {NgtscTestEnvironment} from './env'; + +const testFiles = loadStandardTestFiles(); + +runInEachFileSystem(() => { + describe('import generation', () => { + let env!: NgtscTestEnvironment; + + beforeEach(() => { + env = NgtscTestEnvironment.setup(testFiles, absoluteFrom('/app')); + const tsconfig: {[key: string]: any} = { + extends: '../tsconfig-base.json', + compilerOptions: { + baseUrl: '.', + rootDirs: ['/app'], + }, + angularCompilerOptions: {}, + }; + env.write('tsconfig.json', JSON.stringify(tsconfig, null, 2)); + }); + + it('should report an error when using a directive outside of rootDirs', () => { + env.write('/app/module.ts', ` + import {NgModule} from '@angular/core'; + import {ExternalDir} from '../lib/dir'; + import {MyComponent} from './comp'; + + @NgModule({ + declarations: [ExternalDir, MyComponent], + }) + export class MyModule {} + `); + env.write('/app/comp.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: '
', + }) + export class MyComponent {} + `); + env.write('/lib/dir.ts', ` + import {Directive} from '@angular/core'; + + @Directive({selector: '[external]'}) + export class ExternalDir {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) + .toEqual(`Unable to import class ExternalDir. + The file ${absoluteFrom('/lib/dir.ts')} is outside of the configured 'rootDir'.`); + expect(diags[0].file!.fileName).toEqual(absoluteFrom('/app/module.ts')); + expect(getDiagnosticSourceCode(diags[0])).toEqual('ExternalDir'); + }); + + it('should report an error when a library entry-point does not export the symbol', () => { + env.write('/app/module.ts', ` + import {NgModule} from '@angular/core'; + import {ExternalModule} from 'lib'; + import {MyComponent} from './comp'; + + @NgModule({ + imports: [ExternalModule], + declarations: [MyComponent], + }) + export class MyModule {} + `); + env.write('/app/comp.ts', ` + import {Component} from '@angular/core'; + + @Component({ + template: '
', + }) + export class MyComponent {} + `); + env.write('/node_modules/lib/index.d.ts', ` + import {ɵɵNgModuleDeclaration} from '@angular/core'; + import {ExternalDir} from './dir'; + + export class ExternalModule { + static ɵmod: ɵɵNgModuleDeclaration; + } + `); + env.write('/node_modules/lib/dir.d.ts', ` + import {ɵɵDirectiveDeclaration} from '@angular/core'; + + export class ExternalDir { + static ɵdir: ɵɵDirectiveDeclaration; + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) + .toEqual(`Unable to import directive ExternalDir. + The symbol is not exported from ${absoluteFrom('/node_modules/lib/index.d.ts')} (module 'lib').`); + expect(diags[0].file!.fileName).toEqual(absoluteFrom('/app/comp.ts')); + expect(getDiagnosticSourceCode(diags[0])).toEqual('MyComponent'); + }); + }); +}); + +function getDiagnosticSourceCode(diag: ts.Diagnostic): string { + return diag.file!.text.substring(diag.start!, diag.start! + diag.length!); +}