8000 feat(no-misused-signals): check use of signals in logical expressions · angular-eslint/angular-eslint@33e6dfc · GitHub
[go: up one dir, main page]
Skip to content

Commit 33e6dfc

Browse files
author
Stephen Jackson
committed
feat(no-misused-signals): check use of signals in logical expressions
Checks that signals used in logical expressions are invoked. For #2302
1 parent 5303199 commit 33e6dfc

File tree

6 files changed

+266
-0
lines changed

6 files changed

+266
-0
lines changed

‎packages/eslint-plugin/src/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ import noPipeImpure, {
7373
import noQueriesMetadataProperty, {
7474
RULE_NAME as noQueriesMetadataPropertyRuleName,
7575
} from './rules/no-queries-metadata-property';
76+
import noUncalledSignals, {
77+
RULE_NAME as noUncalledSignalsRuleName,
78+
} from './rules/no-uncalled-signals';
7679
import pipePrefix, {
7780
RULE_NAME as pipePrefixRuleName,
7881
} from './rules/pipe-prefix';
@@ -149,6 +152,7 @@ export = {
149152
[noInputRenameRuleName]: noInputRename,
150153
[noInputsMetadataPropertyRuleName]: noInputsMetadataProperty,
151154
[noLifecycleCallRuleName]: noLifecycleCall,
155+
[noUncalledSignalsRuleName]: noUncalledSignals,
152156
[noOutputNativeRuleName]: noOutputNative,
153157
[noOutputOnPrefixRuleName]: noOutputOnPrefix,
154158
[noOutputRenameRuleName]: noOutputRename,

‎packages/eslint-plugin/src/rules/no-uncalled-signals.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import {
2+
AST_NODE_TYPES,
3+
ESLintUtils,
4+
ParserServicesWithTypeInformation,
5+
TSESTree,
6+
} from '@typescript-eslint/utils';
7+
import { createESLintRule } from '../utils/create-eslint-rule';
8+
9+
export type Options = [];
10+
export type MessageIds = 'noUncalledSignals';
11+
export const RULE_NAME = 'no-uncalled-signals';
12+
13+
const KNOWN_SIGNAL_TYPES: ReadonlySet<string> = new Set([
14+
'InputSignal',
15+
'ModelSignal',
16+
'Signal',
17+
'WritableSignal',
18+
]);
19+
20+
export default createESLintRule<Options, MessageIds>({
21+
name: RULE_NAME,
22+
meta: {
23+
type: 'suggestion',
24+
docs: {
25+
description:
26+
"Warns user about unintentionally doing logic on the signal, rather than the signal's value",
27+
},
28+
hasSuggestions: true,
29+
schema: [],
30+
messages: {
31+
noUncalledSignals:
32+
'Doing logic operations on signals will give unexpected results, you probably want to invoke the signal to get its value',
33+
},
34+
},
35+
defaultOptions: [],
36+
create(context) {
37+
const services: ParserServicesWithTypeInformation =
38+
ESLintUtils.getParserServices(context);
39+
40+
return {
41+
'*.test[type=Identifier], *.test Identifier'(node: TSESTree.Identifier) {
42+
if (node.parent.type === AST_NODE_TYPES.CallExpression) {
43+
return;
44+
}
45+
46+
const type = services.getTypeAtLocation(node);
47+
const identifierType = type.getSymbol()?.name;
48+
49+
if (identifierType && KNOWN_SIGNAL_TYPES.has(identifierType)) {
50+
context.report({
51+
node,
52+
messageId: 'noUncalledSignals',
53+
});
54+
}
55+
},
56+
};
57+
},
58+
});

‎packages/eslint-plugin/tests/rules/no-uncalled-signals/cases.ts

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import { convertAnnotatedSourceToFailureCase } from '@angular-eslint/test-utils';
2+
import type {
3+
InvalidTestCase,
4+
ValidTestCase,
5+
} from '@typescript-eslint/rule-tester';
6+
import { MessageIds, Options } from '../../../src/rules/no-uncalled-signals';
7+
8+
const messageId: MessageIds = 'noUncalledSignals';
9+
10+
export const valid: readonly (string | ValidTestCase<Options>)[] = [
11+
`
12+
const arbitraryVar = 1;
13+
if (arbitraryVar) {
14+
}
15+
`,
16+
`
17+
const aSignal = createSignal();
18+
if (aSignal()) {
19+
}
20+
declare function createSignal(): Signal<boolean>;
21+
interface Signal<T> {}
22+
`,
23+
`
24+
const aSignal = createSignal();
25+
if (aSignal() || true) {
26+
}
27+
declare function createSignal(): Signal<boolean>;
28+
interface Signal<T> {}
29+
`,
30+
`
31+
const aSignal = createSignal();
32+
if (aSignal() == "hello") {
33+
}
34+
declare function createSignal(): Signal<boolean>;
35+
interface Signal<T> {}
36+
`,
37+
`
38+
const aSignal = createSignal();
39+
if (false || (aSignal() ?? true)) {
40+
}
41+
declare function createSignal(): Signal<boolean>;
42+
interface Signal<T> {}
43+
`,
44+
`
45+
const aSignal = createSignal();
46+
if (false) {
47+
aSignal
48+
}
49+
declare function createSignal(): Signal<boolean>;
50+
interface Signal<T> {}
51+
`,
52+
`
53+
let aSignal: Signal | null = createSignal();
54+
if (aSignal) {
55+
}
56+
declare function createSignal(): Signal<boolean>;
57+
interface Signal<T> {}
58+
`,
59+
`
60+
let aSignal: Signal | undefined = createSignal();
61+
if (aSignal) {
62+
}
63+
declare function createSignal(): Signal<boolean>;
64+
interface Signal<T> {}
65+
`,
66+
`
67+
let aSignal: Signal | NonSignal = createSignal();
68+
if (aSignal) {
69+
}
70+
declare function createSignal(): Signal<boolean>;
71+
interface Signal<T> {}
72+
interface NonSignal {}
73+
`,
74+
];
75+
76+
export const invalid: readonly InvalidTestCase<MessageIds, Options>[] = [
77+
// If statements:
78+
convertAnnotatedSou 10BC0 rceToFailureCase({
79+
description:
80+
'(If Statement) should fail if the signal is not invoked as the only expression',
81+
annotatedSource: `
82+
const aSignal = createSignal();
83+
if (aSignal) {
84+
~~~~~~~
85+
}
86+
declare function createSignal(): Signal<boolean>;
87+
interface Signal<T> {}
88+
`,
89+
messageId,
90+
}),
91+
convertAnnotatedSourceToFailureCase({
92+
description:
93+
'(If Statement) should fail if the signal is not invoked as part of a logical expression',
94+
annotatedSource: `
95+
const aSignal = createSignal(false);
96+
if (aSignal || true) {
97+
~~~~~~~
98+
}
99+
declare function createSignal(): Signal<boolean>;
100+
interface Signal<T> {}
101+
`,
102+
messageId,
103+
}),
104+
convertAnnotatedSourceToFailureCase({
105+
description:
106+
'(If Statement) should fail if a signal is not invoked as part of a comparison',
107+
annotatedSource: `
108+
const aSignal = createSignal("hello");
109+
if (aSignal == "hello") {
110+
~~~~~~~
111+
}
112+
declare function createSignal(): Signal<boolean>;
113+
interface Signal<T> {}
114+
`,
115+
messageId,
116+
}),
117+
convertAnnotatedSourceToFailureCase({
118+
description:
119+
'(If Statement) should fail if the signal is not invoked deep in an expression',
120+
annotatedSource: `
121+
const aSignal = createSignal();
122+
if (false || (aSignal ?? true)) {
123+
~~~~~~~
124+
}
125+
declare function createSignal(): Signal<boolean>;
126+
interface Signal<T> {}
127+
`,
128+
messageId,
129+
}),
130+
// Conditional Expressions
131+
convertAnnotatedSourceToFailureCase({
132+
description:
133+
'(conditional expression) should fail if the signal is not invoked as the only expression',
134+
annotatedSource: `
135+
const aSignal = createSignal();
136+
const v = aSignal ? true : false;
137+
~~~~~~~
138+
declare function createSignal(): Signal<boolean>;
139+
interface Signal<T> {}
140+
`,
141+
messageId,
142+
}),
143+
convertAnnotatedSourceToFailureCase({
144+
description:
145+
'(conditional expression) should fail if the signal is not invoked as part of a logical expression',
146+
annotatedSource: `
147+
const aSignal = createSignal(false);
148+
const v = (aSignal || true) ? true : false;
149+
~~~~~~~
150+
declare function createSignal(): Signal<boolean>;
151+
interface Signal<T> {}
152+
`,
153+
messageId,
154+
}),
155+
convertAnnotatedSourceToFailureCase({
156+
description:
157+
'(conditional expression) should fail if a signal is not invoked as part of a comparison',
158+
annotatedSource: `
159+
const aSignal = createSignal("hello");
160+
const v = (aSignal == "hello") ? true : false;
161+
~~~~~~~
162+
declare function createSignal(): Signal<boolean>;
163+
interface Signal<T> {}
164+
`,
165+
messageId,
166+
}),
167+
convertAnnotatedSourceToFailureCase({
168+
description:
169+
'(conditional expression) should fail if the signal is not invoked deep in an expression',
170+
annotatedSource: `
171+
const aSignal = createSignal();
172+
const v = (false || (aSignal ?? true)) ? true : false
173+
~~~~~~~
174+
declare function createSignal(): Signal<boolean>;
175+
interface Signal<T> {}
176+
`,
177+
messageId,
178+
}),
179+
];

‎packages/eslint-plugin/tests/rules/no-uncalled-signals/project/file.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
// Used for type-checked tests.

‎packages/eslint-plugin/tests/rules/no-uncalled-signals/project/tsconfig.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"compilerOptions": {
3+
"strict": true
4+
},
5+
"include": ["file.ts"]
6+
}

‎packages/eslint-plugin/tests/rules/no-uncalled-signals/spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { RuleTester } from '@angular-eslint/test-utils';
2+
import path from 'node:path';
3+
import rule, { RULE_NAME } from '../../../src/rules/no-uncalled-signals';
4+
import { invalid, valid } from './cases';
5+
6+
const ruleTester = new RuleTester({
7+
languageOptions: {
8+
parserOptions: {
9+
projectService: true,
10+
tsconfigRootDir: path.join(__dirname, 'project'),
11+
},
12+
},
13+
});
14+
15+
ruleTester.run(RULE_NAME, rule, {
16+
valid,
17+
invalid,
18+
});

0 commit comments

Comments
 (0)
0