-
-
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
Apply infer_kwarg_from_call()
to more checks
#8775
Apply infer_kwarg_from_call()
to more checks
#8775
Conversation
These mostly solve false negatives for various checks, save for one false positive for `use-maxsplit-arg`. Closes pylint-dev#7761
@@ -2228,6 +2230,13 @@ def _check_unnecessary_list_index_lookup( | |||
# is not redundant, hence we should not report an error. | |||
return | |||
|
|||
# Preserve preliminary_confidence if it was INFERENCE |
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.
Not terribly important, but maybe someday a merge_confidence()
helper would be a usability win.
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.
What about a __add__
?
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.
It should just wait for #7121, when we might have a numeric level we can just compare directly.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8775 +/- ##
=======================================
Coverage 95.83% 95.83%
=======================================
Files 173 173
Lines 18451 18459 +8
=======================================
+ Hits 17683 17691 +8
Misses 768 768
|
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.
Just a nit, looks great !
try: | ||
iterable_arg = utils.get_argument_from_call( | ||
node.iter, position=0, keyword="iterable" | ||
) | ||
except utils.NoSuchArgumentError: | ||
return | ||
iterable_arg = utils.infer_kwarg_from_call(node.iter, keyword="iterable") | ||
preliminary_confidence = INFERENCE |
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.
At this point it's not preliminary anymore so it c/sould be named confidence ?
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.
It is preliminary if the second one turns out to be CONTROL_FLOW, was my thought. But then I should change the second condition to != HIGH.
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.
Well, that's not quite right either. I guess I'd rather keep preliminary for now.
@@ -2228,6 +2230,13 @@ def _check_unnecessary_list_index_lookup( | |||
# is not redundant, hence we should not report an error. | |||
return | |||
|
|||
# Preserve preliminary_confidence if it was INFERENCE |
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.
What about a __add__
?
Thanks for the review! |
Closes #7761
Type of Changes
Description
Follow-up to #8728, which created
infer_kwarg_from_call()
. I looked for other places we should use to close #7761.These mostly solve false negatives for various checks, save for one false positive for
use-maxsplit-arg
. Not worth pulling it out to backport.I did open a separate issue for #8774, though, because it's a crash fix we could backport.
Closes #7761