Add `prefer-simple-condition-first` rule by costajohnt · Pull Request #2902 · sindresorhus/eslint-plugin-unicorn

costajohnt · 2026-02-24T01:03:39Z

Adds a new rule that flags logical expressions (&&, ||) where a simple condition appears on the right but a complex one appears on the left, and offers to swap them.

What's "simple"

Per fisker's scoping in #2853 (comment):

Bare identifier (foo)
identifier === literal or identifier !== literal (strict equality/inequality between identifiers and/or literals)

Everything else is considered "complex."

Fix safety

Auto-fix when the complex side has no function calls — safe to reorder
Suggestion when the complex side contains CallExpression or NewExpression — reordering changes short-circuit evaluation timing, as noted by @axetroy

Examples

// ❌
if (check(foo) && bar);
if (foo.bar.baz === 1 && bar === 2);

// ✅
if (bar && check(foo));
if (bar === 2 && foo.bar.baz === 1);

sindresorhus · 2026-02-24T05:35:53Z

The auto-fix currently runs whenever the left side is "complex" and has no call/new expression. That still includes expressions like assignment/update/member chains, where swapping sides can change behvaior (or whether an exception is thrown because of short-circuiting order).

Examples:

(state.ready = true) && ok -> ok && (state.ready = true)
object.deep.value && ok -> ok && object.deep.value

Because of that, these should be suggestion-only, not auto-fixed.

Could we also add tests for these non-call side-effect/throwing cases? The current tests do not cover them, so this would be easy to miss.

sindresorhus · 2026-02-24T05:41:33Z

+		'if (foo);',
+
+		// Simple on left, complex on right — correct order
+		'if (bar || foo());',


Suggested change

'if (bar || foo());',

'if (bar || foo());',

// Potentially unsafe to reorder (side effects / throws)

'if ((state.ready = true) && ok);',

'if (++counter && ok);',

'if (object.deep.value && ok);',

'const x = object.deep.value || ok;',

'if (tag`x` && ok);',

costajohnt · 2026-03-22T01:46:31Z

Updated to follow your inline suggestion — side-effect patterns (assignments, updates, deep member chains, tagged templates) are now skipped entirely instead of flagged as suggestions. Calls/new still get suggestion-only treatment.

fisker · 2026-03-22T02:51:11Z

 	}

