Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions goldens/public-api/compiler-cli/error_code.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export enum ErrorCode {
// (undocumented)
SYMBOL_NOT_EXPORTED = 3001,
TEMPLATE_PARSE_ERROR = 5002,
TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER = 8119,
TEXT_ATTRIBUTE_NOT_BINDING = 8104,
UNCLAIMED_DIRECTIVE_BINDING = 8018,
UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export enum ExtendedTemplateDiagnosticName {
// (undocumented)
SUFFIX_NOT_SUPPORTED = "suffixNotSupported",
// (undocumented)
TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER = "templateReferenceShadowsClassMember",
// (undocumented)
TEXT_ATTRIBUTE_NOT_BINDING = "textAttributeNotBinding",
// (undocumented)
UNINVOKED_FUNCTION_IN_EVENT_BINDING = "uninvokedFunctionInEventBinding",
Expand Down
19 changes: 19 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,25 @@ export enum ErrorCode {
*/
FORBIDDEN_REQUIRED_INITIALIZER_INVOCATION = 8118,

/**
* A template reference variable shadows a property or method of the component class.
*
* For example:
* ```typescript
* @Component({
* template: '<div #name></div>'
* })
* export class MyComponent {
* name = 'Angular';
* }
* ```
*
* In this case, inside the template `name` refers to the template reference, not the component
* property. This can lead to confusion when the developer expects `name` to resolve to the
* component class member.
*/
TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER = 8119,

/**
* The template type-checking engine would need to generate an inline type check block for a
* component, but the current type-checking environment doesn't support it.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@ export enum ExtendedTemplateDiagnosticName {
UNPARENTHESIZED_NULLISH_COALESCING = 'unparenthesizedNullishCoalescing',
UNINVOKED_FUNCTION_IN_TEXT_INTERPOLATION = 'uninvokedFunctionInTextInterpolation',
DEFER_TRIGGER_MISCONFIGURATION = 'deferTriggerMisconfiguration',
TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER = 'templateReferenceShadowsClassMember',
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ ts_project(
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/optional_chain_not_nullable",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_not_static",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/template_reference_shadows_class_member",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_function_in_event_binding",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_function_in_text_interpolation",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
load("//tools:defaults.bzl", "ts_project")

ts_project(
name = "template_reference_shadows_class_member",
srcs = ["index.ts"],
visibility = [
"//packages/compiler-cli/src/ngtsc:__subpackages__",
"//packages/compiler-cli/test/ngtsc:__pkg__",
],
deps = [
"//:node_modules/typescript",
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/

import {AST, TmplAstNode, TmplAstReference} from '@angular/compiler';
import ts from 'typescript';

import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
import {NgTemplateDiagnostic} from '../../../api';
import {
TemplateCheckFactory,
TemplateCheckWithVisitor,
TemplateContext,
formatExtendedError,
} from '../../api';

/**
* Ensures that template reference variables do not shadow component class members.
*/
class TemplateReferenceShadowsClassMemberCheck extends TemplateCheckWithVisitor<ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER> {
override code = ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER as const;

override visitNode(
ctx: TemplateContext<ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER>,
component: ts.ClassDeclaration,
node: TmplAstNode | AST,
): NgTemplateDiagnostic<ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER>[] {
if (!(node instanceof TmplAstReference)) {
return [];
}

const refName = node.name;

// Check if any member of the component class (including inherited members) has the same name.
const componentType = ctx.typeChecker.getTypeAtLocation(component);
const classMember = componentType.getProperty(refName);
if (classMember === undefined) {
return [];
}

const errorString = formatExtendedError(
ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER,
`Template reference variable '#${refName}' shadows the class member '${refName}'. ` +
`Within the template, '${refName}' will refer to the template reference, ` +
`not the class property.`,
);

const diagnostic = ctx.makeTemplateDiagnostic(node.keySpan, errorString);
return [diagnostic];
}
}

export const factory: TemplateCheckFactory<
ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER,
ExtendedTemplateDiagnosticName.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER
> = {
code: ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER,
name: ExtendedTemplateDiagnosticName.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER,
create: () => new TemplateReferenceShadowsClassMemberCheck(),
};
2 changes: 2 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {factory as unusedLetDeclarationFactory} from './checks/unused_let_declar
import {factory as uninvokedTrackFunctionFactory} from './checks/uninvoked_track_function';
import {factory as uninvokedFunctionInTextInterpolationFactory} from './checks/uninvoked_function_in_text_interpolation';
import {factory as deferTriggerMisconfigurationFactory} from './checks/defer_trigger_misconfiguration';
import {factory as templateReferenceShadowsClassMemberFactory} from './checks/template_reference_shadows_class_member';

export {ExtendedTemplateCheckerImpl} from './src/extended_template_checker';

Expand All @@ -48,6 +49,7 @@ export const ALL_DIAGNOSTIC_FACTORIES: readonly TemplateCheckFactory<
uninvokedTrackFunctionFactory,
uninvokedFunctionInTextInterpolationFactory,
deferTriggerMisconfigurationFactory,
templateReferenceShadowsClassMemberFactory,
];

export const SUPPORTED_DIAGNOSTIC_NAMES = new Set<string>([
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
load("//tools:defaults.bzl", "jasmine_test", "ts_project")

ts_project(
name = "test_lib",
testonly = True,
srcs = ["template_reference_shadows_class_member_spec.ts"],
deps = [
"//:node_modules/typescript",
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/core:api",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/file_system",
"//packages/compiler-cli/src/ngtsc/file_system/testing",
"//packages/compiler-cli/src/ngtsc/testing",
"//packages/compiler-cli/src/ngtsc/typecheck/extended",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/template_reference_shadows_class_member",
"//packages/compiler-cli/src/ngtsc/typecheck/testing",
],
)

jasmine_test(
name = "test",
data = [
":test_lib",
"//packages/core:npm_package",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/

import ts from 'typescript';

import {ErrorCode, ExtendedTemplateDiagnosticName, ngErrorCode} from '../../../../../diagnostics';
import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
import {runInEachFileSystem} from '../../../../../file_system/testing';
import {getSourceCodeForDiagnostic} from '../../../../../testing';
import {getClass, setup} from '../../../../testing';
import {factory as templateReferenceShadowsClassMemberFactory} from '../../../checks/template_reference_shadows_class_member/index';
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';

runInEachFileSystem(() => {
describe('TemplateReferenceShadowsClassMemberCheck', () => {
function diagnose(template: string, source?: string) {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([
{
fileName,
templates: {
'TestCmp': template,
},
source:
source ??
`
export class TestCmp {
name: string = 'test';
value: number = 42;
onClick() {}
}
`,
},
]);
const sf = getSourceFileOrError(program, fileName);
const component = getClass(sf, 'TestCmp');
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
templateTypeChecker,
program.getTypeChecker(),
[templateReferenceShadowsClassMemberFactory],
{},
);
return extendedTemplateChecker.getDiagnosticsForComponent(component);
}

it('binds the error code to its extended template diagnostic name', () => {
expect(templateReferenceShadowsClassMemberFactory.code).toBe(
ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER,
);
expect(templateReferenceShadowsClassMemberFactory.name).toBe(
ExtendedTemplateDiagnosticName.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER,
);
});

it('should report when a template reference shadows a component property', () => {
const diags = diagnose(`<div #name></div>`);

expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER));
expect(diags[0].messageText).toContain(`'#name'`);
expect(diags[0].messageText).toContain(`shadows`);
expect(getSourceCodeForDiagnostic(diags[0])).toBe('#name');
});

it('should report when a template reference shadows a component method', () => {
const diags = diagnose(`<button #onClick></button>`);

expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.TEMPLATE_REFERENCE_SHADOWS_CLASS_MEMBER));
expect(diags[0].messageText).toContain(`'#onClick'`);
});

it('should report multiple shadowing references', () => {
const diags = diagnose(`<div #name></div><span #value></span>`);

expect(diags.length).toBe(2);
});

it('should not report when no shadowing occurs', () => {
const diags = diagnose(`<div #myRef></div>`);

expect(diags.length).toBe(0);
});

it('should report when a template reference shadows an inherited property', () => {
const diags = diagnose(
`<div #inheritedProp></div>`,
`
class BaseCmp {
inheritedProp: string = 'base';
}
export class TestCmp extends BaseCmp {}
`,
);

expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain(`'#inheritedProp'`);
});

it('should report when a template reference shadows a private property', () => {
const diags = diagnose(
`<div #secret></div>`,
`
export class TestCmp {
private secret: string = 'hidden';
}
`,
);

expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain(`'#secret'`);
});

it('should not report template references that do not match any class member', () => {
const diags = diagnose(
`<div #foo></div><span #bar></span>`,
`
export class TestCmp {
baz: string = 'test';
}
`,
);

expect(diags.length).toBe(0);
});
});
});
Loading