From 8f73169979274eea61af52b68758a38a470bc65b Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Thu, 16 Jul 2020 15:23:30 -0700 Subject: [PATCH] refactor(compiler-cli): add `TemplateId` to template diagnostics (#38105) Previously, a stable template id was implemented for each component in a file. This commit adds this id to each `TemplateDiagnostic` generated from the template type-checker, so it can potentially be used for filtration. PR Close #38105 --- .../src/ngtsc/typecheck/src/context.ts | 9 ++-- .../src/ngtsc/typecheck/src/diagnostics.ts | 17 +++++-- .../src/ngtsc/typecheck/src/dom.ts | 12 ++--- .../src/ngtsc/typecheck/src/oob.ts | 47 ++++++++++++------- .../src/ngtsc/typecheck/test/test_utils.ts | 5 +- 5 files changed, 57 insertions(+), 33 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 5c5bb0c2f7..9bd6897fde 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -15,6 +15,7 @@ import {ClassDeclaration, ReflectionHost} from '../../reflection'; import {ImportManager} from '../../translator'; import {ComponentToShimMappingStrategy, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api'; +import {TemplateDiagnostic} from './diagnostics'; import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom'; import {Environment} from './environment'; import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob'; @@ -34,7 +35,7 @@ export interface ShimTypeCheckingData { * * Some diagnostics are produced during creation time and are tracked here. */ - genesisDiagnostics: ts.Diagnostic[]; + genesisDiagnostics: TemplateDiagnostic[]; /** * Whether any inline operations for the input file were required to generate this shim. @@ -182,7 +183,6 @@ export class TypeCheckContextImpl implements TypeCheckContext { } const sfPath = absoluteFromSourceFile(ref.node.getSourceFile()); - const overrideTemplate = this.host.getTemplateOverride(sfPath, ref.node); if (overrideTemplate !== null) { template = overrideTemplate; @@ -231,12 +231,13 @@ export class TypeCheckContextImpl implements TypeCheckContext { // and inlining would be required. // Record diagnostics to indicate the issues with this template. + const templateId = fileData.sourceManager.getTemplateId(ref.node); if (tcbRequiresInline) { - shimData.oobRecorder.requiresInlineTcb(ref.node); + shimData.oobRecorder.requiresInlineTcb(templateId, ref.node); } if (missingInlines.length > 0) { - shimData.oobRecorder.requiresInlineTypeConstructors(ref.node, missingInlines); + shimData.oobRecorder.requiresInlineTypeConstructors(templateId, ref.node, missingInlines); } // Checking this template would be unsupported, so don't try. diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts index 584dabd984..2b61b1bc39 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts @@ -21,6 +21,11 @@ export interface TemplateDiagnostic extends ts.Diagnostic { * The component with the template that resulted in this diagnostic. */ componentFile: ts.SourceFile; + + /** + * The template id of the component that resulted in this diagnostic. + */ + templateId: TemplateId; } /** @@ -119,7 +124,7 @@ export function shouldReportDiagnostic(diagnostic: ts.Diagnostic): boolean { * file from being reported as type-check errors. */ export function translateDiagnostic( - diagnostic: ts.Diagnostic, resolver: TemplateSourceResolver): ts.Diagnostic|null { + diagnostic: ts.Diagnostic, resolver: TemplateSourceResolver): TemplateDiagnostic|null { if (diagnostic.file === undefined || diagnostic.start === undefined) { return null; } @@ -139,7 +144,8 @@ export function translateDiagnostic( const mapping = resolver.getSourceMapping(sourceLocation.id); return makeTemplateDiagnostic( - mapping, span, diagnostic.category, diagnostic.code, diagnostic.messageText); + sourceLocation.id, mapping, span, diagnostic.category, diagnostic.code, + diagnostic.messageText); } export function findTypeCheckBlock(file: ts.SourceFile, id: TemplateId): ts.Node|null { @@ -155,8 +161,9 @@ export function findTypeCheckBlock(file: ts.SourceFile, id: TemplateId): ts.Node * Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template. */ export function makeTemplateDiagnostic( - mapping: TemplateSourceMapping, span: ParseSourceSpan, category: ts.DiagnosticCategory, - code: number, messageText: string|ts.DiagnosticMessageChain, relatedMessage?: { + templateId: TemplateId, mapping: TemplateSourceMapping, span: ParseSourceSpan, + category: ts.DiagnosticCategory, code: number, messageText: string|ts.DiagnosticMessageChain, + relatedMessage?: { text: string, span: ParseSourceSpan, }): TemplateDiagnostic { @@ -182,6 +189,7 @@ export function makeTemplateDiagnostic( messageText, file: mapping.node.getSourceFile(), componentFile: mapping.node.getSourceFile(), + templateId, start: span.start.offset, length: span.end.offset - span.start.offset, relatedInformation, @@ -233,6 +241,7 @@ export function makeTemplateDiagnostic( messageText, file: sf, componentFile: componentSf, + templateId, start: span.start.offset, length: span.end.offset - span.start.offset, // Show a secondary message indicating the component whose template contains the error. diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts index 3a4f43af3a..c6b39be34d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts @@ -12,7 +12,7 @@ import * as ts from 'typescript'; import {ErrorCode, ngErrorCode} from '../../diagnostics'; import {TemplateId} from '../api'; -import {makeTemplateDiagnostic, TemplateSourceResolver} from './diagnostics'; +import {makeTemplateDiagnostic, TemplateDiagnostic, TemplateSourceResolver} from './diagnostics'; const REGISTRY = new DomElementSchemaRegistry(); const REMOVE_XHTML_REGEX = /^:xhtml:/; @@ -31,7 +31,7 @@ export interface DomSchemaChecker { * Get the `ts.Diagnostic`s that have been generated via `checkElement` and `checkProperty` calls * thus far. */ - readonly diagnostics: ReadonlyArray; + readonly diagnostics: ReadonlyArray; /** * Check a non-Angular element and record any diagnostics about it. @@ -64,9 +64,9 @@ export interface DomSchemaChecker { * maintained by the Angular team via extraction from a browser IDL. */ export class RegistryDomSchemaChecker implements DomSchemaChecker { - private _diagnostics: ts.Diagnostic[] = []; + private _diagnostics: TemplateDiagnostic[] = []; - get diagnostics(): ReadonlyArray { + get diagnostics(): ReadonlyArray { return this._diagnostics; } @@ -93,7 +93,7 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker { } const diag = makeTemplateDiagnostic( - mapping, element.sourceSpan, ts.DiagnosticCategory.Error, + id, mapping, element.sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.SCHEMA_INVALID_ELEMENT), errorMsg); this._diagnostics.push(diag); } @@ -123,7 +123,7 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker { } const diag = makeTemplateDiagnostic( - mapping, span, ts.DiagnosticCategory.Error, + id, mapping, span, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.SCHEMA_INVALID_ATTRIBUTE), errorMsg); this._diagnostics.push(diag); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index 1e728be7c3..0d9ab782a6 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -13,7 +13,7 @@ import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '.. import {ClassDeclaration} from '../../reflection'; import {TemplateId} from '../api'; -import {makeTemplateDiagnostic, TemplateSourceResolver} from './diagnostics'; +import {makeTemplateDiagnostic, TemplateDiagnostic, TemplateSourceResolver} from './diagnostics'; @@ -27,7 +27,7 @@ import {makeTemplateDiagnostic, TemplateSourceResolver} from './diagnostics'; * recorder for later display. */ export interface OutOfBandDiagnosticRecorder { - readonly diagnostics: ReadonlyArray; + readonly diagnostics: ReadonlyArray; /** * Reports a `#ref="target"` expression in the template for which a target directive could not be @@ -63,17 +63,18 @@ export interface OutOfBandDiagnosticRecorder { duplicateTemplateVar( templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void; - requiresInlineTcb(node: ClassDeclaration): void; + requiresInlineTcb(templateId: TemplateId, node: ClassDeclaration): void; - requiresInlineTypeConstructors(node: ClassDeclaration, directives: ClassDeclaration[]): void; + requiresInlineTypeConstructors( + templateId: TemplateId, node: ClassDeclaration, directives: ClassDeclaration[]): void; } export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder { - private _diagnostics: ts.Diagnostic[] = []; + private _diagnostics: TemplateDiagnostic[] = []; constructor(private resolver: TemplateSourceResolver) {} - get diagnostics(): ReadonlyArray { + get diagnostics(): ReadonlyArray { return this._diagnostics; } @@ -83,7 +84,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor const errorMsg = `No directive found with exportAs '${value}'.`; this._diagnostics.push(makeTemplateDiagnostic( - mapping, ref.valueSpan || ref.sourceSpan, ts.DiagnosticCategory.Error, + templateId, mapping, ref.valueSpan || ref.sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.MISSING_REFERENCE_TARGET), errorMsg)); } @@ -97,8 +98,8 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor `Assertion failure: no SourceLocation found for usage of pipe '${ast.name}'.`); } this._diagnostics.push(makeTemplateDiagnostic( - mapping, sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.MISSING_PIPE), - errorMsg)); + templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error, + ngErrorCode(ErrorCode.MISSING_PIPE), errorMsg)); } illegalAssignmentToTemplateVar( @@ -113,7 +114,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor throw new Error(`Assertion failure: no SourceLocation found for property binding.`); } this._diagnostics.push(makeTemplateDiagnostic( - mapping, sourceSpan, ts.DiagnosticCategory.Error, + templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.WRITE_TO_READ_ONLY_VARIABLE), errorMsg, { text: `The variable ${assignment.name} is declared here.`, span: target.valueSpan || target.sourceSpan, @@ -132,20 +133,21 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor // // TODO(alxhub): allocate to a tighter span once one is available. this._diagnostics.push(makeTemplateDiagnostic( - mapping, variable.sourceSpan, ts.DiagnosticCategory.Error, + templateId, mapping, variable.sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION), errorMsg, { text: `The variable '${firstDecl.name}' was first declared here.`, span: firstDecl.sourceSpan, })); } - requiresInlineTcb(node: ClassDeclaration): void { - this._diagnostics.push(makeDiagnostic( - ErrorCode.INLINE_TCB_REQUIRED, node.name, + requiresInlineTcb(templateId: TemplateId, node: ClassDeclaration): void { + this._diagnostics.push(makeInlineDiagnostic( + templateId, ErrorCode.INLINE_TCB_REQUIRED, node.name, `This component requires inline template type-checking, which is not supported by the current environment.`)); } - requiresInlineTypeConstructors(node: ClassDeclaration, directives: ClassDeclaration[]): void { + requiresInlineTypeConstructors( + templateId: TemplateId, node: ClassDeclaration, directives: ClassDeclaration[]): void { let message: string; if (directives.length > 1) { message = @@ -155,9 +157,20 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor `This component uses a directive which requires an inline type constructor, which is not supported by the current environment.`; } - this._diagnostics.push(makeDiagnostic( - ErrorCode.INLINE_TYPE_CTOR_REQUIRED, node.name, message, + this._diagnostics.push(makeInlineDiagnostic( + templateId, ErrorCode.INLINE_TYPE_CTOR_REQUIRED, node.name, message, directives.map( dir => makeRelatedInformation(dir.name, `Requires an inline type constructor.`)))); } } + +function makeInlineDiagnostic( + templateId: TemplateId, code: ErrorCode.INLINE_TCB_REQUIRED|ErrorCode.INLINE_TYPE_CTOR_REQUIRED, + node: ts.Node, messageText: string|ts.DiagnosticMessageChain, + relatedInformation?: ts.DiagnosticRelatedInformation[]): TemplateDiagnostic { + return { + ...makeDiagnostic(code, node, messageText, relatedInformation), + componentFile: node.getSourceFile(), + templateId, + }; +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index a0dee2a118..99043a6428 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -20,6 +20,7 @@ import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckContext} from '.. import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckingConfig, UpdateMode} from '../api/api'; import {ReusedProgramStrategy} from '../src/augmented_program'; import {TemplateTypeCheckerImpl} from '../src/checker'; +import {TemplateDiagnostic} from '../src/diagnostics'; import {DomSchemaChecker} from '../src/dom'; import {Environment} from '../src/environment'; import {OutOfBandDiagnosticRecorder} from '../src/oob'; @@ -487,7 +488,7 @@ class FakeEnvironment /* implements Environment */ { } export class NoopSchemaChecker implements DomSchemaChecker { - get diagnostics(): ReadonlyArray { + get diagnostics(): ReadonlyArray { return []; } @@ -498,7 +499,7 @@ export class NoopSchemaChecker implements DomSchemaChecker { } export class NoopOobRecorder implements OutOfBandDiagnosticRecorder { - get diagnostics(): ReadonlyArray { + get diagnostics(): ReadonlyArray { return []; } missingReferenceTarget(): void {}