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
This commit is contained in:
Alex Rickabaugh 2020-07-16 15:23:30 -07:00 committed by Misko Hevery
parent 3c0424e7e0
commit 8f73169979
5 changed files with 57 additions and 33 deletions

View File

@ -15,6 +15,7 @@ import {ClassDeclaration, ReflectionHost} from '../../reflection';
import {ImportManager} from '../../translator'; import {ImportManager} from '../../translator';
import {ComponentToShimMappingStrategy, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api'; import {ComponentToShimMappingStrategy, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api';
import {TemplateDiagnostic} from './diagnostics';
import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom'; import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom';
import {Environment} from './environment'; import {Environment} from './environment';
import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob'; import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob';
@ -34,7 +35,7 @@ export interface ShimTypeCheckingData {
* *
* Some diagnostics are produced during creation time and are tracked here. * 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. * 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 sfPath = absoluteFromSourceFile(ref.node.getSourceFile());
const overrideTemplate = this.host.getTemplateOverride(sfPath, ref.node); const overrideTemplate = this.host.getTemplateOverride(sfPath, ref.node);
if (overrideTemplate !== null) { if (overrideTemplate !== null) {
template = overrideTemplate; template = overrideTemplate;
@ -231,12 +231,13 @@ export class TypeCheckContextImpl implements TypeCheckContext {
// and inlining would be required. // and inlining would be required.
// Record diagnostics to indicate the issues with this template. // Record diagnostics to indicate the issues with this template.
const templateId = fileData.sourceManager.getTemplateId(ref.node);
if (tcbRequiresInline) { if (tcbRequiresInline) {
shimData.oobRecorder.requiresInlineTcb(ref.node); shimData.oobRecorder.requiresInlineTcb(templateId, ref.node);
} }
if (missingInlines.length > 0) { 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. // Checking this template would be unsupported, so don't try.

View File

@ -21,6 +21,11 @@ export interface TemplateDiagnostic extends ts.Diagnostic {
* The component with the template that resulted in this diagnostic. * The component with the template that resulted in this diagnostic.
*/ */
componentFile: ts.SourceFile; 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. * file from being reported as type-check errors.
*/ */
export function translateDiagnostic( 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) { if (diagnostic.file === undefined || diagnostic.start === undefined) {
return null; return null;
} }
@ -139,7 +144,8 @@ export function translateDiagnostic(
const mapping = resolver.getSourceMapping(sourceLocation.id); const mapping = resolver.getSourceMapping(sourceLocation.id);
return makeTemplateDiagnostic( 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 { 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. * Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template.
*/ */
export function makeTemplateDiagnostic( export function makeTemplateDiagnostic(
mapping: TemplateSourceMapping, span: ParseSourceSpan, category: ts.DiagnosticCategory, templateId: TemplateId, mapping: TemplateSourceMapping, span: ParseSourceSpan,
code: number, messageText: string|ts.DiagnosticMessageChain, relatedMessage?: { category: ts.DiagnosticCategory, code: number, messageText: string|ts.DiagnosticMessageChain,
relatedMessage?: {
text: string, text: string,
span: ParseSourceSpan, span: ParseSourceSpan,
}): TemplateDiagnostic { }): TemplateDiagnostic {
@ -182,6 +189,7 @@ export function makeTemplateDiagnostic(
messageText, messageText,
file: mapping.node.getSourceFile(), file: mapping.node.getSourceFile(),
componentFile: mapping.node.getSourceFile(), componentFile: mapping.node.getSourceFile(),
templateId,
start: span.start.offset, start: span.start.offset,
length: span.end.offset - span.start.offset, length: span.end.offset - span.start.offset,
relatedInformation, relatedInformation,
@ -233,6 +241,7 @@ export function makeTemplateDiagnostic(
messageText, messageText,
file: sf, file: sf,
componentFile: componentSf, componentFile: componentSf,
templateId,
start: span.start.offset, start: span.start.offset,
length: span.end.offset - span.start.offset, length: span.end.offset - span.start.offset,
// Show a secondary message indicating the component whose template contains the error. // Show a secondary message indicating the component whose template contains the error.

View File

@ -12,7 +12,7 @@ import * as ts from 'typescript';
import {ErrorCode, ngErrorCode} from '../../diagnostics'; import {ErrorCode, ngErrorCode} from '../../diagnostics';
import {TemplateId} from '../api'; import {TemplateId} from '../api';
import {makeTemplateDiagnostic, TemplateSourceResolver} from './diagnostics'; import {makeTemplateDiagnostic, TemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
const REGISTRY = new DomElementSchemaRegistry(); const REGISTRY = new DomElementSchemaRegistry();
const REMOVE_XHTML_REGEX = /^:xhtml:/; 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 * Get the `ts.Diagnostic`s that have been generated via `checkElement` and `checkProperty` calls
* thus far. * thus far.
*/ */
readonly diagnostics: ReadonlyArray<ts.Diagnostic>; readonly diagnostics: ReadonlyArray<TemplateDiagnostic>;
/** /**
* Check a non-Angular element and record any diagnostics about it. * 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. * maintained by the Angular team via extraction from a browser IDL.
*/ */
export class RegistryDomSchemaChecker implements DomSchemaChecker { export class RegistryDomSchemaChecker implements DomSchemaChecker {
private _diagnostics: ts.Diagnostic[] = []; private _diagnostics: TemplateDiagnostic[] = [];
get diagnostics(): ReadonlyArray<ts.Diagnostic> { get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
return this._diagnostics; return this._diagnostics;
} }
@ -93,7 +93,7 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker {
} }
const diag = makeTemplateDiagnostic( const diag = makeTemplateDiagnostic(
mapping, element.sourceSpan, ts.DiagnosticCategory.Error, id, mapping, element.sourceSpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.SCHEMA_INVALID_ELEMENT), errorMsg); ngErrorCode(ErrorCode.SCHEMA_INVALID_ELEMENT), errorMsg);
this._diagnostics.push(diag); this._diagnostics.push(diag);
} }
@ -123,7 +123,7 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker {
} }
const diag = makeTemplateDiagnostic( const diag = makeTemplateDiagnostic(
mapping, span, ts.DiagnosticCategory.Error, id, mapping, span, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.SCHEMA_INVALID_ATTRIBUTE), errorMsg); ngErrorCode(ErrorCode.SCHEMA_INVALID_ATTRIBUTE), errorMsg);
this._diagnostics.push(diag); this._diagnostics.push(diag);
} }

View File

@ -13,7 +13,7 @@ import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '..
import {ClassDeclaration} from '../../reflection'; import {ClassDeclaration} from '../../reflection';
import {TemplateId} from '../api'; 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. * recorder for later display.
*/ */
export interface OutOfBandDiagnosticRecorder { export interface OutOfBandDiagnosticRecorder {
readonly diagnostics: ReadonlyArray<ts.Diagnostic>; readonly diagnostics: ReadonlyArray<TemplateDiagnostic>;
/** /**
* Reports a `#ref="target"` expression in the template for which a target directive could not be * Reports a `#ref="target"` expression in the template for which a target directive could not be
@ -63,17 +63,18 @@ export interface OutOfBandDiagnosticRecorder {
duplicateTemplateVar( duplicateTemplateVar(
templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void; 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 { export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
private _diagnostics: ts.Diagnostic[] = []; private _diagnostics: TemplateDiagnostic[] = [];
constructor(private resolver: TemplateSourceResolver) {} constructor(private resolver: TemplateSourceResolver) {}
get diagnostics(): ReadonlyArray<ts.Diagnostic> { get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
return this._diagnostics; return this._diagnostics;
} }
@ -83,7 +84,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
const errorMsg = `No directive found with exportAs '${value}'.`; const errorMsg = `No directive found with exportAs '${value}'.`;
this._diagnostics.push(makeTemplateDiagnostic( 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)); 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}'.`); `Assertion failure: no SourceLocation found for usage of pipe '${ast.name}'.`);
} }
this._diagnostics.push(makeTemplateDiagnostic( this._diagnostics.push(makeTemplateDiagnostic(
mapping, sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.MISSING_PIPE), templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error,
errorMsg)); ngErrorCode(ErrorCode.MISSING_PIPE), errorMsg));
} }
illegalAssignmentToTemplateVar( illegalAssignmentToTemplateVar(
@ -113,7 +114,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
throw new Error(`Assertion failure: no SourceLocation found for property binding.`); throw new Error(`Assertion failure: no SourceLocation found for property binding.`);
} }
this._diagnostics.push(makeTemplateDiagnostic( this._diagnostics.push(makeTemplateDiagnostic(
mapping, sourceSpan, ts.DiagnosticCategory.Error, templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.WRITE_TO_READ_ONLY_VARIABLE), errorMsg, { ngErrorCode(ErrorCode.WRITE_TO_READ_ONLY_VARIABLE), errorMsg, {
text: `The variable ${assignment.name} is declared here.`, text: `The variable ${assignment.name} is declared here.`,
span: target.valueSpan || target.sourceSpan, 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. // TODO(alxhub): allocate to a tighter span once one is available.
this._diagnostics.push(makeTemplateDiagnostic( this._diagnostics.push(makeTemplateDiagnostic(
mapping, variable.sourceSpan, ts.DiagnosticCategory.Error, templateId, mapping, variable.sourceSpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION), errorMsg, { ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION), errorMsg, {
text: `The variable '${firstDecl.name}' was first declared here.`, text: `The variable '${firstDecl.name}' was first declared here.`,
span: firstDecl.sourceSpan, span: firstDecl.sourceSpan,
})); }));
} }
requiresInlineTcb(node: ClassDeclaration): void { requiresInlineTcb(templateId: TemplateId, node: ClassDeclaration): void {
this._diagnostics.push(makeDiagnostic( this._diagnostics.push(makeInlineDiagnostic(
ErrorCode.INLINE_TCB_REQUIRED, node.name, templateId, ErrorCode.INLINE_TCB_REQUIRED, node.name,
`This component requires inline template type-checking, which is not supported by the current environment.`)); `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; let message: string;
if (directives.length > 1) { if (directives.length > 1) {
message = 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 component uses a directive which requires an inline type constructor, which is not supported by the current environment.`;
} }
this._diagnostics.push(makeDiagnostic( this._diagnostics.push(makeInlineDiagnostic(
ErrorCode.INLINE_TYPE_CTOR_REQUIRED, node.name, message, templateId, ErrorCode.INLINE_TYPE_CTOR_REQUIRED, node.name, message,
directives.map( directives.map(
dir => makeRelatedInformation(dir.name, `Requires an inline type constructor.`)))); 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,
};
}

View File

@ -20,6 +20,7 @@ import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckContext} from '..
import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckingConfig, UpdateMode} from '../api/api'; import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckingConfig, UpdateMode} from '../api/api';
import {ReusedProgramStrategy} from '../src/augmented_program'; import {ReusedProgramStrategy} from '../src/augmented_program';
import {TemplateTypeCheckerImpl} from '../src/checker'; import {TemplateTypeCheckerImpl} from '../src/checker';
import {TemplateDiagnostic} from '../src/diagnostics';
import {DomSchemaChecker} from '../src/dom'; import {DomSchemaChecker} from '../src/dom';
import {Environment} from '../src/environment'; import {Environment} from '../src/environment';
import {OutOfBandDiagnosticRecorder} from '../src/oob'; import {OutOfBandDiagnosticRecorder} from '../src/oob';
@ -487,7 +488,7 @@ class FakeEnvironment /* implements Environment */ {
} }
export class NoopSchemaChecker implements DomSchemaChecker { export class NoopSchemaChecker implements DomSchemaChecker {
get diagnostics(): ReadonlyArray<ts.Diagnostic> { get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
return []; return [];
} }
@ -498,7 +499,7 @@ export class NoopSchemaChecker implements DomSchemaChecker {
} }
export class NoopOobRecorder implements OutOfBandDiagnosticRecorder { export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
get diagnostics(): ReadonlyArray<ts.Diagnostic> { get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
return []; return [];
} }
missingReferenceTarget(): void {} missingReferenceTarget(): void {}