diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index b306ecc00b..23c80c91e5 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -122,6 +122,16 @@ export enum ErrorCode { */ WRITE_TO_READ_ONLY_VARIABLE = 8005, + /** + * A template variable was declared twice. For example: + * + * ```html + *
+ *
+ * ``` + */ + DUPLICATE_VARIABLE_DECLARATION = 8006, + /** * An injectable already has a `ɵprov` property. */ diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index 58096e4fdb..2f1a8b6e9c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -50,6 +50,17 @@ export interface OutOfBandDiagnosticRecorder { illegalAssignmentToTemplateVar( templateId: TemplateId, assignment: PropertyWrite, target: TmplAstVariable): void; + + /** + * Reports a duplicate declaration of a template variable. + * + * @param templateId the template type-checking ID of the template which contains the duplicate + * declaration. + * @param variable the `TmplAstVariable` which duplicates a previously declared variable. + * @param firstDecl the first variable declaration which uses the same name as `variable`. + */ + duplicateTemplateVar( + templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void; } export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder { @@ -100,4 +111,23 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor span: target.valueSpan || target.sourceSpan, })); } + + duplicateTemplateVar( + templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void { + const mapping = this.resolver.getSourceMapping(templateId); + const errorMsg = + `Cannot redeclare variable '${variable.name}' as it was previously declared elsewhere for the same template.`; + + // The allocation of the error here is pretty useless for variables declared in microsyntax, + // since the sourceSpan refers to the entire microsyntax property, not a span for the specific + // variable in question. + // + // TODO(alxhub): allocate to a tighter span once one is available. + this._diagnostics.push(makeTemplateDiagnostic( + mapping, variable.sourceSpan, ts.DiagnosticCategory.Error, + ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION), errorMsg, { + text: `The variable '${firstDecl.name}' was first declared here.`, + span: firstDecl.sourceSpan, + })); + } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 70c6e8f078..667957411a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -738,7 +738,17 @@ class Scope { // has. if (templateOrNodes instanceof TmplAstTemplate) { // The template's variable declarations need to be added as `TcbVariableOp`s. + const varMap = new Map(); + for (const v of templateOrNodes.variables) { + // Validate that variables on the `TmplAstTemplate` are only declared once. + if (!varMap.has(v.name)) { + varMap.set(v.name, v); + } else { + const firstDecl = varMap.get(v.name) !; + tcb.oobRecorder.duplicateTemplateVar(tcb.id, v, firstDecl); + } + const opIndex = scope.opQueue.push(new TcbVariableOp(tcb, scope, templateOrNodes, v)) - 1; scope.varMap.set(v, opIndex); } 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 c5dcb906d5..da668db467 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -400,4 +400,5 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder { missingReferenceTarget(): void {} missingPipe(): void {} illegalAssignmentToTemplateVar(): void {} + duplicateTemplateVar(): void {} } diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 01f4fc0e18..e1286f0193 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -8,6 +8,7 @@ import * as ts from 'typescript'; +import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics'; import {absoluteFrom as _, getFileSystem} from '../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing'; import {loadStandardTestFiles} from '../helpers/src/mock_file_loading'; @@ -1303,6 +1304,36 @@ export declare class AnimationEvent { expect(getSourceCodeForDiagnostic(diags[0])).toEqual('y = !y'); }); + it('should detect a duplicate variable declaration', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + import {CommonModule} from '@angular/common'; + + @Component({ + selector: 'test', + template: \` +
+ {{i}} +
+ \`, + }) + export class TestCmp { + items!: string[]; + } + + @NgModule({ + declarations: [TestCmp], + imports: [CommonModule], + }) + export class Module {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toEqual(1); + expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION)); + expect(getSourceCodeForDiagnostic(diags[0])).toContain('let i of items;'); + }); + it('should still type-check when fileToModuleName aliasing is enabled, but alias exports are not in the .d.ts file', () => { // The template type-checking file imports directives/pipes in order to type-check their diff --git a/tools/public_api_guard/error_code.d.ts b/tools/public_api_guard/error_code.d.ts index 738dbaea60..14c75dc2a1 100644 --- a/tools/public_api_guard/error_code.d.ts +++ b/tools/public_api_guard/error_code.d.ts @@ -30,5 +30,6 @@ export declare enum ErrorCode { MISSING_REFERENCE_TARGET = 8003, MISSING_PIPE = 8004, WRITE_TO_READ_ONLY_VARIABLE = 8005, + DUPLICATE_VARIABLE_DECLARATION = 8006, INJECTABLE_DUPLICATE_PROV = 9001 }