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

Commit 41eaa68

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 c69c743 commit 41eaa68

File tree

15 files changed

+278
-12
lines changed

15 files changed

+278
-12
lines changed

‎packages/angular-eslint/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "angular-eslint",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"description": "The tooling which enables ESLint to work with Angular projects",
55
"license": "MIT",
66
"main": "dist/index.js",

‎packages/builder/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@angular-eslint/builder",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"description": "Angular CLI builder for ESLint",
55
"license": "MIT",
66
"main": "dist/index.js",

‎packages/bundled-angular-compiler/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@angular-eslint/bundled-angular-compiler",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"description": "A CJS bundled version of @angular/compiler",
55
"license": "MIT",
66
"main": "dist/index.js",

‎packages/eslint-plugin-template/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@angular-eslint/eslint-plugin-template",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"description": "ESLint plugin for Angular Templates",
55
"license": "MIT",
66
"main": "dist/index.js",

‎packages/eslint-plugin/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@angular-eslint/eslint-plugin",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"description": "ESLint plugin for Angular applications, following https://angular.dev/style-guide",
55
"license": "MIT",
66
"main": "dist/index.js",

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ import noAttributeDecorator, {
3434
import noConflictingLifecycle, {
3535
RULE_NAME as noConflictingLifecycleRuleName,
3636
} from './rules/no-conflicting-lifecycle';
37+
import noDuplicatesInMetadataArrays, {
38+
RULE_NAME as noDuplicatesInMetadataArraysRuleName,
39+
} from './rules/no-duplicates-in-metadata-arrays';
3740
import noEmptyLifecycleMethod, {
3841
RULE_NAME as noEmptyLifecycleMethodRuleName,
3942
} from './rules/no-empty-lifecycle-method';
@@ -52,6 +55,9 @@ import noInputsMetadataProperty, {
5255
import noLifecycleCall, {
5356
RULE_NAME as noLifecycleCallRuleName,
5457
} from './rules/no-lifecycle-call';
58+
import noMisusedSignals, {
59+
RULE_NAME as noMisusedSignalsRuleName,
60+
} from './rules/no-misused-signals';
5561
import noOutputNative, {
5662
RULE_NAME as noOutputNativeRuleName,
5763
} from './rules/no-output-native';
@@ -118,9 +124,6 @@ import useLifecycleInterface, {
118124
import usePipeTransformInterface, {
119125
RULE_NAME as usePipeTransformInterfaceRuleName,
120126
} from './rules/use-pipe-transform-interface';
121-
import noDuplicatesInMetadataArrays, {
122-
RULE_NAME as noDuplicatesInMetadataArraysRuleName,
123-
} from './rules/no-duplicates-in-metadata-arrays';
124127

125128
export = {
126129
configs: {
@@ -146,6 +149,7 @@ export = {
146149
[noInputRenameRuleName]: noInputRename,
147150
[noInputsMetadataPropertyRuleName]: noInputsMetadataProperty,
148151
[noLifecycleCallRuleName]: noLifecycleCall,
152+
[noMisusedSignalsRuleName]: noMisusedSignals,
149153
[noOutputNativeRuleName]: noOutputNative,
150154
[noOutputOnPrefixRuleName]: noOutputOnPrefix,
151155
[noOutputRenameRuleName]: noOutputRename,

‎packages/eslint-plugin/src/rules/no-misused-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 = 'noMisusedSignals';
11+
export const RULE_NAME = 'no-misused-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+
noMisusedSignals:
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: 'noMisusedSignals',
53+
});
54+
}
55+
},
56+
};
57+
},
58+
});

‎packages/eslint-plugin/tests/rules/no-misused-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-misused-signals';
7+
8+
const messageId: MessageIds = 'noMisusedSignals';
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+
convertAnnotatedSourceToFailureCase({
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> {}
 10000
176+
`,
177+
messageId,
178+
}),
179+
];

‎packages/eslint-plugin/tests/rules/no-misused-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-misused-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-misused-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-misused-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+
});

‎packages/schematics/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@angular-eslint/schematics",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"description": "Angular Schematics for angular-eslint",
55
"license": "MIT",
66
"main": "dist/index.js",

‎packages/template-parser/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@angular-eslint/template-parser",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"description": "Angular Template parser for ESLint",
55
"license": "MIT",
66
"main": "dist/index.js",

‎packages/test-utils/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@angular-eslint/test-utils",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"license": "MIT",
55
"main": "dist/index.js",
66
"types": "dist/index.d.ts",

‎packages/utils/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@angular-eslint/utils",
3-
"version": "19.3.0",
3+
"version": "0.0.0-e2e",
44
"license": "MIT",
55
"main": "dist/index.js",
66
"types": "dist/index.d.ts",

0 commit comments

Comments
 (0)
0