-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
] Correct ordering of arguments in fix for if-stmt-min-max
(PLR1730
)
#16080
Conversation
|
pylint
] Correct ordering of arguments in fix for if-stmt-min-max
(PLR1730
)
else if cmp_left == assignment_value && cmp_right == target { | ||
match op { | ||
CmpOp::Lt => (MinMax::Min, true), | ||
CmpOp::LtE => (MinMax::Min, true), |
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.
I think this is incorrect and should instead be false
to match your table from the PR summary.
Please also carefully review the snapshot changes.
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.
In this case where are in the situation:
if a <= b:
b = a
which is fixed by b = min(a, b)
.
So you are right ;-)
Please also carefully review the snapshot changes.
I corrected the snapshot and I double-checked all the other changes I did. Everything follow the table posted in the description of the PR.
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.
Thank you
The PR addresses the issue #16040 .
The logic used into the rule is the following:
Suppose to have an expression of the form
where
a
,b
,c
andd
are Python obj andcmp
one of<
,>
,<=
,>=
.Then:
if a=c and b=d
<=
fix witha = max(b, a)
>=
fix witha = min(b, a)
>
fix witha = min(a, b)
<
fix witha = max(a, b)
if a=d and b=c
<=
fix withb = min(a, b)
>=
fix withb = max(a, b)
>
fix withb = max(b, a)
<
fix withb = min(b, a)
do nothing, i.e., we cannot fix this case.
In total we have 8 different and possible cases.
I added them in the tests.
Please double-check that I didn't make any mistakes. It's quite easy to mix up > and <.