Skip to content
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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/whatsnew/fragments/7761.false_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Apply ``infer_kwarg_from_call()`` to more checks

These mostly solve false negatives for various checks,
save for one false positive for ``use-maxsplit-arg``.

Closes #7761
16 changes: 13 additions & 3 deletions pylint/checkers/refactoring/recommendation_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,22 @@ def _check_use_maxsplit_arg(self, node: nodes.Call) -> None:
):
return

confidence = HIGH
try:
sep = utils.get_argument_from_call(node, 0, "sep")
except utils.NoSuchArgumentError:
return
sep = utils.infer_kwarg_from_call(node, keyword="sep")
confidence = INFERENCE
if not sep:
return

try:
# Ignore if maxsplit arg has been set
utils.get_argument_from_call(node, 1, "maxsplit")
return
except utils.NoSuchArgumentError:
pass
if utils.infer_kwarg_from_call(node, keyword="maxsplit"):
return

if isinstance(node.parent, nodes.Subscript):
try:
Expand Down Expand Up @@ -171,7 +176,12 @@ def _check_use_maxsplit_arg(self, node: nodes.Call) -> None:
+ new_fn
+ f"({sep.as_string()}, maxsplit=1)[{subscript_value}]"
)
self.add_message("use-maxsplit-arg", node=node, args=(new_name,))
self.add_message(
"use-maxsplit-arg",
node=node,
args=(new_name,),
confidence=confidence,
)

@utils.only_required_for_messages(
"consider-using-enumerate",
Expand Down
11 changes: 10 additions & 1 deletion pylint/checkers/refactoring/refactoring_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2203,12 +2203,14 @@ def _check_unnecessary_list_index_lookup(
):
return

preliminary_confidence = HIGH
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
Copy link
Member

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 ?

Copy link
Member Author

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.

Copy link
Member Author

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.


if not isinstance(iterable_arg, nodes.Name):
return
Expand All @@ -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
Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a __add__ ?

Copy link
Member Author

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.

confidence = (
preliminary_confidence
if preliminary_confidence == INFERENCE
else confidence
)

iterating_object_name = iterable_arg.name
value_variable = node.target.elts[1]

Expand Down
2 changes: 1 addition & 1 deletion pylint/checkers/stdlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ def _check_lru_cache_decorators(self, node: nodes.FunctionDef) -> None:
d_node, position=0, keyword="maxsize"
)
except utils.NoSuchArgumentError:
break
arg = utils.infer_kwarg_from_call(d_node, "maxsize")

if not isinstance(arg, nodes.Const) or arg.value is not None:
break
Expand Down
5 changes: 5 additions & 0 deletions tests/functional/m/method_cache_max_size_none.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ def my_func(self, param):
def my_func(self, param):
return param + 1

maxsizeKwarg = {"maxsize": None}
@lru_cache(**maxsizeKwarg) # [method-cache-max-size-none]
def my_func(self, param):
return param + 1


class MyClassWithMethodsAndMaxSize:
@lru_cache(maxsize=1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ def return_start(start):

result = [my_list[idx] for idx, val in enumerate(iterable=my_list)] # [unnecessary-list-index-lookup]

iterable_kwarg = {"iterable": my_list}
result = [my_list[idx] for idx, val in enumerate(**iterable_kwarg)] # [unnecessary-list-index-lookup]

for idx, val in enumerate():
print(my_list[idx])

Expand Down
4 changes: 4 additions & 0 deletions tests/functional/u/use/use_maxsplit_arg.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@


# Test varying maxsplit argument -- all these will be okay
maxSplit = {"maxSplit": 2}
# ## str.split() tests
good_split = '1,2,3'.split(sep=',', maxsplit=1)[-1]
good_split = '1,2,3'.split(sep=',', maxsplit=1)[0]
good_split = '1,2,3'.split(sep=',', maxsplit=2)[-1]
good_split = '1,2,3'.split(sep=',', maxsplit=2)[0]
good_split = '1,2,3'.split(sep=',', maxsplit=2)[1]
good_split = '1,2,3'.split(sep=',', **maxSplit)[1]

# ## str.rsplit() tests
good_split = '1,2,3'.rsplit(sep=',', maxsplit=1)[-1]
Expand Down Expand Up @@ -91,9 +93,11 @@ class Bar():
print(source.split('.')[i])
i = i + 1

sepNone = {"sep": None}
# Test for crash when sep is given by keyword
# https://github.com/pylint-dev/pylint/issues/5737
get_last = SEQ.split(sep=None)[-1] # [use-maxsplit-arg]
get_last = SEQ.split(**sepNone)[-1] # [use-maxsplit-arg]


class FalsePositive4857:
Expand Down