-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[pylint
] Include parentheses and multiple comparators in check for boolean-chained-comparison (PLR1716)
#14781
Conversation
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
PLR1716 | 2 | 0 | 0 | 2 | 0 |
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 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. I suggest adding some more tests with comments because the comment range (or parentheses for that fact) isn't part of the expression range
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)) < c
becomesa < (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
and
between 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 < c
used to result ina<(b<c
which gives a syntax error. It could in theory have been fixed in a separate PR, but seems to be on theme here.