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 7e43c198ee..6b5b356311 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -22,7 +22,7 @@ import {getSourceFile} from '../../util/src/typescript'; import {generateSetClassMetadataCall} from './metadata'; import {ReferencesRegistry} from './references_registry'; -import {combineResolvers, findAngularDecorator, forwardRefResolver, getReferenceOriginForDiagnostics, getValidConstructorDependencies, isExpressionForwardReference, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from './util'; +import {combineResolvers, findAngularDecorator, forwardRefResolver, getValidConstructorDependencies, isExpressionForwardReference, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from './util'; export interface NgModuleAnalysis { mod: R3NgModuleMetadata; @@ -119,7 +119,7 @@ export class NgModuleDecoratorHandler implements // Look through the declarations to make sure they're all a part of the current compilation. for (const ref of declarationRefs) { if (ref.node.getSourceFile().isDeclarationFile) { - const errorNode: ts.Expression = getReferenceOriginForDiagnostics(ref, rawDeclarations); + const errorNode: ts.Expression = ref.getOriginForDiagnostics(rawDeclarations); diagnostics.push(makeDiagnostic( ErrorCode.NGMODULE_INVALID_DECLARATION, errorNode, @@ -317,24 +317,6 @@ export class NgModuleDecoratorHandler implements injectorImports: [], }; - for (const decl of analysis.declarations) { - if (this.metaReader.getDirectiveMetadata(decl) === null && - this.metaReader.getPipeMetadata(decl) === null) { - // This declaration is neither a directive(or component) nor a pipe, so produce a diagnostic - // for it. - - // Locate the error on the 'declarations' field of the NgModule decorator to start. - const errorNode: ts.Expression = - getReferenceOriginForDiagnostics(decl, analysis.rawDeclarations !); - diagnostics.push(makeDiagnostic( - ErrorCode.NGMODULE_INVALID_DECLARATION, errorNode, - `The class '${decl.node.name.text}' is listed in the declarations of the NgModule '${node.name.text}', but is not a directive, a component, or a pipe. - -Either remove it from the NgModule's declarations, or add an appropriate Angular decorator.`, - [{node: decl.node.name, messageText: `'${decl.node.name.text}' is declared here.`}])); - } - } - if (scope !== null) { // Using the scope information, extend the injector's imports using the modules that are // specified as module exports. diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts index 989275dfbf..3d730de347 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts @@ -378,33 +378,6 @@ export function wrapFunctionExpressionsInParens(expression: ts.Expression): ts.E return ts.transform(expression, [parensWrapperTransformerFactory]).transformed[0]; } -/** - * Given a 'container' expression and a `Reference` extracted from that container, produce a - * `ts.Expression` to use in a diagnostic which best indicates the position within the container - * expression that generated the `Reference`. - * - * For example, given a `Reference` to the class 'Bar' and the containing expression: - * `[Foo, Bar, Baz]`, this function would attempt to return the `ts.Identifier` for `Bar` within the - * array. This could be used to produce a nice diagnostic context: - * - * ```text - * [Foo, Bar, Baz] - * ~~~ - * ``` - * - * If no specific node can be found, then the `fallback` expression is used, which defaults to the - * entire containing expression. - */ -export function getReferenceOriginForDiagnostics( - ref: Reference, container: ts.Expression, fallback: ts.Expression = container): ts.Expression { - const id = ref.getIdentityInExpression(container); - if (id !== null) { - return id; - } - - return fallback; -} - /** * Create a `ts.Diagnostic` which indicates the given class is part of the declarations of two or * more NgModules. @@ -421,8 +394,7 @@ export function makeDuplicateDeclarationError( } // Try to find the reference to the declaration within the declarations array, to hang the // error there. If it can't be found, fall back on using the NgModule's name. - const contextNode = - getReferenceOriginForDiagnostics(decl.ref, decl.rawDeclarations, decl.ngModule.name); + const contextNode = decl.ref.getOriginForDiagnostics(decl.rawDeclarations, decl.ngModule.name); context.push({ node: contextNode, messageText: diff --git a/packages/compiler-cli/src/ngtsc/imports/src/references.ts b/packages/compiler-cli/src/ngtsc/imports/src/references.ts index f50dd581b3..f2432d9c3f 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/references.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/references.ts @@ -135,6 +135,29 @@ export class Reference { null; } + /** + * Given the 'container' expression from which this `Reference` was extracted, produce a + * `ts.Expression` to use in a diagnostic which best indicates the position within the container + * expression that generated the `Reference`. + * + * For example, given a `Reference` to the class 'Bar' and the containing expression: + * `[Foo, Bar, Baz]`, this function would attempt to return the `ts.Identifier` for `Bar` within + * the array. This could be used to produce a nice diagnostic context: + * + * ```text + * [Foo, Bar, Baz] + * ~~~ + * ``` + * + * If no specific node can be found, then the `fallback` expression is used, which defaults to the + * entire containing expression. + */ + getOriginForDiagnostics(container: ts.Expression, fallback: ts.Expression = container): + ts.Expression { + const id = this.getIdentityInExpression(container); + return id !== null ? id : fallback; + } + cloneWithAlias(alias: Expression): Reference { const ref = new Reference(this.node, this.bestGuessOwningModule); ref.identifiers = [...this.identifiers]; diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index 7d1d1f4a9e..b55cca14e7 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -298,8 +298,13 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop } else if (pipe !== null) { compilationPipes.set(decl.node, {...pipe, ref: decl}); } else { - // TODO(alxhub): produce a ts.Diagnostic. This can't be an error right now since some - // ngtools tests rely on analysis of broken components. + const errorNode = decl.getOriginForDiagnostics(ngModule.rawDeclarations !); + diagnostics.push(makeDiagnostic( + ErrorCode.NGMODULE_INVALID_DECLARATION, errorNode, + `The class '${decl.node.name.text}' is listed in the declarations of the NgModule '${ngModule.ref.node.name.text}', but is not a directive, a component, or a pipe. + +Either remove it from the NgModule's declarations, or add an appropriate Angular decorator.`, + [{node: decl.node.name, messageText: `'${decl.node.name.text}' is declared here.`}])); continue; }