fix(compiler-cli): default template diagnostic related message source file to template

For external templates (using `templateUrl`), primary diagnostics are
reported against the synthetic `ts.SourceFile` representing the HTML
template document. However, secondary related messages (such as those in
`foreign_component.ts` and `oob.ts`) were explicitly passing the
component's TypeScript file as `sourceFile`.

Because the character offsets (`start` and `end`) originate from the
HTML template AST, associating them with the TypeScript source file
caused IDEs and CLI diagnostics to map HTML offsets onto the `.ts` file,
resulting in corrupt or out-of-bounds source locations.

This commit resolves the issue by:
1. Making `sourceFile` optional in `makeTemplateDiagnostic` and related
   checker interfaces (`TemplateTypeChecker`, `TemplateContext`).
2. Defaulting `relatedMessage.sourceFile` to the template's source file
   (`sf` for external/indirect templates, or the component `.ts` file
   for direct inline templates) when not explicitly provided.
3. Removing explicit `sourceFile: this.sourceMapping.node.getSourceFile()`
   mappings from `foreign_component.ts` and DOM element checks in
   `oob.ts`, allowing them to automatically resolve to the template file.
4. Adding unit test coverage for external templates encountering foreign
   component conflicts with related messages.

(cherry picked from commit a46292af26)
This commit is contained in:
leonsenft
2026-08-25 22:02:28 +00:00
parent 7546b7a805
commit afe8499a14
8 changed files with 170 additions and 29 deletions
@@ -166,7 +166,6 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
text: 'Child nodes are defined here.',
start: firstChild.sourceSpan.start.offset,
end: firstChild.sourceSpan.end.offset,
sourceFile: this.sourceMapping.node.getSourceFile(),
},
],
),
@@ -307,7 +306,6 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
text: `The @content block '${block.name}' was first defined here.`,
start: firstDecl.sourceSpan.start.offset,
end: firstDecl.sourceSpan.end.offset,
sourceFile: this.sourceMapping.node.getSourceFile(),
},
],
),
@@ -340,7 +338,6 @@ class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
text: `The property '${block.name}' is defined here.`,
start: conflict.sourceSpan.start.offset,
end: conflict.sourceSpan.end.offset,
sourceFile: this.sourceMapping.node.getSourceFile(),
},
],
),
@@ -366,6 +366,10 @@ export interface TemplateTypeChecker {
/**
* Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template.
*
* @param relatedInformation Optional list of secondary related messages:
* - Omit `sourceFile` when `start` and `end` offsets are locations within the template itself.
* - Specify `sourceFile` only when referencing a separate file (e.g. directive class declaration).
*/
makeTemplateDiagnostic<T extends ErrorCode>(
clazz: ts.ClassDeclaration,
@@ -377,7 +381,7 @@ export interface TemplateTypeChecker {
text: string;
start: number;
end: number;
sourceFile: ts.SourceFile;
sourceFile?: ts.SourceFile;
}[],
): NgTemplateDiagnostic<T>;
}
@@ -25,6 +25,19 @@ interface DeprecatedDiagnosticInfo {
/**
* Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template.
*
* @param id The unique type-check ID for the component.
* @param mapping The source mapping for the template (direct, indirect, or external).
* @param span The source span within the template where the diagnostic occurred.
* @param category The diagnostic category (Error, Warning, Suggestion, Message).
* @param code The numeric Angular error code.
* @param messageText The primary diagnostic message.
* @param relatedMessages Optional list of secondary related messages:
* - Omit `sourceFile` (leave `undefined`) when `start` and `end` offsets correspond to
* positions within the template itself. The diagnostic will automatically associate them
* with the template source file (such as the parsed external HTML file or inline template node).
* - Specify `sourceFile` only when the message points to an external file (e.g., a component,
* directive, or pipe TypeScript declaration file) where `start` and `end` are offsets within
* that specific source file.
* @param deprecatedDiagInfo Optional information about deprecation and related messages.
*/
export function makeTemplateDiagnostic(
@@ -38,7 +51,7 @@ export function makeTemplateDiagnostic(
text: string;
start: number;
end: number;
sourceFile: ts.SourceFile;
sourceFile?: ts.SourceFile;
}[],
deprecatedDiagInfo?: DeprecatedDiagnosticInfo,
): TemplateDiagnostic {
@@ -49,7 +62,7 @@ export function makeTemplateDiagnostic(
relatedInformation.push({
category: ts.DiagnosticCategory.Message,
code: 0,
file: relatedMessage.sourceFile,
file: relatedMessage.sourceFile ?? mapping.node.getSourceFile(),
start: relatedMessage.start,
length: relatedMessage.end - relatedMessage.start,
messageText: relatedMessage.text,
@@ -89,24 +102,24 @@ export function makeTemplateDiagnostic(
? `${componentSf.fileName} (${componentName} template)`
: mapping.templateUrl;
let relatedInformation: ts.DiagnosticRelatedInformation[] = [];
if (relatedMessages !== undefined) {
for (const relatedMessage of relatedMessages) {
relatedInformation.push({
category: ts.DiagnosticCategory.Message,
code: 0,
file: relatedMessage.sourceFile,
start: relatedMessage.start,
length: relatedMessage.end - relatedMessage.start,
messageText: relatedMessage.text,
});
}
}
let sf: ts.SourceFile;
try {
sf = getParsedTemplateSourceFile(fileName, mapping);
} catch (e) {
let relatedInformation: ts.DiagnosticRelatedInformation[] = [];
if (relatedMessages !== undefined) {
for (const relatedMessage of relatedMessages) {
relatedInformation.push({
category: ts.DiagnosticCategory.Message,
code: 0,
file: relatedMessage.sourceFile ?? componentSf,
start: relatedMessage.start,
length: relatedMessage.end - relatedMessage.start,
messageText: relatedMessage.text,
});
}
}
const failureChain = makeDiagnosticChain(
`Failed to report an error in '${fileName}' at ${span.start.line + 1}:${
span.start.col + 1
@@ -130,6 +143,20 @@ export function makeTemplateDiagnostic(
};
}
let relatedInformation: ts.DiagnosticRelatedInformation[] = [];
if (relatedMessages !== undefined) {
for (const relatedMessage of relatedMessages) {
relatedInformation.push({
category: ts.DiagnosticCategory.Message,
code: 0,
file: relatedMessage.sourceFile ?? sf,
start: relatedMessage.start,
length: relatedMessage.end - relatedMessage.start,
messageText: relatedMessage.text,
});
}
}
let typeForMessage: string;
if (category === ts.DiagnosticCategory.Warning) {
@@ -51,6 +51,10 @@ export interface TemplateContext<Code extends ErrorCode> {
/**
* Creates a template diagnostic with the given information for the template being processed and
* using the diagnostic category configured for the extended template diagnostic.
*
* @param relatedInformation Optional list of secondary related messages:
* - Omit `sourceFile` when `start` and `end` offsets are locations within the template itself.
* - Specify `sourceFile` only when referencing a separate file (e.g. directive class declaration).
*/
makeTemplateDiagnostic(
sourceSpan: ParseSourceSpan,
@@ -59,7 +63,7 @@ export interface TemplateContext<Code extends ErrorCode> {
text: string;
start: number;
end: number;
sourceFile: ts.SourceFile;
sourceFile?: ts.SourceFile;
}[],
): NgTemplateDiagnostic<Code>;
}
@@ -80,9 +84,9 @@ export interface TemplateCheckFactory<
/**
* This abstract class provides a base implementation for the run method.
*/
export abstract class TemplateCheckWithVisitor<Code extends ErrorCode>
implements TemplateCheck<Code>
{
export abstract class TemplateCheckWithVisitor<
Code extends ErrorCode,
> implements TemplateCheck<Code> {
abstract code: Code;
/**
@@ -85,7 +85,7 @@ export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
text: string;
start: number;
end: number;
sourceFile: ts.SourceFile;
sourceFile?: ts.SourceFile;
}[],
): NgTemplateDiagnostic<ErrorCode> => {
return this.partialCtx.templateTypeChecker.makeTemplateDiagnostic(
@@ -908,7 +908,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
text: string;
start: number;
end: number;
sourceFile: ts.SourceFile;
sourceFile?: ts.SourceFile;
}[],
): NgTemplateDiagnostic<T> {
const sfPath = absoluteFromSourceFile(clazz.getSourceFile());
@@ -279,8 +279,12 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
const errorMsg = `The property and event halves of the two-way binding '${input.name}' are not bound to the same target.
Find more at ${DOC_PAGE_BASE_URL}/guide/templates/two-way-binding`;
const relatedMessages: {text: string; start: number; end: number; sourceFile: ts.SourceFile}[] =
[];
const relatedMessages: {
text: string;
start: number;
end: number;
sourceFile?: ts.SourceFile;
}[] = [];
if (inputConsumer.ref.nodeNameSpan && inputConsumer.ref.nodeFilePath) {
const sf = this.getSourceFile(inputConsumer.ref.nodeFilePath);
@@ -303,7 +307,6 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
text: message,
start: outputConsumer.sourceSpan.start.offset + 1,
end: outputConsumer.sourceSpan.start.offset + outputConsumer.name.length + 1,
sourceFile: mapping.node.getSourceFile(),
});
} else {
if (outputConsumer.ref.nodeNameSpan && outputConsumer.ref.nodeFilePath) {
@@ -2659,6 +2659,112 @@ runInEachFileSystem(() => {
'Child nodes are defined here.',
);
});
it('should detect duplicate @content blocks in an external template', () => {
env.write(
'test.ts',
`
${foreignSetupCode}
@Component({
selector: 'test',
templateUrl: './test.html',
foreignImports: [frameworkImport(FancyButton)],
})
export class TestCmp {}
`,
);
env.write(
'test.html',
'<FancyButton> @content (icon) {} @content (icon) {} </FancyButton>',
);
const diags = env.driveDiagnostics();
expect(diags.length).toEqual(1);
expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.CONFLICTING_CONTENT_DECLARATION));
expect(diags[0].file?.fileName).toMatch(/test\.html$/);
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('@content (icon) {}');
expect(diags[0].relatedInformation).toBeDefined();
expect(diags[0].relatedInformation!.length).toEqual(2);
expect(diags[0].relatedInformation![0].file?.fileName).toMatch(/test\.html$/);
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0])).toEqual(
'@content (icon) {}',
);
expect(diags[0].relatedInformation![0].messageText).toEqual(
"The @content block 'icon' was first defined here.",
);
});
it('should detect a conflict between a @content block and property binding in an external template', () => {
env.write(
'test.ts',
`
${foreignSetupCode}
@Component({
selector: 'test',
templateUrl: './test.html',
foreignImports: [frameworkImport(FancyButton)],
})
export class TestCmp {
myIcon = document.createTextNode('circle');
}
`,
);
env.write(
'test.html',
'<FancyButton [icon]="myIcon"> @content (icon) {square} </FancyButton>',
);
const diags = env.driveDiagnostics();
expect(diags.length).toEqual(1);
expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.CONFLICTING_CONTENT_AND_PROPERTY));
expect(diags[0].file?.fileName).toMatch(/test\.html$/);
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('@content (icon) {square}');
expect(diags[0].relatedInformation).toBeDefined();
expect(diags[0].relatedInformation!.length).toEqual(2);
expect(diags[0].relatedInformation![0].file?.fileName).toMatch(/test\.html$/);
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0])).toEqual(
'[icon]="myIcon"',
);
expect(diags[0].relatedInformation![0].messageText).toEqual(
"The property 'icon' is defined here.",
);
});
it('should detect a conflict between implicit children and [children] binding in an external template', () => {
env.write(
'test.ts',
`
${foreignSetupCode}
@Component({
selector: 'test',
templateUrl: './test.html',
foreignImports: [frameworkImport(FancyButton)],
})
export class TestCmp {
myChildren = [];
}
`,
);
env.write(
'test.html',
'<FancyButton [children]="myChildren"> <div>child</div> </FancyButton>',
);
const diags = env.driveDiagnostics();
expect(diags.length).toEqual(1);
expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.CONFLICTING_CONTENT_AND_PROPERTY));
expect(diags[0].file?.fileName).toMatch(/test\.html$/);
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('[children]="myChildren"');
expect(diags[0].relatedInformation).toBeDefined();
expect(diags[0].relatedInformation!.length).toEqual(2);
expect(diags[0].relatedInformation![0].file?.fileName).toMatch(/test\.html$/);
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0])).toEqual(
'<div>child</div>',
);
expect(diags[0].relatedInformation![0].messageText).toEqual(
'Child nodes are defined here.',
);
});
});
it('should detect a duplicate variable declaration', () => {