diff --git a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts index 9589818870..579ab00546 100644 --- a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts @@ -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, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, EnumMember, 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'; @@ -1594,7 +1594,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N {decorators: null, typeExpression: null}; const nameNode = node.name; - let typeValueReference: TypeValueReference|null = null; + let typeValueReference: TypeValueReference; if (typeExpression !== null) { // `typeExpression` is an expression in a "type" context. Resolve it to a declared value. // Either it's a reference to an imported type, or a type declared locally. Distinguish the @@ -1603,7 +1603,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N if (decl !== null && decl.node !== null && decl.viaModule !== null && isNamedDeclaration(decl.node)) { typeValueReference = { - local: false, + kind: TypeValueReferenceKind.IMPORTED, valueDeclaration: decl.node, moduleName: decl.viaModule, importedName: decl.node.name.text, @@ -1611,11 +1611,16 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N }; } else { typeValueReference = { - local: true, + kind: TypeValueReferenceKind.LOCAL, expression: typeExpression, defaultImportStatement: null, }; } + } else { + typeValueReference = { + kind: TypeValueReferenceKind.UNAVAILABLE, + reason: {kind: ValueUnavailableKind.MISSING_TYPE}, + }; } return { diff --git a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts index df863ee554..922b88f777 100644 --- a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing'; import {MockLogger} from '../../../src/ngtsc/logging/testing'; -import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection'; +import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection'; import {getDeclaration} from '../../../src/ngtsc/testing'; import {loadFakeCore, loadTestFiles} from '../../../test/helpers'; import {CommonJsReflectionHost} from '../../src/host/commonjs_host'; @@ -1599,7 +1599,7 @@ exports.MissingClass2 = MissingClass2; isNamedVariableDeclaration); const ctrDecorators = host.getConstructorParameters(classNode)!; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as { - local: true, + kind: TypeValueReferenceKind.LOCAL, expression: ts.Identifier, defaultImportStatement: null, }).expression; diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts index 466484722a..20a8c35a8b 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing'; import {MockLogger} from '../../../src/ngtsc/logging/testing'; -import {ClassMemberKind, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection'; +import {ClassMemberKind, isNamedVariableDeclaration, TypeValueReferenceKind} from '../../../src/ngtsc/reflection'; import {getDeclaration} from '../../../src/ngtsc/testing'; import {loadFakeCore, loadTestFiles, loadTsLib} from '../../../test/helpers'; import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; @@ -484,7 +484,7 @@ runInEachFileSystem(() => { isNamedVariableDeclaration); const ctrDecorators = host.getConstructorParameters(classNode)!; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as { - local: true, + kind: TypeValueReferenceKind.LOCAL, expression: ts.Identifier, defaultImportStatement: null, }).expression; diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts index 6b91881644..5e7251bfc3 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing'; import {MockLogger} from '../../../src/ngtsc/logging/testing'; -import {ClassMemberKind, isNamedFunctionDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection'; +import {ClassMemberKind, isNamedFunctionDeclaration, isNamedVariableDeclaration, TypeValueReferenceKind} from '../../../src/ngtsc/reflection'; import {getDeclaration} from '../../../src/ngtsc/testing'; import {loadFakeCore, loadTestFiles, loadTsLib} from '../../../test/helpers'; import {getIifeBody} from '../../src/host/esm2015_host'; @@ -544,7 +544,7 @@ export { AliasedDirective$1 }; isNamedVariableDeclaration); const ctrDecorators = host.getConstructorParameters(classNode)!; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as { - local: true, + kind: TypeValueReferenceKind.LOCAL, expression: ts.Identifier, defaultImportStatement: null, }).expression; diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts index e7663a72ee..6e219658c5 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing'; import {MockLogger} from '../../../src/ngtsc/logging/testing'; -import {ClassMemberKind, ConcreteDeclaration, CtorParameter, Decorator, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection'; +import {ClassMemberKind, ConcreteDeclaration, CtorParameter, Decorator, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection'; import {getDeclaration} from '../../../src/ngtsc/testing'; import {loadFakeCore, loadTestFiles} from '../../../test/helpers'; import {DelegatingReflectionHost} from '../../src/host/delegating_host'; @@ -1670,7 +1670,7 @@ runInEachFileSystem(() => { bundle.program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', isNamedVariableDeclaration); const ctrDecorators = host.getConstructorParameters(classNode)!; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as { - local: true, + kind: TypeValueReferenceKind.LOCAL, expression: ts.Identifier, defaultImportStatement: null, }).expression; diff --git a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts index e4f712f0f6..01a10eb4bb 100644 --- a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing'; import {MockLogger} from '../../../src/ngtsc/logging/testing'; -import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, Import, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection'; +import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, Import, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection'; import {getDeclaration} from '../../../src/ngtsc/testing'; import {loadFakeCore, loadTestFiles} from '../../../test/helpers'; import {DelegatingReflectionHost} from '../../src/host/delegating_host'; @@ -1709,7 +1709,7 @@ runInEachFileSystem(() => { bundle.program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', isNamedVariableDeclaration); const ctrDecorators = host.getConstructorParameters(classNode)!; const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as { - local: true, + kind: TypeValueReferenceKind.LOCAL, expression: ts.Identifier, defaultImportStatement: null, }).expression; diff --git a/packages/compiler-cli/ngcc/test/host/util.ts b/packages/compiler-cli/ngcc/test/host/util.ts index 0295155af5..c27594c6cf 100644 --- a/packages/compiler-cli/ngcc/test/host/util.ts +++ b/packages/compiler-cli/ngcc/test/host/util.ts @@ -1,4 +1,3 @@ - /** * @license * Copyright Google LLC All Rights Reserved. @@ -7,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; -import {CtorParameter} from '../../../src/ngtsc/reflection'; +import {CtorParameter, TypeValueReferenceKind} from '../../../src/ngtsc/reflection'; /** * Check that a given list of `CtorParameter`s has `typeValueReference`s of specific `ts.Identifier` @@ -18,19 +17,21 @@ export function expectTypeValueReferencesForParameters( parameters!.forEach((param, idx) => { const expected = expectedParams[idx]; if (expected !== null) { - if (param.typeValueReference === null) { + if (param.typeValueReference.kind === TypeValueReferenceKind.UNAVAILABLE) { fail(`Incorrect typeValueReference generated, expected ${expected}`); - } else if (param.typeValueReference.local && fromModule !== null) { + } else if ( + param.typeValueReference.kind === TypeValueReferenceKind.LOCAL && fromModule !== null) { fail(`Incorrect typeValueReference generated, expected non-local`); - } else if (!param.typeValueReference.local && fromModule === null) { + } else if ( + param.typeValueReference.kind !== TypeValueReferenceKind.LOCAL && fromModule === null) { fail(`Incorrect typeValueReference generated, expected local`); - } else if (param.typeValueReference.local) { + } else if (param.typeValueReference.kind === TypeValueReferenceKind.LOCAL) { if (!ts.isIdentifier(param.typeValueReference.expression)) { - fail(`Incorrect typeValueReference generated, expected identifer`); + fail(`Incorrect typeValueReference generated, expected identifier`); } else { expect(param.typeValueReference.expression.text).toEqual(expected); } - } else if (param.typeValueReference !== null) { + } else if (param.typeValueReference.kind === TypeValueReferenceKind.IMPORTED) { expect(param.typeValueReference.moduleName).toBe(fromModule!); expect(param.typeValueReference.importedName).toBe(expected); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts index 3ac72e8d10..af855d3b43 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts @@ -10,7 +10,7 @@ import {Expression, ExternalExpr, FunctionExpr, Identifiers, InvokeFunctionExpr, import * as ts from 'typescript'; import {DefaultImportRecorder} from '../../imports'; -import {CtorParameter, Decorator, ReflectionHost} from '../../reflection'; +import {CtorParameter, Decorator, ReflectionHost, TypeValueReferenceKind} from '../../reflection'; import {valueReferenceToExpression, wrapFunctionExpressionsInParens} from './util'; @@ -105,7 +105,7 @@ function ctorParameterToMetadata( isCore: boolean): Expression { // Parameters sometimes have a type that can be referenced. If so, then use it, otherwise // its type is undefined. - const type = param.typeValueReference !== null ? + const type = param.typeValueReference.kind !== TypeValueReferenceKind.UNAVAILABLE ? valueReferenceToExpression(param.typeValueReference, defaultImportRecorder) : new LiteralExpr(undefined); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts index 53c93fc868..6db327724c 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts @@ -12,13 +12,9 @@ import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; import {DefaultImportRecorder, ImportFlags, Reference, ReferenceEmitter} from '../../imports'; import {ForeignFunctionResolver, PartialEvaluator} from '../../partial_evaluator'; -import {ClassDeclaration, CtorParameter, Decorator, Import, isNamedClassDeclaration, ReflectionHost, TypeValueReference} from '../../reflection'; +import {ClassDeclaration, CtorParameter, Decorator, Import, ImportedTypeValueReference, isNamedClassDeclaration, LocalTypeValueReference, ReflectionHost, TypeValueReference, TypeValueReferenceKind, UnavailableValue, ValueUnavailableKind} from '../../reflection'; import {DeclarationData} from '../../scope'; -export enum ConstructorDepErrorKind { - NO_SUITABLE_TOKEN, -} - export type ConstructorDeps = { deps: R3DependencyMetadata[]; }|{ @@ -29,7 +25,7 @@ export type ConstructorDeps = { export interface ConstructorDepError { index: number; param: CtorParameter; - kind: ConstructorDepErrorKind; + reason: UnavailableValue; } export function getConstructorDependencies( @@ -94,10 +90,14 @@ export function getConstructorDependencies( resolved = R3ResolvedDependencyType.ChangeDetectorRef; } if (token === null) { + if (param.typeValueReference.kind !== TypeValueReferenceKind.UNAVAILABLE) { + throw new Error( + 'Illegal state: expected value reference to be unavailable if no token is present'); + } errors.push({ index: idx, - kind: ConstructorDepErrorKind.NO_SUITABLE_TOKEN, param, + reason: param.typeValueReference.reason, }); } else { deps.push({token, attribute, optional, self, skipSelf, host, resolved}); @@ -118,18 +118,15 @@ export function getConstructorDependencies( * file in which the `TypeValueReference` originated. */ export function valueReferenceToExpression( - valueRef: TypeValueReference, defaultImportRecorder: DefaultImportRecorder): Expression; + valueRef: LocalTypeValueReference|ImportedTypeValueReference, + defaultImportRecorder: DefaultImportRecorder): Expression; export function valueReferenceToExpression( - valueRef: null, defaultImportRecorder: DefaultImportRecorder): null; + valueRef: TypeValueReference, defaultImportRecorder: DefaultImportRecorder): Expression|null; export function valueReferenceToExpression( - valueRef: TypeValueReference|null, defaultImportRecorder: DefaultImportRecorder): Expression| - null; -export function valueReferenceToExpression( - valueRef: TypeValueReference|null, defaultImportRecorder: DefaultImportRecorder): Expression| - null { - if (valueRef === null) { + valueRef: TypeValueReference, defaultImportRecorder: DefaultImportRecorder): Expression|null { + if (valueRef.kind === TypeValueReferenceKind.UNAVAILABLE) { return null; - } else if (valueRef.local) { + } else if (valueRef.kind === TypeValueReferenceKind.LOCAL) { if (defaultImportRecorder !== null && valueRef.defaultImportStatement !== null && ts.isIdentifier(valueRef.expression)) { defaultImportRecorder.recordImportedIdentifier( @@ -137,16 +134,10 @@ export function valueReferenceToExpression( } return new WrappedNodeExpr(valueRef.expression); } else { - // TODO(alxhub): this cast is necessary because the g3 typescript version doesn't narrow here. - const ref = valueRef as { - moduleName: string; - importedName: string; - nestedPath: string[]|null; - }; let importExpr: Expression = - new ExternalExpr({moduleName: ref.moduleName, name: ref.importedName}); - if (ref.nestedPath !== null) { - for (const property of ref.nestedPath) { + new ExternalExpr({moduleName: valueRef.moduleName, name: valueRef.importedName}); + if (valueRef.nestedPath !== null) { + for (const property of valueRef.nestedPath) { importExpr = new ReadPropExpr(importExpr, property); } } @@ -195,17 +186,82 @@ export function validateConstructorDependencies( return deps.deps; } else { // TODO(alxhub): this cast is necessary because the g3 typescript version doesn't narrow here. - const {param, index} = (deps as {errors: ConstructorDepError[]}).errors[0]; // There is at least one error. - throw new FatalDiagnosticError( - ErrorCode.PARAM_MISSING_TOKEN, param.nameNode, - `No suitable injection token for parameter '${param.name || index}' of class '${ - clazz.name.text}'.\n` + - (param.typeNode !== null ? `Found ${param.typeNode.getText()}` : - 'no type or decorator')); + const error = (deps as {errors: ConstructorDepError[]}).errors[0]; + throw createUnsuitableInjectionTokenError(clazz, error); } } +/** + * Creates a fatal error with diagnostic for an invalid injection token. + * @param clazz The class for which the injection token was unavailable. + * @param error The reason why no valid injection token is available. + */ +function createUnsuitableInjectionTokenError( + clazz: ClassDeclaration, error: ConstructorDepError): FatalDiagnosticError { + const {param, index, reason} = error; + let chainMessage: string|undefined = undefined; + let hints: ts.DiagnosticRelatedInformation[]|undefined = undefined; + switch (reason.kind) { + case ValueUnavailableKind.UNSUPPORTED: + chainMessage = 'Consider using the @Inject decorator to specify an injection token.'; + hints = [ + makeRelatedInformation(reason.typeNode, 'This type is not supported as injection token.'), + ]; + break; + case ValueUnavailableKind.NO_VALUE_DECLARATION: + chainMessage = 'Consider using the @Inject decorator to specify an injection token.'; + hints = [ + makeRelatedInformation( + reason.typeNode, + 'This type does not have a value, so it cannot be used as injection token.'), + makeRelatedInformation(reason.decl, 'The type is declared here.'), + ]; + break; + case ValueUnavailableKind.TYPE_ONLY_IMPORT: + chainMessage = + 'Consider changing the type-only import to a regular import, or use the @Inject decorator to specify an injection token.'; + hints = [ + makeRelatedInformation( + reason.typeNode, + 'This type is imported using a type-only import, which prevents it from being usable as an injection token.'), + makeRelatedInformation(reason.importClause, 'The type-only import occurs here.'), + ]; + break; + case ValueUnavailableKind.NAMESPACE: + chainMessage = 'Consider using the @Inject decorator to specify an injection token.'; + hints = [ + makeRelatedInformation( + reason.typeNode, + 'This type corresponds with a namespace, which cannot be used as injection token.'), + makeRelatedInformation(reason.importClause, 'The namespace import occurs here.'), + ]; + break; + case ValueUnavailableKind.UNKNOWN_REFERENCE: + chainMessage = 'The type should reference a known declaration.'; + hints = [makeRelatedInformation(reason.typeNode, 'This type could not be resolved.')]; + break; + case ValueUnavailableKind.MISSING_TYPE: + chainMessage = + 'Consider adding a type to the parameter or use the @Inject decorator to specify an injection token.'; + break; + } + + const chain: ts.DiagnosticMessageChain = { + messageText: `No suitable injection token for parameter '${param.name || index}' of class '${ + clazz.name.text}'.`, + category: ts.DiagnosticCategory.Error, + code: 0, + next: [{ + messageText: chainMessage, + category: ts.DiagnosticCategory.Message, + code: 0, + }], + }; + + return new FatalDiagnosticError(ErrorCode.PARAM_MISSING_TOKEN, param.nameNode, chain, hints); +} + export function toR3Reference( valueRef: Reference, typeRef: Reference, valueContext: ts.SourceFile, typeContext: ts.SourceFile, refEmitter: ReferenceEmitter): R3Reference { diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index bb46477213..3d28a12d31 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts @@ -224,25 +224,39 @@ export interface ClassMember { decorators: Decorator[]|null; } +export const enum TypeValueReferenceKind { + LOCAL, + IMPORTED, + UNAVAILABLE, +} + /** - * A reference to a value that originated from a type position. - * - * For example, a constructor parameter could be declared as `foo: Foo`. A `TypeValueReference` - * extracted from this would refer to the value of the class `Foo` (assuming it was actually a - * type). - * - * There are two kinds of such references. A reference with `local: false` refers to a type that was - * imported, and gives the symbol `name` and the `moduleName` of the import. Note that this - * `moduleName` may be a relative path, and thus is likely only valid within the context of the file - * which contained the original type reference. - * - * A reference with `local: true` refers to any other kind of type via a `ts.Expression` that's - * valid within the local file where the type was referenced. + * A type reference that refers to any type via a `ts.Expression` that's valid within the local file + * where the type was referenced. */ -export type TypeValueReference = { - local: true; expression: ts.Expression; defaultImportStatement: ts.ImportDeclaration | null; -}|{ - local: false; +export interface LocalTypeValueReference { + kind: TypeValueReferenceKind.LOCAL; + + /** + * The synthesized expression to reference the type in a value position. + */ + expression: ts.Expression; + + /** + * If the type originates from a default import, the import statement is captured here to be able + * to track its usages, preventing the import from being elided if it was originally only used in + * a type-position. See `DefaultImportTracker` for details. + */ + defaultImportStatement: ts.ImportDeclaration|null; +} + +/** + * A reference that refers to a type that was imported, and gives the symbol `name` and the + * `moduleName` of the import. Note that this `moduleName` may be a relative path, and thus is + * likely only valid within the context of the file which contained the original type reference. + */ +export interface ImportedTypeValueReference { + kind: TypeValueReferenceKind.IMPORTED; /** * The module specifier from which the `importedName` symbol should be imported. @@ -262,7 +276,107 @@ export type TypeValueReference = { nestedPath: string[]|null; valueDeclaration: ts.Declaration; -}; +} + +/** + * A representation for a type value reference that is used when no value is available. This can + * occur due to various reasons, which is indicated in the `reason` field. + */ +export interface UnavailableTypeValueReference { + kind: TypeValueReferenceKind.UNAVAILABLE; + + /** + * The reason why no value reference could be determined for a type. + */ + reason: UnavailableValue; +} + +/** + * The various reasons why the compiler may be unable to synthesize a value from a type reference. + */ +export const enum ValueUnavailableKind { + /** + * No type node was available. + */ + MISSING_TYPE, + + /** + * The type does not have a value declaration, e.g. an interface. + */ + NO_VALUE_DECLARATION, + + /** + * The type is imported using a type-only imports, so it is not suitable to be used in a + * value-position. + */ + TYPE_ONLY_IMPORT, + + /** + * The type reference could not be resolved to a declaration. + */ + UNKNOWN_REFERENCE, + + /** + * The type corresponds with a namespace. + */ + NAMESPACE, + + /** + * The type is not supported in the compiler, for example union types. + */ + UNSUPPORTED, +} + + +export interface UnsupportedType { + kind: ValueUnavailableKind.UNSUPPORTED; + typeNode: ts.TypeNode; +} + +export interface NoValueDeclaration { + kind: ValueUnavailableKind.NO_VALUE_DECLARATION; + typeNode: ts.TypeNode; + decl: ts.Declaration; +} + +export interface TypeOnlyImport { + kind: ValueUnavailableKind.TYPE_ONLY_IMPORT; + typeNode: ts.TypeNode; + importClause: ts.ImportClause; +} + +export interface NamespaceImport { + kind: ValueUnavailableKind.NAMESPACE; + typeNode: ts.TypeNode; + importClause: ts.ImportClause; +} + +export interface UnknownReference { + kind: ValueUnavailableKind.UNKNOWN_REFERENCE; + typeNode: ts.TypeNode; +} + +export interface MissingType { + kind: ValueUnavailableKind.MISSING_TYPE; +} + +/** + * The various reasons why a type node may not be referred to as a value. + */ +export type UnavailableValue = + UnsupportedType|NoValueDeclaration|TypeOnlyImport|NamespaceImport|UnknownReference|MissingType; + +/** + * A reference to a value that originated from a type position. + * + * For example, a constructor parameter could be declared as `foo: Foo`. A `TypeValueReference` + * extracted from this would refer to the value of the class `Foo` (assuming it was actually a + * type). + * + * See the individual types for additional information. + */ +export type TypeValueReference = + LocalTypeValueReference|ImportedTypeValueReference|UnavailableTypeValueReference; /** * A parameter to a constructor. @@ -288,14 +402,10 @@ export interface CtorParameter { * Reference to the value of the parameter's type annotation, if it's possible to refer to the * parameter's type as a value. * - * This can either be a reference to a local value, in which case it has `local` set to `true` and - * contains a `ts.Expression`, or it's a reference to an imported value, in which case `local` is - * set to `false` and the symbol and module name of the imported value are provided instead. - * - * If the type is not present or cannot be represented as an expression, `typeValueReference` is - * `null`. + * This can either be a reference to a local value, a reference to an imported value, or no + * value if no is present or cannot be represented as an expression. */ - typeValueReference: TypeValueReference|null; + typeValueReference: TypeValueReference; /** * TypeScript `ts.TypeNode` representing the type node found in the type position. diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts index 8b71bb3f49..acef675409 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {TypeValueReference} from './host'; +import {TypeValueReference, TypeValueReferenceKind, UnavailableTypeValueReference, ValueUnavailableKind} from './host'; /** * Potentially convert a `ts.TypeNode` to a `TypeValueReference`, which indicates how to use the @@ -18,22 +18,26 @@ import {TypeValueReference} from './host'; * declaration, or if it is not possible to statically understand. */ export function typeToValue( - typeNode: ts.TypeNode|null, checker: ts.TypeChecker): TypeValueReference|null { + typeNode: ts.TypeNode|null, checker: ts.TypeChecker): TypeValueReference { // It's not possible to get a value expression if the parameter doesn't even have a type. - if (typeNode === null || !ts.isTypeReferenceNode(typeNode)) { - return null; + if (typeNode === null) { + return missingType(); + } + + if (!ts.isTypeReferenceNode(typeNode)) { + return unsupportedType(typeNode); } const symbols = resolveTypeSymbols(typeNode, checker); if (symbols === null) { - return null; + return unknownReference(typeNode); } const {local, decl} = symbols; // It's only valid to convert a type reference to a value reference if the type actually // has a value declaration associated with it. if (decl.valueDeclaration === undefined) { - return null; + return noValueDeclaration(typeNode, decl.declarations[0]); } // The type points to a valid value declaration. Rewrite the TypeReference into an @@ -47,8 +51,13 @@ export function typeToValue( // This is a default import. // import Foo from 'foo'; + if (firstDecl.isTypeOnly) { + // Type-only imports cannot be represented as value. + return typeOnlyImport(typeNode, firstDecl); + } + return { - local: true, + kind: TypeValueReferenceKind.LOCAL, // Copying the name here ensures the generated references will be correctly transformed // along with the import. expression: ts.updateIdentifier(firstDecl.name), @@ -60,6 +69,11 @@ export function typeToValue( // or // import {Foo as Bar} from 'foo'; + if (firstDecl.parent.parent.isTypeOnly) { + // Type-only imports cannot be represented as value. + return typeOnlyImport(typeNode, firstDecl.parent.parent); + } + // Determine the name to import (`Foo`) from the import specifier, as the symbol names of // the imported type could refer to a local alias (like `Bar` in the example above). const importedName = (firstDecl.propertyName || firstDecl.name).text; @@ -70,7 +84,7 @@ export function typeToValue( const moduleName = extractModuleName(firstDecl.parent.parent.parent); return { - local: false, + kind: TypeValueReferenceKind.IMPORTED, valueDeclaration: decl.valueDeclaration, moduleName, importedName, @@ -80,9 +94,14 @@ export function typeToValue( // The import is a namespace import // import * as Foo from 'foo'; + if (firstDecl.parent.isTypeOnly) { + // Type-only imports cannot be represented as value. + return typeOnlyImport(typeNode, firstDecl.parent); + } + if (symbols.symbolNames.length === 1) { // The type refers to the namespace itself, which cannot be represented as a value. - return null; + return namespaceImport(typeNode, firstDecl.parent); } // The first symbol name refers to the local name of the namespace, which is is discarded @@ -92,7 +111,7 @@ export function typeToValue( const moduleName = extractModuleName(firstDecl.parent.parent); return { - local: false, + kind: TypeValueReferenceKind.IMPORTED, valueDeclaration: decl.valueDeclaration, moduleName, importedName, @@ -105,15 +124,60 @@ export function typeToValue( const expression = typeNodeToValueExpr(typeNode); if (expression !== null) { return { - local: true, + kind: TypeValueReferenceKind.LOCAL, expression, defaultImportStatement: null, }; } else { - return null; + return unsupportedType(typeNode); } } +function unsupportedType(typeNode: ts.TypeNode): UnavailableTypeValueReference { + return { + kind: TypeValueReferenceKind.UNAVAILABLE, + reason: {kind: ValueUnavailableKind.UNSUPPORTED, typeNode}, + }; +} + +function noValueDeclaration( + typeNode: ts.TypeNode, decl: ts.Declaration): UnavailableTypeValueReference { + return { + kind: TypeValueReferenceKind.UNAVAILABLE, + reason: {kind: ValueUnavailableKind.NO_VALUE_DECLARATION, typeNode, decl}, + }; +} + +function typeOnlyImport( + typeNode: ts.TypeNode, importClause: ts.ImportClause): UnavailableTypeValueReference { + return { + kind: TypeValueReferenceKind.UNAVAILABLE, + reason: {kind: ValueUnavailableKind.TYPE_ONLY_IMPORT, typeNode, importClause}, + }; +} + +function unknownReference(typeNode: ts.TypeNode): UnavailableTypeValueReference { + return { + kind: TypeValueReferenceKind.UNAVAILABLE, + reason: {kind: ValueUnavailableKind.UNKNOWN_REFERENCE, typeNode}, + }; +} + +function namespaceImport( + typeNode: ts.TypeNode, importClause: ts.ImportClause): UnavailableTypeValueReference { + return { + kind: TypeValueReferenceKind.UNAVAILABLE, + reason: {kind: ValueUnavailableKind.NAMESPACE, typeNode, importClause}, + }; +} + +function missingType(): UnavailableTypeValueReference { + return { + kind: TypeValueReferenceKind.UNAVAILABLE, + reason: {kind: ValueUnavailableKind.MISSING_TYPE}, + }; +} + /** * Attempt to extract a `ts.Expression` that's equivalent to a `ts.TypeNode`, as the two have * different AST shapes but can reference the same symbols. diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts index 56b54b24dc..fa8626c78a 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts @@ -68,8 +68,6 @@ export class TypeScriptReflectionHost implements ReflectionHost { if (childTypeNodes.length === 1) { typeNode = childTypeNodes[0]; - } else { - typeNode = null; } } diff --git a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts index 54ce4125d2..f4cfd540ac 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getSourceFileOrError} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; import {getDeclaration, makeProgram} from '../../testing'; -import {ClassMember, ClassMemberKind, CtorParameter} from '../src/host'; +import {ClassMember, ClassMemberKind, CtorParameter, TypeValueReferenceKind} from '../src/host'; import {TypeScriptReflectionHost} from '../src/typescript'; import {isNamedClassDeclaration} from '../src/util'; @@ -178,7 +178,7 @@ runInEachFileSystem(() => { const args = host.getConstructorParameters(clazz)!; expect(args.length).toBe(1); const param = args[0].typeValueReference; - if (param === null || !param.local) { + if (param === null || param.kind !== TypeValueReferenceKind.LOCAL) { return fail('Expected local parameter'); } expect(param).not.toBeNull(); @@ -548,17 +548,20 @@ runInEachFileSystem(() => { if (type === undefined) { expect(param.typeValueReference).toBeNull(); } else { - if (param.typeValueReference === null) { + if (param.typeValueReference.kind === TypeValueReferenceKind.UNAVAILABLE) { return fail(`Expected parameter ${name} to have a typeValueReference`); } - if (param.typeValueReference.local && typeof type === 'string') { + if (param.typeValueReference.kind === TypeValueReferenceKind.LOCAL && + typeof type === 'string') { expect(argExpressionToString(param.typeValueReference.expression)).toEqual(type); - } else if (!param.typeValueReference.local && typeof type !== 'string') { + } else if ( + param.typeValueReference.kind === TypeValueReferenceKind.IMPORTED && + typeof type !== 'string') { expect(param.typeValueReference.moduleName).toEqual(type.moduleName); expect(param.typeValueReference.importedName).toEqual(type.name); } else { return fail(`Mismatch between typeValueReference and expected type: ${param.name} / ${ - param.typeValueReference.local}`); + param.typeValueReference.kind}`); } } if (decorator !== undefined) { diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 5ae96b6e5c..58575c1a4e 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2079,7 +2079,13 @@ runInEachFileSystem(os => { const errors = env.driveDiagnostics(); expect(errors.length).toBe(1); - expect(errors[0].messageText).toContain('No suitable injection token for parameter'); + expect(ts.flattenDiagnosticMessageText(errors[0].messageText, '\n')) + .toBe( + `No suitable injection token for parameter 'notInjectable' of class 'Test'.\n` + + ` Consider using the @Inject decorator to specify an injection token.`); + expect(errors[0].relatedInformation!.length).toBe(1); + expect(errors[0].relatedInformation![0].messageText) + .toBe('This type is not supported as injection token.'); }); it('should give a compile-time error if an invalid @Injectable is used with an argument', @@ -2096,9 +2102,139 @@ runInEachFileSystem(os => { const errors = env.driveDiagnostics(); expect(errors.length).toBe(1); - expect(errors[0].messageText).toContain('No suitable injection token for parameter'); + expect(ts.flattenDiagnosticMessageText(errors[0].messageText, '\n')) + .toBe( + `No suitable injection token for parameter 'notInjectable' of class 'Test'.\n` + + ` Consider using the @Inject decorator to specify an injection token.`); + expect(errors[0].relatedInformation!.length).toBe(1); + expect(errors[0].relatedInformation![0].messageText) + .toBe('This type is not supported as injection token.'); }); + it('should report an error when using a type-only import as injection token', () => { + env.tsconfig({strictInjectionParameters: true}); + env.write(`types.ts`, ` + export class TypeOnly {} + `); + env.write(`test.ts`, ` + import {Injectable} from '@angular/core'; + import type {TypeOnly} from './types'; + + @Injectable() + export class MyService { + constructor(param: TypeOnly) {} + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) + .toBe( + `No suitable injection token for parameter 'param' of class 'MyService'.\n` + + ` Consider changing the type-only import to a regular import, ` + + `or use the @Inject decorator to specify an injection token.`); + expect(diags[0].relatedInformation!.length).toBe(2); + expect(diags[0].relatedInformation![0].messageText) + .toBe( + 'This type is imported using a type-only import, ' + + 'which prevents it from being usable as an injection token.'); + expect(diags[0].relatedInformation![1].messageText) + .toBe('The type-only import occurs here.'); + }); + + it('should report an error when using a primitive type as injection token', () => { + env.tsconfig({strictInjectionParameters: true}); + env.write(`test.ts`, ` + import {Injectable} from '@angular/core'; + + @Injectable() + export class MyService { + constructor(param: string) {} + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) + .toBe( + `No suitable injection token for parameter 'param' of class 'MyService'.\n` + + ` Consider using the @Inject decorator to specify an injection token.`); + expect(diags[0].relatedInformation!.length).toBe(1); + expect(diags[0].relatedInformation![0].messageText) + .toBe('This type is not supported as injection token.'); + }); + + it('should report an error when using a union type as injection token', () => { + env.tsconfig({strictInjectionParameters: true}); + env.write(`test.ts`, ` + import {Injectable} from '@angular/core'; + + export class ClassA {} + export class ClassB {} + + @Injectable() + export class MyService { + constructor(param: ClassA|ClassB) {} + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) + .toBe( + `No suitable injection token for parameter 'param' of class 'MyService'.\n` + + ` Consider using the @Inject decorator to specify an injection token.`); + expect(diags[0].relatedInformation!.length).toBe(1); + expect(diags[0].relatedInformation![0].messageText) + .toBe('This type is not supported as injection token.'); + }); + + it('should report an error when using an interface as injection token', () => { + env.tsconfig({strictInjectionParameters: true}); + env.write(`test.ts`, ` + import {Injectable} from '@angular/core'; + + export interface Interface {} + + @Injectable() + export class MyService { + constructor(param: Interface) {} + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) + .toBe( + `No suitable injection token for parameter 'param' of class 'MyService'.\n` + + ` Consider using the @Inject decorator to specify an injection token.`); + expect(diags[0].relatedInformation!.length).toBe(2); + expect(diags[0].relatedInformation![0].messageText) + .toBe('This type does not have a value, so it cannot be used as injection token.'); + expect(diags[0].relatedInformation![1].messageText).toBe('The type is declared here.'); + }); + + it('should report an error when no type is present', () => { + env.tsconfig({strictInjectionParameters: true, noImplicitAny: false}); + env.write(`test.ts`, ` + import {Injectable} from '@angular/core'; + + @Injectable() + export class MyService { + constructor(param) {} + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) + .toBe( + `No suitable injection token for parameter 'param' of class 'MyService'.\n` + + ` Consider adding a type to the parameter or ` + + `use the @Inject decorator to specify an injection token.`); + expect(diags[0].relatedInformation).toBeUndefined(); + }); + it('should not give a compile-time error if an invalid @Injectable is used with useValue', () => { env.tsconfig({strictInjectionParameters: true}); @@ -2240,7 +2376,8 @@ runInEachFileSystem(os => { const errors = env.driveDiagnostics(); expect(errors.length).toBe(1); - expect(errors[0].messageText).toContain('No suitable injection token for parameter'); + expect(ts.flattenDiagnosticMessageText(errors[0].messageText, '\n')) + .toContain('No suitable injection token for parameter'); }); }); @@ -4194,9 +4331,16 @@ runInEachFileSystem(os => { const diags = env.driveDiagnostics(); expect(diags.length).toBe(1); - expect(diags[0].messageText) + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) .toBe( - `No suitable injection token for parameter 'foo' of class 'MyService'.\nFound Foo`); + `No suitable injection token for parameter 'foo' of class 'MyService'.\n` + + ` Consider using the @Inject decorator to specify an injection token.`); + expect(diags[0].relatedInformation!.length).toBe(2); + expect(diags[0].relatedInformation![0].messageText) + .toBe( + 'This type corresponds with a namespace, which cannot be used as injection token.'); + expect(diags[0].relatedInformation![1].messageText) + .toBe('The namespace import occurs here.'); }); }); @@ -4225,6 +4369,43 @@ runInEachFileSystem(os => { expect(jsContents).toMatch(setClassMetadataRegExp('type: undefined')); }); + it('should use `undefined` in setClassMetadata if types originate from type-only imports', + () => { + env.write(`types.ts`, ` + export default class {} + export class TypeOnly {} + `); + env.write(`test.ts`, ` + import {Component, Inject, Injectable} from '@angular/core'; + import type DefaultImport from './types'; + import type {TypeOnly} from './types'; + import type * as types from './types'; + + @Component({ + selector: 'some-comp', + template: '...', + }) + export class SomeComp { + constructor( + @Inject('token') namedImport: TypeOnly, + @Inject('token') defaultImport: DefaultImport, + @Inject('token') namespacedImport: types.TypeOnly, + ) {} + } + `); + + env.driveMain(); + const jsContents = trim(env.getContents('test.js')); + // Module specifier for type-only import should not be emitted + expect(jsContents).not.toContain('./types'); + // Default type-only import should not be emitted + expect(jsContents).not.toContain('DefaultImport'); + // Named type-only import should not be emitted + expect(jsContents).not.toContain('TypeOnly'); + // The parameter type in class metadata should be undefined + expect(jsContents).toMatch(setClassMetadataRegExp('type: undefined')); + }); + it('should not throw in case whitespaces and HTML comments are present inside ', () => { env.write('test.ts', `