Skip to content

Commit

Permalink
Apply infer_kwarg_from_call() to more checks (#8775)
Browse files Browse the repository at this point in the history
These mostly solve false negatives for various checks,
save for one false positive for `use-maxsplit-arg`.
  • Loading branch information
jacobtylerwalls authored Jun 14, 2023
1 parent bafb229 commit 507dfc5
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 28 deletions.
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

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
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
3 changes: 2 additions & 1 deletion tests/functional/m/method_cache_max_size_none.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ method-cache-max-size-none:34:5:34:38:MyClassWithMethods.my_func:'lru_cache(maxs
method-cache-max-size-none:38:5:38:24:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:43:5:43:24:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:44:5:44:24:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:74:5:74:40:MyClassWithMethodsAndMaxSize.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:49:5:49:30:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:79:5:79:40:MyClassWithMethodsAndMaxSize.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ unnecessary-list-index-lookup:119:10:119:21::Unnecessary list index lookup, use
unnecessary-list-index-lookup:122:10:122:21::Unnecessary list index lookup, use 'val' instead:INFERENCE
unnecessary-list-index-lookup:135:10:135:21::Unnecessary list index lookup, use 'val' instead:HIGH
unnecessary-list-index-lookup:137:10:137:22::Unnecessary list index lookup, use 'val' instead:HIGH
unnecessary-list-index-lookup:140:10:140:22::Unnecessary list index lookup, use 'val' instead:INFERENCE
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
max_split = {"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=',', **max_split)[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
45 changes: 23 additions & 22 deletions tests/functional/u/use/use_maxsplit_arg.txt
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
use-maxsplit-arg:5:12:5:30::Use '1,2,3'.split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:6:11:6:35::"Use '1,2,3'[::-1].split(',', maxsplit=1)[0] instead":UNDEFINED
use-maxsplit-arg:9:12:9:26::Use SEQ.split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:10:11:10:25::Use SEQ.rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:11:12:11:27::Use SEQ.split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:12:11:12:26::Use SEQ.rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:45:12:45:36::Use Foo.class_str.split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:46:11:46:35::Use Foo.class_str.rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:47:12:47:37::Use Foo.class_str.split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:48:11:48:36::Use Foo.class_str.rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:56:12:56:40::Use test.get_string().split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:57:11:57:39::Use test.get_string().rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:66:10:66:22::Use s.split(' ', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:67:10:67:22::Use s.rsplit(' ', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:76:6:76:26::Use Bar.split.split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:77:6:77:26::Use Bar.split.rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:78:6:78:27::Use Bar.split.split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:79:6:79:27::Use Bar.split.rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:82:4:82:23::Use '1,2,3'.split('\n', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:83:4:83:26::Use '1,2,3'.rsplit('split', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:84:4:84:28::Use '1,2,3'.split('rsplit', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:96:11:96:30::Use SEQ.rsplit(None, maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:5:12:5:30::Use '1,2,3'.split(',', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:6:11:6:35::"Use '1,2,3'[::-1].split(',', maxsplit=1)[0] instead":HIGH
use-maxsplit-arg:9:12:9:26::Use SEQ.split(',', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:10:11:10:25::Use SEQ.rsplit(',', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:11:12:11:27::Use SEQ.split(',', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:12:11:12:26::Use SEQ.rsplit(',', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:47:12:47:36::Use Foo.class_str.split(',', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:48:11:48:35::Use Foo.class_str.rsplit(',', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:49:12:49:37::Use Foo.class_str.split(',', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:50:11:50:36::Use Foo.class_str.rsplit(',', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:58:12:58:40::Use test.get_string().split(',', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:59:11:59:39::Use test.get_string().rsplit(',', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:68:10:68:22::Use s.split(' ', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:69:10:69:22::Use s.rsplit(' ', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:78:6:78:26::Use Bar.split.split(',', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:79:6:79:26::Use Bar.split.rsplit(',', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:80:6:80:27::Use Bar.split.split(',', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:81:6:81:27::Use Bar.split.rsplit(',', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:84:4:84:23::Use '1,2,3'.split('\n', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:85:4:85:26::Use '1,2,3'.rsplit('split', maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:86:4:86:28::Use '1,2,3'.split('rsplit', maxsplit=1)[0] instead:HIGH
use-maxsplit-arg:99:11:99:30::Use SEQ.rsplit(None, maxsplit=1)[-1] instead:HIGH
use-maxsplit-arg:100:11:100:31::Use SEQ.rsplit(None, maxsplit=1)[-1] instead:INFERENCE

0 comments on commit 507dfc5

Please sign in to comment.