Skip to content

Commit 4096bbf

Browse files
alxhubSplaktar
authored andcommitted
refactor(compiler-cli): add TemplateId to template diagnostics (angular#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 angular#38105
1 parent 0314fd4 commit 4096bbf

File tree

5 files changed

+57
-33
lines changed

5 files changed

+57
-33
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/context.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {ClassDeclaration, ReflectionHost} from '../../reflection';
1515
import {ImportManager} from '../../translator';
1616
import {ComponentToShimMappingStrategy, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api';
1717

18+
import {TemplateDiagnostic} from './diagnostics';
1819
import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom';
1920
import {Environment} from './environment';
2021
import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob';
@@ -34,7 +35,7 @@ export interface ShimTypeCheckingData {
3435
*
3536
* Some diagnostics are produced during creation time and are tracked here.
3637
*/
37-
genesisDiagnostics: ts.Diagnostic[];
38+
genesisDiagnostics: TemplateDiagnostic[];
3839

3940
/**
4041
* Whether any inline operations for the input file were required to generate this shim.
@@ -182,7 +183,6 @@ export class TypeCheckContextImpl implements TypeCheckContext {
182183
}
183184

184185
const sfPath = absoluteFromSourceFile(ref.node.getSourceFile());
185-
186186
const overrideTemplate = this.host.getTemplateOverride(sfPath, ref.node);
187187
if (overrideTemplate !== null) {
188188
template = overrideTemplate;
@@ -231,12 +231,13 @@ export class TypeCheckContextImpl implements TypeCheckContext {
231231
// and inlining would be required.
232232

233233
// Record diagnostics to indicate the issues with this template.
234+
const templateId = fileData.sourceManager.getTemplateId(ref.node);
234235
if (tcbRequiresInline) {
235-
shimData.oobRecorder.requiresInlineTcb(ref.node);
236+
shimData.oobRecorder.requiresInlineTcb(templateId, ref.node);
236237
}
237238

238239
if (missingInlines.length > 0) {
239-
shimData.oobRecorder.requiresInlineTypeConstructors(ref.node, missingInlines);
240+
shimData.oobRecorder.requiresInlineTypeConstructors(templateId, ref.node, missingInlines);
240241
}
241242

242243
// Checking this template would be unsupported, so don't try.

packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ export interface TemplateDiagnostic extends ts.Diagnostic {
2121
* The component with the template that resulted in this diagnostic.
2222
*/
2323
componentFile: ts.SourceFile;
24+
25+
/**
26+
* The template id of the component that resulted in this diagnostic.
27+
*/
28+
templateId: TemplateId;
2429
}
2530

2631
/**
@@ -119,7 +124,7 @@ export function shouldReportDiagnostic(diagnostic: ts.Diagnostic): boolean {
119124
* file from being reported as type-check errors.
120125
*/
121126
export function translateDiagnostic(
122-
diagnostic: ts.Diagnostic, resolver: TemplateSourceResolver): ts.Diagnostic|null {
127+
diagnostic: ts.Diagnostic, resolver: TemplateSourceResolver): TemplateDiagnostic|null {
123128
if (diagnostic.file === undefined || diagnostic.start === undefined) {
124129
return null;
125130
}
@@ -139,7 +144,8 @@ export function translateDiagnostic(
139144

140145
const mapping = resolver.getSourceMapping(sourceLocation.id);
141146
return makeTemplateDiagnostic(
142-
mapping, span, diagnostic.category, diagnostic.code, diagnostic.messageText);
147+
sourceLocation.id, mapping, span, diagnostic.category, diagnostic.code,
148+
diagnostic.messageText);
143149
}
144150

145151
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
155161
* Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template.
156162
*/
157163
export function makeTemplateDiagnostic(
158-
mapping: TemplateSourceMapping, span: ParseSourceSpan, category: ts.DiagnosticCategory,
159-
code: number, messageText: string|ts.DiagnosticMessageChain, relatedMessage?: {
164+
templateId: TemplateId, mapping: TemplateSourceMapping, span: ParseSourceSpan,
165+
category: ts.DiagnosticCategory, code: number, messageText: string|ts.DiagnosticMessageChain,
166+
relatedMessage?: {
160167
text: string,
161168
span: ParseSourceSpan,
162169
}): TemplateDiagnostic {
@@ -182,6 +189,7 @@ export function makeTemplateDiagnostic(
182189
messageText,
183190
file: mapping.node.getSourceFile(),
184191
componentFile: mapping.node.getSourceFile(),
192+
templateId,
185193
start: span.start.offset,
186194
length: span.end.offset - span.start.offset,
187195
relatedInformation,
@@ -233,6 +241,7 @@ export function makeTemplateDiagnostic(
233241
messageText,
234242
file: sf,
235243
componentFile: componentSf,
244+
templateId,
236245
start: span.start.offset,
237246
length: span.end.offset - span.start.offset,
238247
// Show a secondary message indicating the component whose template contains the error.

packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import * as ts from 'typescript';
1212
import {ErrorCode, ngErrorCode} from '../../diagnostics';
1313
import {TemplateId} from '../api';
1414

15-
import {makeTemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
15+
import {makeTemplateDiagnostic, TemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
1616

1717
const REGISTRY = new DomElementSchemaRegistry();
1818
const REMOVE_XHTML_REGEX = /^:xhtml:/;
@@ -31,7 +31,7 @@ export interface DomSchemaChecker {
3131
* Get the `ts.Diagnostic`s that have been generated via `checkElement` and `checkProperty` calls
3232
* thus far.
3333
*/
34-
readonly diagnostics: ReadonlyArray<ts.Diagnostic>;
34+
readonly diagnostics: ReadonlyArray<TemplateDiagnostic>;
3535

3636
/**
3737
* Check a non-Angular element and record any diagnostics about it.
@@ -64,9 +64,9 @@ export interface DomSchemaChecker {
6464
* maintained by the Angular team via extraction from a browser IDL.
6565
*/
6666
export class RegistryDomSchemaChecker implements DomSchemaChecker {
67-
private _diagnostics: ts.Diagnostic[] = [];
67+
private _diagnostics: TemplateDiagnostic[] = [];
6868

69-
get diagnostics(): ReadonlyArray<ts.Diagnostic> {
69+
get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
7070
return this._diagnostics;
7171
}
7272

@@ -93,7 +93,7 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker {
9393
}
9494

9595
const diag = makeTemplateDiagnostic(
96-
mapping, element.sourceSpan, ts.DiagnosticCategory.Error,
96+
id, mapping, element.sourceSpan, ts.DiagnosticCategory.Error,
9797
ngErrorCode(ErrorCode.SCHEMA_INVALID_ELEMENT), errorMsg);
9898
this._diagnostics.push(diag);
9999
}
@@ -123,7 +123,7 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker {
123123
}
124124

125125
const diag = makeTemplateDiagnostic(
126-
mapping, span, ts.DiagnosticCategory.Error,
126+
id, mapping, span, ts.DiagnosticCategory.Error,
127127
ngErrorCode(ErrorCode.SCHEMA_INVALID_ATTRIBUTE), errorMsg);
128128
this._diagnostics.push(diag);
129129
}

packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '..
1313
import {ClassDeclaration} from '../../reflection';
1414
import {TemplateId} from '../api';
1515

16-
import {makeTemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
16+
import {makeTemplateDiagnostic, TemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
1717

1818

1919

@@ -27,7 +27,7 @@ import {makeTemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
2727
* recorder for later display.
2828
*/
2929
export interface OutOfBandDiagnosticRecorder {
30-
readonly diagnostics: ReadonlyArray<ts.Diagnostic>;
30+
readonly diagnostics: ReadonlyArray<TemplateDiagnostic>;
3131

3232
/**
3333
* Reports a `#ref="target"` expression in the template for which a target directive could not be
@@ -63,17 +63,18 @@ export interface OutOfBandDiagnosticRecorder {
6363
duplicateTemplateVar(
6464
templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void;
6565

66-
requiresInlineTcb(node: ClassDeclaration): void;
66+
requiresInlineTcb(templateId: TemplateId, node: ClassDeclaration): void;
6767

68-
requiresInlineTypeConstructors(node: ClassDeclaration, directives: ClassDeclaration[]): void;
68+
requiresInlineTypeConstructors(
69+
templateId: TemplateId, node: ClassDeclaration, directives: ClassDeclaration[]): void;
6970
}
7071

7172
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
72-
private _diagnostics: ts.Diagnostic[] = [];
73+
private _diagnostics: TemplateDiagnostic[] = [];
7374

7475
constructor(private resolver: TemplateSourceResolver) {}
7576

76-
get diagnostics(): ReadonlyArray<ts.Diagnostic> {
77+
get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
7778
return this._diagnostics;
7879
}
7980

@@ -83,7 +84,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
8384

8485
const errorMsg = `No directive found with exportAs '${value}'.`;
8586
this._diagnostics.push(makeTemplateDiagnostic(
86-
mapping, ref.valueSpan || ref.sourceSpan, ts.DiagnosticCategory.Error,
87+
templateId, mapping, ref.valueSpan || ref.sourceSpan, ts.DiagnosticCategory.Error,
8788
ngErrorCode(ErrorCode.MISSING_REFERENCE_TARGET), errorMsg));
8889
}
8990

@@ -97,8 +98,8 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
9798
`Assertion failure: no SourceLocation found for usage of pipe '${ast.name}'.`);
9899
}
99100
this._diagnostics.push(makeTemplateDiagnostic(
100-
mapping, sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.MISSING_PIPE),
101-
errorMsg));
101+
templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error,
102+
ngErrorCode(ErrorCode.MISSING_PIPE), errorMsg));
102103
}
103104

104105
illegalAssignmentToTemplateVar(
@@ -113,7 +114,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
113114
throw new Error(`Assertion failure: no SourceLocation found for property binding.`);
114115
}
115116
this._diagnostics.push(makeTemplateDiagnostic(
116-
mapping, sourceSpan, ts.DiagnosticCategory.Error,
117+
templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error,
117118
ngErrorCode(ErrorCode.WRITE_TO_READ_ONLY_VARIABLE), errorMsg, {
118119
text: `The variable ${assignment.name} is declared here.`,
119120
span: target.valueSpan || target.sourceSpan,
@@ -132,20 +133,21 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
132133
//
133134
// TODO(alxhub): allocate to a tighter span once one is available.
134135
this._diagnostics.push(makeTemplateDiagnostic(
135-
mapping, variable.sourceSpan, ts.DiagnosticCategory.Error,
136+
templateId, mapping, variable.sourceSpan, ts.DiagnosticCategory.Error,
136137
ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION), errorMsg, {
137138
text: `The variable '${firstDecl.name}' was first declared here.`,
138139
span: firstDecl.sourceSpan,
139140
}));
140141
}
141142

142-
requiresInlineTcb(node: ClassDeclaration): void {
143-
this._diagnostics.push(makeDiagnostic(
144-
ErrorCode.INLINE_TCB_REQUIRED, node.name,
143+
requiresInlineTcb(templateId: TemplateId, node: ClassDeclaration): void {
144+
this._diagnostics.push(makeInlineDiagnostic(
145+
templateId, ErrorCode.INLINE_TCB_REQUIRED, node.name,
145146
`This component requires inline template type-checking, which is not supported by the current environment.`));
146147
}
147148

148-
requiresInlineTypeConstructors(node: ClassDeclaration, directives: ClassDeclaration[]): void {
149+
requiresInlineTypeConstructors(
150+
templateId: TemplateId, node: ClassDeclaration, directives: ClassDeclaration[]): void {
149151
let message: string;
150152
if (directives.length > 1) {
151153
message =
@@ -155,9 +157,20 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
155157
`This component uses a directive which requires an inline type constructor, which is not supported by the current environment.`;
156158
}
157159

158-
this._diagnostics.push(makeDiagnostic(
159-
ErrorCode.INLINE_TYPE_CTOR_REQUIRED, node.name, message,
160+
this._diagnostics.push(makeInlineDiagnostic(
161+
templateId, ErrorCode.INLINE_TYPE_CTOR_REQUIRED, node.name, message,
160162
directives.map(
161163
dir => makeRelatedInformation(dir.name, `Requires an inline type constructor.`))));
162164
}
163165
}
166+
167+
function makeInlineDiagnostic(
168+
templateId: TemplateId, code: ErrorCode.INLINE_TCB_REQUIRED|ErrorCode.INLINE_TYPE_CTOR_REQUIRED,
169+
node: ts.Node, messageText: string|ts.DiagnosticMessageChain,
170+
relatedInformation?: ts.DiagnosticRelatedInformation[]): TemplateDiagnostic {
171+
return {
172+
...makeDiagnostic(code, node, messageText, relatedInformation),
173+
componentFile: node.getSourceFile(),
174+
templateId,
175+
};
176+
}

packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckContext} from '..
2020
import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckingConfig, UpdateMode} from '../api/api';
2121
import {ReusedProgramStrategy} from '../src/augmented_program';
2222
import {TemplateTypeCheckerImpl} from '../src/checker';
23+
import {TemplateDiagnostic} from '../src/diagnostics';
2324
import {DomSchemaChecker} from '../src/dom';
2425
import {Environment} from '../src/environment';
2526
import {OutOfBandDiagnosticRecorder} from '../src/oob';
@@ -487,7 +488,7 @@ class FakeEnvironment /* implements Environment */ {
487488
}
488489

489490
export class NoopSchemaChecker implements DomSchemaChecker {
490-
get diagnostics(): ReadonlyArray<ts.Diagnostic> {
491+
get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
491492
return [];
492493
}
493494

@@ -498,7 +499,7 @@ export class NoopSchemaChecker implements DomSchemaChecker {
498499
}
499500

500501
export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
501-
get diagnostics(): ReadonlyArray<ts.Diagnostic> {
502+
get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
502503
return [];
503504
}
504505
missingReferenceTarget(): void {}

0 commit comments

Comments
 (0)