[pylint] Include parentheses and multiple comparators in check for boolean-chained-comparison (PLR1716)#14781
Merged
dylwil3 merged 14 commits intoastral-sh:mainfrom Dec 9, 2024
Merged
Conversation
Contributor
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PLR1716 | 2 | 0 | 0 | 2 | 0 |
Collaborator
Author
|
No obligation to review @zanieb , but figured I'd ping you since you wrote the first PR addressing this issue. Let me know if this fix is way off base from what you had in mind 😄 |
MichaReiser
approved these changes
Dec 6, 2024
crates/ruff_linter/resources/test/fixtures/pylint/boolean_chained_comparison.py
Show resolved
Hide resolved
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.
This PR introduces three changes to the diagnostic and fix behavior (still under preview) for boolean-chained-comparison (PLR1716).
(a < b) and b < c. The fix will merge the chains of comparisons and then balance parentheses by adding parentheses to one side of the expression.a < b < c and c < d.a < (b) and ((b)) < cbecomesa < (b) < c.While these seem like somewhat disconnected changes, they are actually related. If we only offered (1), then we would see the following fix behavior:
This is because the fix which add parentheses to the first pair of comparisons overlaps with the fix that removes the
andbetween the second two comparisons. So the latter fix is deferred. However, the latter fix does not get a second chance because, upon the next lint iteration, there is no violation ofPLR1716.Upon adopting (2), however, both fixes occur by the time ruff completes several iterations and we get:
Finally, (3) fixes a previously unobserved bug wherein the autofix for
a < (b) and b < cused to result ina<(b<cwhich gives a syntax error. It could in theory have been fixed in a separate PR, but seems to be on theme here.