Skip to content

Commit 55db479

Browse files
authored
feat: add checkConditionalExpressions to no-unmodified-loop-condition (#21175)
* feat: add checkConditionalExpressions to `no-unmodified-loop-condition` * apply suggestions
1 parent 585ef37 commit 55db479

4 files changed

Lines changed: 83 additions & 9 deletions

File tree

‎docs/src/rules/no-unmodified-loop-condition.md‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ while (node) {
2525
This rule finds references which are inside of loop conditions, then checks the
2626
variables of those references are modified in the loop.
2727

28-
If a reference is inside of a binary expression or a ternary expression, this rule checks the result of
29-
the expression instead.
28+
By default, if a reference is inside of a binary expression or a ternary expression, this rule checks
29+
the result of the expression instead. The `checkConditionalExpressions` option stops the rule from
30+
grouping references solely because they belong to the same ternary expression.
3031
If a reference is inside of a dynamic expression (e.g. `CallExpression`,
3132
`YieldExpression`, ...), this rule ignores it.
3233

@@ -99,7 +100,27 @@ while (check(obj)) {
99100

100101
## Options
101102

102-
This rule has no options.
103+
This rule has an object option:
104+
105+
* `"checkConditionalExpressions": false` (default) checks the result of an entire ternary expression
106+
* `"checkConditionalExpressions": true` each branch in a ternary expression is checked independently
107+
108+
Examples of **incorrect** code for this rule with the `{ "checkConditionalExpressions": true }` option:
109+
110+
::: incorrect
111+
112+
```js
113+
/* eslint no-unmodified-loop-condition: ["error", { "checkConditionalExpressions": true }] */
114+
115+
let chunk = getInitialChunk();
116+
let done = false;
117+
118+
while (chunk ? !done : false) {
119+
chunk = nextOrNull();
120+
}
121+
```
122+
123+
:::
103124

104125
## When Not To Use It
105126

‎lib/rules/no-unmodified-loop-condition.js‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ const Traverser = require("../shared/traverser"),
2626
const SENTINEL_PATTERN =
2727
/(?:(?:Call|Class|Function|Member|New|Yield)Expression|Statement|Declaration)$/u;
2828
const LOOP_PATTERN = /^(?:DoWhile|For|While)Statement$/u; // for-in/of statements don't have `test` property.
29-
const GROUP_PATTERN = /^(?:BinaryExpression|ConditionalExpression)$/u;
3029
const SKIP_PATTERN = /^(?:ArrowFunction|Class|Function)Expression$/u;
3130
const DYNAMIC_PATTERN = /^(?:Call|Member|New|TaggedTemplate|Yield)Expression$/u;
3231

@@ -190,7 +189,19 @@ module.exports = {
190189
url: "https://eslint.org/docs/latest/rules/no-unmodified-loop-condition",
191190
},
192191

193-
schema: [],
192+
schema: [
193+
{
194+
type: "object",
195+
properties: {
196+
checkConditionalExpressions: {
197+
type: "boolean",
198+
},
199+
},
200+
additionalProperties: false,
201+
},
202+
],
203+
204+
defaultOptions: [{ checkConditionalExpressions: false }],
194205

195206
messages: {
196207
loopConditionNotModified:
@@ -199,6 +210,7 @@ module.exports = {
199210
},
200211

201212
create(context) {
213+
const [{ checkConditionalExpressions }] = context.options;
202214
const sourceCode = context.sourceCode;
203215
let groupMap = null;
204216

@@ -305,10 +317,14 @@ module.exports = {
305317
}
306318

307319
/*
308-
* If it's inside of a group, OK if either operand is modified.
309-
* So stores the group this reference belongs to.
320+
* If it's inside of a group, OK if any reference in that group is
321+
* modified. So stores the group this reference belongs to.
310322
*/
311-
if (GROUP_PATTERN.test(node.type)) {
323+
if (
324+
node.type === "BinaryExpression" ||
325+
(!checkConditionalExpressions &&
326+
node.type === "ConditionalExpression")
327+
) {
312328
// If this expression is dynamic, no need to check.
313329
if (hasDynamicExpressions(node)) {
314330
break;

‎lib/types/rules.d.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4040,7 +4040,16 @@ export interface ESLintRules extends Linter.RulesRecord {
40404040
* @since 2.0.0-alpha-2
40414041
* @see https://eslint.org/docs/latest/rules/no-unmodified-loop-condition
40424042
*/
4043-
"no-unmodified-loop-condition": Linter.RuleEntry<[]>;
4043+
"no-unmodified-loop-condition": Linter.RuleEntry<
4044+
[
4045+
Partial<{
4046+
/**
4047+
* @default false
4048+
*/
4049+
checkConditionalExpressions: boolean;
4050+
}>,
4051+
]
4052+
>;
40444053

40454054
/**
40464055
* Rule to disallow ternary operators when simpler alternatives exist.

‎tests/lib/rules/no-unmodified-loop-condition.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ ruleTester.run("no-unmodified-loop-condition", rule, {
5858
"var foo = 0, bar = 0; for (bar; foo;) { ++foo }",
5959
"var foo; if (foo) { }",
6060
"var a = [1, 2, 3]; var len = a.length; for (var i = 0; i < len - 1; i++) {}",
61+
{
62+
code: "let foo = 0, bar = 1, baz = 2; while (foo ? bar : baz) { foo += 1; bar += 1; baz += 1; }",
63+
options: [{ checkConditionalExpressions: true }],
64+
},
6165
],
6266
invalid: [
6367
{
@@ -161,5 +165,29 @@ ruleTester.run("no-unmodified-loop-condition", rule, {
161165
},
162166
],
163167
},
168+
{
169+
code: "let foo = 0, bar = 1, baz = 2; while (foo ? bar : baz) { foo += 1; }",
170+
options: [{ checkConditionalExpressions: true }],
171+
errors: [
172+
{
173+
messageId: "loopConditionNotModified",
174+
data: { name: "bar" },
175+
},
176+
{
177+
messageId: "loopConditionNotModified",
178+
data: { name: "baz" },
179+
},
180+
],
181+
},
182+
{
183+
code: "let chunk = true, done = false; while (chunk ? !done : false) { chunk = nextOrNull(); }",
184+
options: [{ checkConditionalExpressions: true }],
185+
errors: [
186+
{
187+
messageId: "loopConditionNotModified",
188+
data: { name: "done" },
189+
},
190+
],
191+
},
164192
],
165193
});

0 commit comments

Comments
 (0)