From f8fec600f06dffdc9e30d31c6ab6c0d04801c1c8 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Tue, 13 Jun 2023 17:05:32 -0400 Subject: [PATCH 1/3] 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 --- doc/whatsnew/fragments/7761.false_negative | 6 ++++++ .../refactoring/recommendation_checker.py | 16 +++++++++++++--- .../checkers/refactoring/refactoring_checker.py | 11 ++++++++++- pylint/checkers/stdlib.py | 2 +- tests/functional/m/method_cache_max_size_none.py | 5 +++++ .../unnecessary/unnecessary_list_index_lookup.py | 3 +++ tests/functional/u/use/use_maxsplit_arg.py | 4 ++++ 7 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 doc/whatsnew/fragments/7761.false_negative diff --git a/doc/whatsnew/fragments/7761.false_negative b/doc/whatsnew/fragments/7761.false_negative new file mode 100644 index 0000000000..4c1bea27b8 --- /dev/null +++ b/doc/whatsnew/fragments/7761.false_negative @@ -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 diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py index db2e2f6044..3829cdd933 100644 --- a/pylint/checkers/refactoring/recommendation_checker.py +++ b/pylint/checkers/refactoring/recommendation_checker.py @@ -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: @@ -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", diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index b036ad233e..d873e9c60d 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -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 @@ -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] diff --git a/pylint/checkers/stdlib.py b/pylint/checkers/stdlib.py index d824d614f9..b2f4a709b7 100644 --- a/pylint/checkers/stdlib.py +++ b/pylint/checkers/stdlib.py @@ -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 diff --git a/tests/functional/m/method_cache_max_size_none.py b/tests/functional/m/method_cache_max_size_none.py index 6604a73258..bb8ac75a29 100644 --- a/tests/functional/m/method_cache_max_size_none.py +++ b/tests/functional/m/method_cache_max_size_none.py @@ -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) diff --git a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py index 9e99388d84..a30416b444 100644 --- a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py +++ b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py @@ -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]) diff --git a/tests/functional/u/use/use_maxsplit_arg.py b/tests/functional/u/use/use_maxsplit_arg.py index 11e51c1167..4dd00ed23a 100644 --- a/tests/functional/u/use/use_maxsplit_arg.py +++ b/tests/functional/u/use/use_maxsplit_arg.py @@ -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] @@ -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: From 8b692e1a967125eb29ca478f35f6f2961e1d8da6 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Tue, 13 Jun 2023 17:11:55 -0400 Subject: [PATCH 2/3] Update functional output --- .../m/method_cache_max_size_none.txt | 3 +- .../unnecessary_list_index_lookup.txt | 1 + tests/functional/u/use/use_maxsplit_arg.txt | 45 ++++++++++--------- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/tests/functional/m/method_cache_max_size_none.txt b/tests/functional/m/method_cache_max_size_none.txt index 35512db855..bd916c6178 100644 --- a/tests/functional/m/method_cache_max_size_none.txt +++ b/tests/functional/m/method_cache_max_size_none.txt @@ -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 diff --git a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt index da658a20d0..dd128f9a55 100644 --- a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt +++ b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt @@ -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 diff --git a/tests/functional/u/use/use_maxsplit_arg.txt b/tests/functional/u/use/use_maxsplit_arg.txt index b8f254004b..2b08857341 100644 --- a/tests/functional/u/use/use_maxsplit_arg.txt +++ b/tests/functional/u/use/use_maxsplit_arg.txt @@ -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 From 840d1abc23f483bdf9023c27a6e2c0ba0d849012 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Tue, 13 Jun 2023 17:33:08 -0400 Subject: [PATCH 3/3] typo --- tests/functional/u/use/use_maxsplit_arg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/u/use/use_maxsplit_arg.py b/tests/functional/u/use/use_maxsplit_arg.py index 4dd00ed23a..969a32d3df 100644 --- a/tests/functional/u/use/use_maxsplit_arg.py +++ b/tests/functional/u/use/use_maxsplit_arg.py @@ -17,14 +17,14 @@ # Test varying maxsplit argument -- all these will be okay -maxSplit = {"maxSplit": 2} +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=',', **maxSplit)[1] +good_split = '1,2,3'.split(sep=',', **max_split)[1] # ## str.rsplit() tests good_split = '1,2,3'.rsplit(sep=',', maxsplit=1)[-1]