-	if (!isStatic || isPrivate || key.type === 'PrivateIdentifier') {
+	if (isPrivate || !isStatic || key.type === 'PrivateIdentifier') {


Let's consider !foo as "simple"?

Done, !identifier is now treated as simple. Reverted the swaps in no-static-only-class and text-encoding-identifier-case since both sides are simple with this change.

sindresorhus · 2026-03-25T13:23:38Z

A few things to address still:

Negative numeric literals aren't treated as simple

In the AST, -1 is a UnaryExpression wrapping a Literal, not a Literal itself. So x === -1 is considered "not simple" by isSimple(), which only checks for Identifier or Literal on each side.

This directly causes the unnecessary swap in the prefer-at rule:

// Both sides are conceptually simple comparisons, but this gets flagged:
(startIndex === -1 && firstElementGetMethod === 'pop')

Fix: in the BinaryExpression branch of isSimple, also treat unary -/+ with a Literal argument as simple (since these are just numeric literals in the AST).

AwaitExpression / YieldExpression not caught by the side-effect check

(await promise) && bar would be auto-fixed to bar && (await promise), which changes when the await happens. If bar is falsy the await gets skipped entirely. Neither hasSideEffectsOrThrows nor hasCallOrNew catches these.

Should add AwaitExpression and YieldExpression to hasSideEffectsOrThrows.

Missing test cases

Would be good to add tests for:

(await foo) && bar / (yield foo) && bar
x === -1 && y (negative literal)
Optional chaining: a?.b && c

Minor: the test/package.js swap looks like unecessary churn

Since the dogfooding config disables the rule, this swap shouldn't have been needed. Looks like it was auto-fixed before the dogfooding disable was added? I'd revert it.

…on-first - Use getParenthesizedText/getParenthesizedRange to preserve parentheses when swapping conditions, preventing precedence changes - Add shouldAddParenthesesToLogicalExpressionChild for readability parens - Detect TaggedTemplateExpression as call-like (suggestion, not auto-fix) - Add test cases for parenthesized expressions, unary, tagged templates, and nullish coalescing operator

- Skip expressions with side effects (assignments, updates, deep member chains, tagged templates) entirely - Prevent auto-fix oscillation in chained logical expressions by treating all-simple chains as simple - Use suggestion instead of auto-fix for mixed chains and calls - Auto-fix simple condition order across codebase - Disable rule in dogfooding config (suggestion-only violations remain in fallback || patterns)

- Revert two || operand swaps in remove-argument.js and prefer-object-from-entries.js that broke fallback patterns (parentheses[0] must be tried first, node/initObject is the fallback) - Merge hasCall and hasSideEffectsOrThrows into single isUnsafeToAutoFix function, eliminating duplicated traversal - Make side-effect patterns report as suggestion-only instead of silently skipping (assignment, update, deep member, tagged template) - Add test cases for nested side effects to verify recursive detection

…ggestion Follow the inline code suggestion: expressions with assignments, updates, deep member chains, or tagged templates are now not flagged at all, matching the documented behavior ("not flagged, since reordering would change program behavior"). Calls/new remain as suggestion-only. - Split isUnsafeToAutoFix into hasSideEffectsOrThrows (skip) and hasCallOrNew (suggestion only) - Move 9 test cases from invalid to valid - Update dogfooding config comment

Per review feedback, negated identifiers like `!foo` are now considered simple. Reverts dogfooding swaps in no-static-only-class and text-encoding-identifier-case where both sides were already simple.

Now that `!identifier` is simple, these dogfooding reorderings are no longer flagged by the rule. Restores original condition order.

- Handle negative/positive numeric literals as simple operands in isSimple() - Add AwaitExpression, YieldExpression, ImportExpression to hasSideEffectsOrThrows() - Add test cases for await, yield, import(), negative literals, optional chaining - Revert unnecessary test/package.js swap

sindresorhus · 2026-03-26T16:59:29Z

Some of the auto-fixes change runtime behavior for expressions that can throw even without side effects. For example, if (a.b && c) {} is auto-fixed to if (c && a.b) {}. With let a; const c = false;, the original throws, while the fixed version does not. I verified the same problem with a[b] and foo in bar. So the current safety check still seems too narrow.
The rule also runs on value-producing logical expressions, not just boolean conditions. For example, it reports const x = foo() || bar; and suggests bar || foo(). But that changes the result, not just evaluation order: if foo() returns 'left' and bar is 'right', the original evaluates to 'left' while the suggested version evaluates to 'right'. So I think this still needs some kind of boolean-context guard, otherwise it will keep flagging fallback patterns incorectly.

sindresorhus · 2026-03-26T17:00:21Z

Some more tests I would add:

A valid case for shallow member access that can throw:
if (a.b && c);
This should stay unreported for the same reason object.deep.value stays unreported.
A valid case for computed member access:
if (a[b] && c);
A valid case for throwing binary operators:
if (foo in bar && baz);
if (foo instanceof bar && baz);
A valid case for value-producing || fallback:
const x = foo() || bar;
That should not be reported if the rule is meant to preserve semantics.
Probably also a valid case for value-producing &&:
const x = a.b && c;
Same reason: outside boolean context, reordering can change the produced value.

- Treat any non-optional MemberExpression, `in`, and `instanceof` as potentially throwing (not just deep member chains) - Only flag LogicalExpressions in boolean contexts (if/while/for/ ternary/!) — value-producing contexts are excluded since reordering changes the result - Move affected test cases from invalid to valid - Add test cases for shallow member access, computed access, in/instanceof, value-producing contexts, and non-if boolean contexts (while, for, ternary)

sindresorhus reviewed Feb 24, 2026

View reviewed changes

costajohnt force-pushed the prefer-simple-condition-first branch from 1ba694e to 75b5e71 Compare February 24, 2026 15:41

costajohnt force-pushed the prefer-simple-condition-first branch from 3dc9a16 to e17e0c2 Compare March 5, 2026 01:19

costajohnt mentioned this pull request Mar 5, 2026

Detect when "needs_response" feedback has already been addressed by subsequent commits costajohnt/oss-autopilot#561

Closed

fisker reviewed Mar 22, 2026

View reviewed changes

costajohnt and others added 12 commits March 25, 2026 16:02

feat: add prefer-simple-condition-first rule

614f21c

docs: add prefer-simple-condition-first documentation

82a50c6

Update prefer-simple-condition-first.md

88e5da0

Update snapshots

ef13499

Update snapshots

41f6a54

Treat !identifier as simple

7ac86a8

Per review feedback, negated identifiers like `!foo` are now considered simple. Reverts dogfooding swaps in no-static-only-class and text-encoding-identifier-case where both sides were already simple.

Update snapshots

9812d59

Revert remaining negated-identifier swaps

70806e6

Now that `!identifier` is simple, these dogfooding reorderings are no longer flagged by the rule. Restores original condition order.

costajohnt force-pushed the prefer-simple-condition-first branch from 5195329 to 70806e6 Compare March 25, 2026 23:02

costajohnt and others added 2 commits March 25, 2026 20:26

Update snapshots

344c8e7

costajohnt and others added 2 commits March 26, 2026 20:11

Update snapshots

1e9d9c8

sindresorhus merged commit b0279dd into sindresorhus:main Mar 27, 2026
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `prefer-simple-condition-first` rule#2902

Add `prefer-simple-condition-first` rule#2902
sindresorhus merged 16 commits into
sindresorhus:mainfrom
costajohnt:prefer-simple-condition-first

costajohnt commented Feb 24, 2026 •

edited by sindresorhus

Loading

Uh oh!

sindresorhus commented Feb 24, 2026

Uh oh!

sindresorhus Feb 24, 2026

Uh oh!

costajohnt commented Mar 22, 2026

Uh oh!

fisker Mar 22, 2026

Uh oh!

costajohnt Mar 22, 2026

Uh oh!

sindresorhus commented Mar 25, 2026

Uh oh!

sindresorhus commented Mar 26, 2026

Uh oh!

sindresorhus commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-		'if (bar || foo());',
+		'if (bar || foo());',
+		// Potentially unsafe to reorder (side effects / throws)
+		'if ((state.ready = true) && ok);',
+		'if (++counter && ok);',
+		'if (object.deep.value && ok);',
+		'const x = object.deep.value || ok;',
+		'if (tag`x` && ok);',

Uh oh!

Conversation

costajohnt commented Feb 24, 2026 • edited by sindresorhus Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's "simple"

Fix safety

Examples

Uh oh!

sindresorhus commented Feb 24, 2026

Uh oh!

sindresorhus Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

costajohnt commented Mar 22, 2026

Uh oh!

fisker Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

costajohnt Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

sindresorhus commented Mar 25, 2026

Uh oh!

sindresorhus commented Mar 26, 2026

Uh oh!

sindresorhus commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

costajohnt commented Feb 24, 2026 •

edited by sindresorhus

Loading