Content-Length: 931205 | pFad | http://github.com/Rohit-byt/angular-open/commit/7a971766dc59691dc68da9439e180a6c4d7b17d8

2E feat(compiler): add extended diagnostic for uninvoked track function … · Rohit-byt/angular-open@7a97176 · GitHub
Skip to content

Commit 7a97176

Browse files
eneajahoatscott
authored andcommitted
feat(compiler): add extended diagnostic for uninvoked track function on @for blocks (angular#60495)
The compiler will warn devs when they haven't invoked the track function passed to track in @for blocks PR Close angular#60495
1 parent 80a3258 commit 7a97176

File tree

10 files changed

+262
-0
lines changed

10 files changed

+262
-0
lines changed

goldens/public-api/compiler-cli/error_code.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ export enum ErrorCode {
107107
UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007,
108108
UNDECORATED_PROVIDER = 2005,
109109
UNINVOKED_FUNCTION_IN_EVENT_BINDING = 8111,
110+
UNINVOKED_TRACK_FUNCTION = 8115,
110111
UNPARENTHESIZED_NULLISH_COALESCING = 8114,
111112
UNSUPPORTED_INITIALIZER_API_USAGE = 8110,
112113
UNUSED_LET_DECLARATION = 8112,

goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export enum ExtendedTemplateDiagnosticName {
2929
// (undocumented)
3030
UNINVOKED_FUNCTION_IN_EVENT_BINDING = "uninvokedFunctionInEventBinding",
3131
// (undocumented)
32+
UNINVOKED_TRACK_FUNCTION = "uninvokedTrackFunction",
33+
// (undocumented)
3234
UNPARENTHESIZED_NULLISH_COALESCING = "unparenthesizedNullishCoalescing",
3335
// (undocumented)
3436
UNUSED_LET_DECLARATION = "unusedLetDeclaration",

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,23 @@ export enum ErrorCode {
525525
*/
526526
UNPARENTHESIZED_NULLISH_COALESCING = 8114,
527527

528+
/**
529+
* The function passed to `@for` track is not invoked.
530+
*
531+
* For example:
532+
* ```angular-html
533+
* @for (item of items; track trackByName) {}
534+
* ```
535+
*
536+
* For the track function to work properly, it must be invoked.
537+
*
538+
* For example:
539+
* ```angular-html
540+
* @for (item of items; track trackByName(item)) {}
541+
* ```
542+
*/
543+
UNINVOKED_TRACK_FUNCTION = 8115,
544+
528545
/**
529546
* The template type-checking engine would need to generate an inline type check block for a
530547
* component, but the current type-checking environment doesn't support it.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export enum ExtendedTemplateDiagnosticName {
2828
INTERPOLATED_SIGNAL_NOT_INVOKED = 'interpolatedSignalNotInvoked',
2929
CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 'controlFlowPreventingContentProjection',
3030
UNUSED_LET_DECLARATION = 'unusedLetDeclaration',
31+
UNINVOKED_TRACK_FUNCTION = 'uninvokedTrackFunction',
3132
UNUSED_STANDALONE_IMPORTS = 'unusedStandaloneImports',
3233
UNPARENTHESIZED_NULLISH_COALESCING = 'unparenthesizedNullishCoalescing',
3334
}

packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ ts_library(
2222
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported",
2323
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding",
2424
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_function_in_event_binding",
25+
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_track_function",
2526
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/unparenthesized_nullish_coalescing",
2627
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/unused_let_declaration",
2728
"@npm//typescript",
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
load("//tools:defaults.bzl", "ts_library")
2+
3+
ts_library(
4+
name = "uninvoked_track_function",
5+
srcs = ["index.ts"],
6+
visibility = [
7+
"//packages/compiler-cli/src/ngtsc:__subpackages__",
8+
"//packages/compiler-cli/test/ngtsc:__pkg__",
9+
],
10+
deps = [
11+
"//packages/compiler",
12+
"//packages/compiler-cli/src/ngtsc/diagnostics",
13+
"//packages/compiler-cli/src/ngtsc/typecheck/api",
14+
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
15+
"@npm//typescript",
16+
],
17+
)
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import {
10+
AST,
11+
ASTWithSource,
12+
Call,
13+
PropertyRead,
14+
SafeCall,
15+
SafePropertyRead,
16+
TmplAstForLoopBlock,
17+
TmplAstNode,
18+
} from '@angular/compiler';
19+
import ts from 'typescript';
20+
21+
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
22+
import {NgTemplateDiagnostic, SymbolKind} from '../../../api';
23+
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
24+
25+
/**
26+
* Ensures that track functions in @for loops are invoked.
27+
*/
28+
class UninvokedTrackFunctionCheck extends TemplateCheckWithVisitor<ErrorCode.UNINVOKED_TRACK_FUNCTION> {
29+
override code = ErrorCode.UNINVOKED_TRACK_FUNCTION as const;
30+
31+
override visitNode(
32+
ctx: TemplateContext<ErrorCode.UNINVOKED_TRACK_FUNCTION>,
33+
component: ts.ClassDeclaration,
34+
node: TmplAstNode | AST,
35+
): NgTemplateDiagnostic<ErrorCode.UNINVOKED_TRACK_FUNCTION>[] {
36+
if (!(node instanceof TmplAstForLoopBlock) || !node.trackBy) {
37+
return [];
38+
}
39+
40+
if (node.trackBy.ast instanceof Call || node.trackBy.ast instanceof SafeCall) {
41+
// If the method is called, skip it.
42+
return [];
43+
}
44+
45+
if (
46+
!(node.trackBy.ast instanceof PropertyRead) &&
47+
!(node.trackBy.ast instanceof SafePropertyRead)
48+
) {
49+
// If the expression is not a property read, skip it.
50+
return [];
51+
}
52+
53+
const symbol = ctx.templateTypeChecker.getSymbolOfNode(node.trackBy.ast, component);
54+
55+
if (
56+
symbol !== null &&
57+
symbol.kind === SymbolKind.Expression &&
58+
symbol.tsType.getCallSignatures()?.length > 0
59+
) {
60+
const fullExpressionText = generateStringFromExpression(
61+
node.trackBy.ast,
62+
node.trackBy.source || '',
63+
);
64+
const errorString = `The track function in the @for block should be invoked: ${fullExpressionText}(/* arguments */)`;
65+
return [ctx.makeTemplateDiagnostic(node.sourceSpan, errorString)];
66+
}
67+
68+
return [];
69+
}
70+
}
71+
72+
function generateStringFromExpression(expression: AST, source: string): string {
73+
return source.substring(expression.span.start, expression.span.end);
74+
}
75+
76+
export const factory: TemplateCheckFactory<
77+
ErrorCode.UNINVOKED_TRACK_FUNCTION,
78+
ExtendedTemplateDiagnosticName.UNINVOKED_TRACK_FUNCTION
79+
> = {
80+
code: ErrorCode.UNINVOKED_TRACK_FUNCTION,
81+
name: ExtendedTemplateDiagnosticName.UNINVOKED_TRACK_FUNCTION,
82+
create: () => new UninvokedTrackFunctionCheck(),
83+
};

packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {factory as textAttributeNotBindingFactory} from './checks/text_attribute
2121
import {factory as uninvokedFunctionInEventBindingFactory} from './checks/uninvoked_function_in_event_binding';
2222
import {factory as unparenthesizedNullishCoalescingFactory} from './checks/unparenthesized_nullish_coalescing';
2323
import {factory as unusedLetDeclarationFactory} from './checks/unused_let_declaration';
24+
import {factory as uninvokedTrackFunctionFactory} from './checks/uninvoked_track_function';
2425

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

@@ -40,6 +41,7 @@ export const ALL_DIAGNOSTIC_FACTORIES: readonly TemplateCheckFactory<
4041
unusedLetDeclarationFactory,
4142
skipHydrationNotStaticFactory,
4243
unparenthesizedNullishCoalescingFactory,
44+
uninvokedTrackFunctionFactory,
4345
];
4446

4547
export const SUPPORTED_DIAGNOSTIC_NAMES = new Set<string>([
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
2+
3+
ts_library(
4+
name = "test_lib",
5+
testonly = True,
6+
srcs = ["uninvoked_track_function.spec.ts"],
7+
deps = [
8+
"//packages/compiler",
9+
"//packages/compiler-cli/src/ngtsc/core:api",
10+
"//packages/compiler-cli/src/ngtsc/diagnostics",
11+
"//packages/compiler-cli/src/ngtsc/file_system",
12+
"//packages/compiler-cli/src/ngtsc/file_system/testing",
13+
"//packages/compiler-cli/src/ngtsc/testing",
14+
"//packages/compiler-cli/src/ngtsc/typecheck/extended",
15+
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_track_function",
16+
"//packages/compiler-cli/src/ngtsc/typecheck/testing",
17+
"@npm//typescript",
18+
],
19+
)
20+
21+
jasmine_node_test(
22+
name = "test",
23+
bootstrap = ["//tools/testing:node_no_angular"],
24+
data = [
25+
"//packages/core:npm_package",
26+
],
27+
deps = [
28+
":test_lib",
29+
],
30+
)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import ts from 'typescript';
10+
11+
import {ErrorCode, ExtendedTemplateDiagnosticName, ngErrorCode} from '../../../../../diagnostics';
12+
import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
13+
import {runInEachFileSystem} from '../../../../../file_system/testing';
14+
import {getSourceCodeForDiagnostic} from '../../../../../testing';
15+
import {getClass, setup} from '../../../../testing';
16+
import {factory as uninvokedTrackFunctionCheckFactory} from '../../../checks/uninvoked_track_function';
17+
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';
18+
19+
runInEachFileSystem(() => {
20+
describe('UninvokedTrackFunctionCheck', () => {
21+
it('binds the error code to its extended template diagnostic name', () => {
22+
expect(uninvokedTrackFunctionCheckFactory.code).toBe(ErrorCode.UNINVOKED_TRACK_FUNCTION);
23+
expect(uninvokedTrackFunctionCheckFactory.name).toBe(
24+
ExtendedTemplateDiagnosticName.UNINVOKED_TRACK_FUNCTION,
25+
);
26+
});
27+
28+
it('should produce a diagnostic when a track function in a @for block is not invoked', () => {
29+
const diags = diagnoseTestComponent(
30+
`
31+
@for (item of items; track trackByName) {}
32+
`,
33+
`trackByName(item) { return item.name; }`,
34+
);
35+
36+
expect(diags.length).toBe(1);
37+
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
38+
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.UNINVOKED_TRACK_FUNCTION));
39+
expect(getSourceCodeForDiagnostic(diags[0])).toBe(
40+
`@for (item of items; track trackByName) {}`,
41+
);
42+
expect(diags[0].messageText).toBe(generateDiagnosticText('trackByName'));
43+
});
44+
45+
it('should not produce a warning when track is set to a getter', () => {
46+
const diags = diagnoseTestComponent(
47+
`
48+
@for (item of items; track nameGetter) {}
49+
`,
50+
`get nameGetter() { return this.items[0].name; }`,
51+
);
52+
53+
expect(diags.length).toBe(0);
54+
});
55+
56+
it('should not produce a warning when the function is invoked', () => {
57+
const diags = diagnoseTestComponent(
58+
`
59+
@for (item of items; track trackByName(item)) {}
60+
`,
61+
`trackByName(item) { return item.name; }`,
62+
);
63+
64+
expect(diags.length).toBe(0);
65+
});
66+
67+
it('should not produce a warning when track is item.name', () => {
68+
const diags = diagnoseTestComponent(
69+
`
70+
@for (item of items; track item.name) {}
71+
`,
72+
``,
73+
);
74+
75+
expect(diags.length).toBe(0);
76+
});
77+
});
78+
});
79+
80+
function diagnoseTestComponent(template: string, classField: string) {
81+
const fileName = absoluteFrom('/main.ts');
82+
const {program, templateTypeChecker} = setup([
83+
{
84+
fileName,
85+
templates: {'TestCmp': template},
86+
source: `
87+
export class TestCmp {
88+
items = [{name: 'a'}, {name: 'b'}];
89+
signalItems = [{name: signal('a')}, {name: signal('b')}];
90+
${classField}
91+
}`,
92+
},
93+
]);
94+
const sf = getSourceFileOrError(program, fileName);
95+
const component = getClass(sf, 'TestCmp');
96+
const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
97+
templateTypeChecker,
98+
program.getTypeChecker(),
99+
[uninvokedTrackFunctionCheckFactory],
100+
{} /* options */,
101+
);
102+
103+
return extendedTemplateChecker.getDiagnosticsForComponent(component);
104+
}
105+
106+
function generateDiagnosticText(method: string): string {
107+
return `The track function in the @for block should be invoked: ${method}(/* arguments */)`;
108+
}

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/Rohit-byt/angular-open/commit/7a971766dc59691dc68da9439e180a6c4d7b17d8

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy