Skip to content

Commit c8e2885

Browse files
eneajahothePunderWoman
authored andcommitted
feat(compiler): Add extended diagnostic to warn when there are uncalled functions in event bindings (#56295) (#56295)
The diagnostic will catch issues like: ```html <button (click)="increment"></button> <button (click)="increment; decrement"></button> <button (click)="true ? increment : decrement"></button> <button (click)="nested.nested1.nested2.increment"></button> ``` PR Close #56295 (cherry picked from commit fd6cd04) PR Close #56295
1 parent d9fe17a commit c8e2885

13 files changed

Lines changed: 486 additions & 12 deletions

File tree

‎adev/src/app/sub-navigation-data.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,6 +1388,11 @@ const REFERENCE_SUB_NAVIGATION_DATA: NavigationItem[] = [
13881388
path: 'extended-diagnostics/NG8109',
13891389
contentPath: 'reference/extended-diagnostics/NG8109',
13901390
},
1391+
{
1392+
label: 'NG8111: Functions must be invoked in event bindings',
1393+
path: 'extended-diagnostics/NG8111',
1394+
contentPath: 'reference/extended-diagnostics/NG8111',
1395+
},
13911396
],
13921397
},
13931398
{
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Functions should be invoked in event bindings.
2+
3+
This diagnostic detects uninvoked functions in event bindings.
4+
5+
<docs-code language="typescript">
6+
7+
import {Component, signal, Signal} from '@angular/core';
8+
9+
@Component({
10+
template: `<button (click)="onClick">Click me</button>`,
11+
})
12+
class MyComponent {
13+
onClick() {
14+
console.log('clicked');
15+
}
16+
}
17+
18+
</docs-code>
19+
20+
## What's wrong with that?
21+
22+
Functions in event bindings should be invoked when the event is triggered.
23+
If the function is not invoked, it will not execute when the event is triggered.
24+
25+
## What should I do instead?
26+
27+
Ensure to invoke the function when you use it in an event binding to execute the function when the event is triggered.
28+
29+
<docs-code language="typescript">
30+
31+
import {Component} from '@angular/core';
32+
33+
@Component({
34+
template: `<button (click)="onClick()">Click me</button>`,
35+
})
36+
class MyComponent {
37+
onClick() {
38+
console.log('clicked');
39+
}
40+
}
41+
42+
</docs-code>
43+
44+
## Configuration requirements
45+
46+
[`strictTemplates`](tools/cli/template-typecheck#strict-mode) must be enabled for any extended diagnostic to emit.
47+
`uninvokedFunctionInEventBinding` has no additional requirements beyond `strictTemplates`.
48+
49+
## What if I can't avoid this?
50+
51+
This diagnostic can be disabled by editing the project's `tsconfig.json` file:
52+
53+
<docs-code language="json">
54+
{
55+
"angularCompilerOptions": {
56+
"extendedDiagnostics": {
57+
"checks": {
58+
"uninvokedFunctionInEventBinding": "suppress"
59+
}
60+
}
61+
}
62+
}
63+
</docs-code>
64+
65+
See [extended diagnostic configuration](extended-diagnostics#configuration) for more info.

‎adev/src/content/reference/extended-diagnostics/overview.md‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,18 @@ The Angular compiler includes "extended diagnostics" which identify many of thes
88

99
Currently, Angular supports the following extended diagnostics:
1010

11-
| Code | Name |
12-
|:--- |:--- |
13-
| `NG8101` | [`invalidBananaInBox`](extended-diagnostics/NG8101) |
14-
| `NG8102` | [`nullishCoalescingNotNullable`](extended-diagnostics/NG8102) |
15-
| `NG8103` | [`missingControlFlowDirective`](extended-diagnostics/NG8103) |
16-
| `NG8104` | [`textAttributeNotBinding`](extended-diagnostics/NG8104) |
17-
| `NG8105` | [`missingNgForOfLet`](extended-diagnostics/NG8105) |
18-
| `NG8106` | [`suffixNotSupported`](extended-diagnostics/NG8106) |
19-
| `NG8107` | [`optionalChainNotNullable`](extended-diagnostics/NG8107) |
20-
| `NG8108` | [`skipHydrationNotStatic`](extended-diagnostics/NG8108) |
21-
| `NG8109` | [`interpolatedSignalNotInvoked`](extended-diagnostics/NG8109) |
11+
| Code | Name |
12+
|:---------|:-----------------------------------------------------------------|
13+
| `NG8101` | [`invalidBananaInBox`](extended-diagnostics/NG8101) |
14+
| `NG8102` | [`nullishCoalescingNotNullable`](extended-diagnostics/NG8102) |
15+
| `NG8103` | [`missingControlFlowDirective`](extended-diagnostics/NG8103) |
16+
| `NG8104` | [`textAttributeNotBinding`](extended-diagnostics/NG8104) |
17+
| `NG8105` | [`missingNgForOfLet`](extended-diagnostics/NG8105) |
18+
| `NG8106` | [`suffixNotSupported`](extended-diagnostics/NG8106) |
19+
| `NG8107` | [`optionalChainNotNullable`](extended-diagnostics/NG8107) |
20+
| `NG8108` | [`skipHydrationNotStatic`](extended-diagnostics/NG8108) |
21+
| `NG8109` | [`interpolatedSignalNotInvoked`](extended-diagnostics/NG8109) |
22+
| `NG8111` | [`uninvokedFunctionInEventBinding`](extended-diagnostics/NG8111) |
2223

2324
## Configuration
2425

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ export enum ErrorCode {
103103
TEXT_ATTRIBUTE_NOT_BINDING = 8104,
104104
UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007,
105105
UNDECORATED_PROVIDER = 2005,
106+
UNINVOKED_FUNCTION_IN_EVENT_BINDING = 8111,
106107
UNSUPPORTED_INITIALIZER_API_USAGE = 8110,
107108
// (undocumented)
108109
VALUE_HAS_WRONG_TYPE = 1010,

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ export enum ExtendedTemplateDiagnosticName {
2525
// (undocumented)
2626
SUFFIX_NOT_SUPPORTED = "suffixNotSupported",
2727
// (undocumented)
28-
TEXT_ATTRIBUTE_NOT_BINDING = "textAttributeNotBinding"
28+
TEXT_ATTRIBUTE_NOT_BINDING = "textAttributeNotBinding",
29+
// (undocumented)
30+
UNINVOKED_FUNCTION_IN_EVENT_BINDING = "uninvokedFunctionInEventBinding"
2931
}
3032

3133
// (No @packageDocumentation comment for this package)

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,19 @@ export enum ErrorCode {
482482
*/
483483
UNSUPPORTED_INITIALIZER_API_USAGE = 8110,
484484

485+
/**
486+
* A function in an event binding is not called.
487+
*
488+
* For example:
489+
* ```
490+
* <button (click)="myFunc"></button>
491+
* ```
492+
*
493+
* This will not call `myFunc` when the button is clicked. Instead, it should be
494+
* `<button (click)="myFunc()"></button>`.
495+
*/
496+
UNINVOKED_FUNCTION_IN_EVENT_BINDING = 8111,
497+
485498
/**
486499
* The template type-checking engine would need to generate an inline type check block for a
487500
* 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
@@ -21,6 +21,7 @@ export enum ExtendedTemplateDiagnosticName {
2121
OPTIONAL_CHAIN_NOT_NULLABLE = 'optionalChainNotNullable',
2222
MISSING_CONTROL_FLOW_DIRECTIVE = 'missingControlFlowDirective',
2323
TEXT_ATTRIBUTE_NOT_BINDING = 'textAttributeNotBinding',
24+
UNINVOKED_FUNCTION_IN_EVENT_BINDING = 'uninvokedFunctionInEventBinding',
2425
MISSING_NGFOROF_LET = 'missingNgForOfLet',
2526
SUFFIX_NOT_SUPPORTED = 'suffixNotSupported',
2627
SKIP_HYDRATION_NOT_STATIC = 'skipHydrationNotStatic',

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ ts_library(
2121
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_not_static",
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",
24+
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/uninvoked_function_in_event_binding",
2425
"@npm//typescript",
2526
],
2627
)
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_function_in_event_binding",
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: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
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.io/license
7+
*/
8+
9+
import {
10+
AST,
11+
ASTWithSource,
12+
Call,
13+
Chain,
14+
Conditional,
15+
ParsedEventType,
16+
PropertyRead,
17+
SafeCall,
18+
SafePropertyRead,
19+
TmplAstBoundEvent,
20+
TmplAstNode,
21+
} from '@angular/compiler';
22+
import ts from 'typescript';
23+
24+
import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
25+
import {NgTemplateDiagnostic, SymbolKind} from '../../../api';
26+
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
27+
28+
/**
29+
* Ensures that function in event bindings are called. For example, `<button (click)="myFunc"></button>`
30+
* will not call `myFunc` when the button is clicked. Instead, it should be `<button (click)="myFunc()"></button>`.
31+
* This is likely not the intent of the developer. Instead, the intent is likely to call `myFunc`.
32+
*/
33+
class UninvokedFunctionInEventBindingSpec extends TemplateCheckWithVisitor<ErrorCode.UNINVOKED_FUNCTION_IN_EVENT_BINDING> {
34+
override code = ErrorCode.UNINVOKED_FUNCTION_IN_EVENT_BINDING as const;
35+
36+
override visitNode(
37+
ctx: TemplateContext<ErrorCode.UNINVOKED_FUNCTION_IN_EVENT_BINDING>,
38+
component: ts.ClassDeclaration,
39+
node: TmplAstNode | AST,
40+
): NgTemplateDiagnostic<ErrorCode.UNINVOKED_FUNCTION_IN_EVENT_BINDING>[] {
41+
// If the node is not a bound event, skip it.
42+
if (!(node instanceof TmplAstBoundEvent)) return [];
43+
44+
// If the node is not a regular or animation event, skip it.
45+
if (node.type !== ParsedEventType.Regular && node.type !== ParsedEventType.Animation) return [];
46+
47+
if (!(node.handler instanceof ASTWithSource)) return [];
48+
49+
const sourceExpressionText = node.handler.source || '';
50+
51+
if (node.handler.ast instanceof Chain) {
52+
// (click)="increment; decrement"
53+
return node.handler.ast.expressions.flatMap((expression) =>
54+
assertExpressionInvoked(expression, component, node, sourceExpressionText, ctx),
55+
);
56+
}
57+
58+
if (node.handler.ast instanceof Conditional) {
59+
// (click)="true ? increment : decrement"
60+
const {trueExp, falseExp} = node.handler.ast;
61+
return [trueExp, falseExp].flatMap((expression) =>
62+
assertExpressionInvoked(expression, component, node, sourceExpressionText, ctx),
63+
);
64+
}
65+
66+
// (click)="increment"
67+
return assertExpressionInvoked(node.handler.ast, component, node, sourceExpressionText, ctx);
68+
}
69+
}
70+
71+
/**
72+
* Asserts that the expression is invoked.
73+
* If the expression is a property read, and it has a call signature, a diagnostic is generated.
74+
*/
75+
function assertExpressionInvoked(
76+
expression: AST,
77+
component: ts.ClassDeclaration,
78+
node: TmplAstBoundEvent,
79+
expressionText: string,
80+
ctx: TemplateContext<ErrorCode.UNINVOKED_FUNCTION_IN_EVENT_BINDING>,
81+
): NgTemplateDiagnostic<ErrorCode.UNINVOKED_FUNCTION_IN_EVENT_BINDING>[] {
82+
if (expression instanceof Call || expression instanceof SafeCall) {
83+
return []; // If the method is called, skip it.
84+
}
85+
86+
if (!(expression instanceof PropertyRead) && !(expression instanceof SafePropertyRead)) {
87+
return []; // If the expression is not a property read, skip it.
88+
}
89+
90+
const symbol = ctx.templateTypeChecker.getSymbolOfNode(expression, component);
91+
92+
if (symbol !== null && symbol.kind === SymbolKind.Expression) {
93+
if (symbol.tsType.getCallSignatures()?.length > 0) {
94+
const fullExpressionText = generateStringFromExpression(expression, expressionText);
95+
const errorString = `Function in event binding should be invoked: ${fullExpressionText}()`;
96+
return [ctx.makeTemplateDiagnostic(node.sourceSpan, errorString)];
97+
}
98+
}
99+
100+
return [];
101+
}
102+
103+
function generateStringFromExpression(expression: AST, source: string): string {
104+
return source.substring(expression.span.start, expression.span.end);
105+
}
106+
107+
export const factory: TemplateCheckFactory<
108+
ErrorCode.UNINVOKED_FUNCTION_IN_EVENT_BINDING,
109+
ExtendedTemplateDiagnosticName.UNINVOKED_FUNCTION_IN_EVENT_BINDING
110+
> = {
111+
code: ErrorCode.UNINVOKED_FUNCTION_IN_EVENT_BINDING,
112+
name: ExtendedTemplateDiagnosticName.UNINVOKED_FUNCTION_IN_EVENT_BINDING,
113+
create: () => new UninvokedFunctionInEventBindingSpec(),
114+
};

0 commit comments

Comments
 (0)