[flake8-simplify] Preserve operand order in SIM109 fix - #27824
Merged
Merged
Conversation
The compare-with-tuple fix always placed the merged `in` comparison first, so an unmatched operand appearing before the merged comparisons in the original expression ended up after them instead, changing the expression's value under short-circuit evaluation (e.g. `x or y == z or y == w` became `y in (z, w) or x`). The merged comparison is now anchored to the position of its earliest matched comparator and sorted together with the unmatched operands, so the original order is preserved except when an unmatched operand sits between two merged comparisons, which is now documented as a known limitation of the fix. Fixes astral-sh#18945
ntBre
approved these changes
Sep 15, 2026
ntBre
left a comment
Contributor
There was a problem hiding this comment.
Thank you! And sorry for the delay. I resolved the merge conflicts and then applied a very minor simplification with itertools, but this looked good to me!
flake8-simplify] Preserve operand order in SIM109 fix
ntBre
enabled auto-merge (squash)
September 15, 2026 19:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #18945.
compare-with-tuple(SIM109) merges repeated equality checks against the same target into a singleincomparison, e.g.a == b or a == cbecomesa in (b, c). When the boolean expression also contains unrelated operands, the fix always placed the mergedincomparison first and appended the unmatched operands after it, regardless of where they originally appeared. That reorders the expression and can change its value under short-circuit evaluation:was fixed to:
Fix
The merged
incomparison is now anchored to the position of its earliest matched comparator, and sorted together with the unmatched operands by position instead of always being placed first. This preserves the original order for operands that appear before or after the group of merged comparisons.There's one case this doesn't fully solve: if an unmatched operand sits between two merged comparisons (e.g.
a == b or None or a == c), there's no reordering that keeps both the merged comparison contiguous and every operand in its original position, so it's still moved after the merged comparison. I've documented this as a known limitation in the rule's## Fix safetysection, along with a regression test that pins down the current (documented) behavior.This follows the approach @ntBre sketched in review on the earlier attempt at this fix (#19753, closed for inactivity).
Test plan
SIM109.pywith a regression case for the exact scenario from the issue (unmatched operand before the merged comparisons) and kept/annotated the existing case that demonstrates the documented limitation (unmatched operand between merged comparisons).cargo dev generate-all(doc comment change).