diff --git a/ChangeLog b/ChangeLog index 112568ea4c..6922a6137b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -51,7 +51,7 @@ modules are added. Closes #4412 -* New checker ``consider-using-str-partition``. Emitted either when accessing only the first or last +* New checker ``consider-using-maxsplit-arg``. Emitted either when accessing only the first or last element of ``str.split()``. Closes #4440 diff --git a/doc/whatsnew/2.9.rst b/doc/whatsnew/2.9.rst index d33756d056..70eda9c72f 100644 --- a/doc/whatsnew/2.9.rst +++ b/doc/whatsnew/2.9.rst @@ -16,7 +16,7 @@ New checkers indexing the same dictionary with the key within loop body. -* ``consider-using-str-partition``: Emitted either when accessing only the first or last +* ``consider-using-maxsplit-arg``: Emitted either when accessing only the first or last element of ``str.split()``. * An ``ignore_signatures`` option has been added to the similarity checker. It will permits to reduce false positives when multiple functions have the same parameters. diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py index bdf2346aaf..34af374e75 100644 --- a/pylint/checkers/refactoring/recommendation_checker.py +++ b/pylint/checkers/refactoring/recommendation_checker.py @@ -37,10 +37,11 @@ class RecommendationChecker(checkers.BaseChecker): ), "C0207": ( "Consider using %s instead", - "consider-using-str-partition", + "consider-using-maxsplit-arg", "Emitted when accessing only the first or last element of str.split(). " - "The first and last element can be accessed by using str.partition()[0] " - "or str.rpartition()[-1] instead.", + "The first and last element can be accessed by using " + "str.split(sep,maxsplit=1)[0] or str.rpartition(sep,maxsplit=1)[-1] " + "instead.", ), } @@ -52,11 +53,11 @@ def _is_builtin(node, function): return utils.is_builtin_object(inferred) and inferred.name == function @utils.check_messages( - "consider-iterating-dictionary", "consider-using-str-partition" + "consider-iterating-dictionary", "consider-using-maxsplit-arg" ) def visit_call(self, node: astroid.Call) -> None: self._check_consider_iterating_dictionary(node) - self._check_consider_using_str_partition(node) + self._check_consider_using_maxsplit_arg(node) def _check_consider_iterating_dictionary(self, node: astroid.Call) -> None: if not isinstance(node.func, astroid.Attribute): @@ -75,7 +76,7 @@ def _check_consider_iterating_dictionary(self, node: astroid.Call) -> None: if isinstance(node.parent, (astroid.For, astroid.Comprehension)): self.add_message("consider-iterating-dictionary", node=node) - def _check_consider_using_str_partition(self, node: astroid.Call) -> None: + def _check_consider_using_maxsplit_arg(self, node: astroid.Call) -> None: """Add message when accessing first or last elements of a str.split() or str.rsplit(), or when split/rsplit with max_split=1 is used""" @@ -86,14 +87,14 @@ def _check_consider_using_str_partition(self, node: astroid.Call) -> None: and isinstance(utils.safe_infer(node.func), astroid.BoundMethod) ): try: - seperator = utils.get_argument_from_call(node, 0, "sep").value + sep = utils.get_argument_from_call(node, 0, "sep").value except utils.NoSuchArgumentError: return # Check if maxsplit is set, and ignore checking if maxsplit is > 1 try: maxsplit = utils.get_argument_from_call(node, 1, "maxsplit").value - if maxsplit > 1: + if maxsplit > 0: return except utils.NoSuchArgumentError: maxsplit = 0 @@ -106,21 +107,16 @@ def _check_consider_using_str_partition(self, node: astroid.Call) -> None: except utils.InferredTypeError: return - if maxsplit == 1 and subscript_value == 1: - new_func = node.func.attrname.replace("split", "partition") - new_name = f"{node.as_string().rpartition('.')[0]}.{new_func}({seperator})[2]" - - elif maxsplit == 0 and subscript_value in (-1, 0): + if subscript_value in (-1, 0): fn_name = node.func.attrname - new_fn = "rpartition" if subscript_value == -1 else "partition" - new_name = f"{new_fn.join(node.as_string().rsplit(fn_name, maxsplit=1))}[{subscript_value}]" - - else: - return - - self.add_message( - "consider-using-str-partition", node=node, args=(new_name,) - ) + new_fn = "rsplit" if subscript_value == -1 else "split" + new_name = ( + f"{node.as_string().rsplit(fn_name, maxsplit=1)[0]}{new_fn}" + f"(sep='{sep}',maxsplit=1)[{subscript_value}]" + ) + self.add_message( + "consider-using-maxsplit-arg", node=node, args=(new_name,) + ) @utils.check_messages("consider-using-enumerate", "consider-using-dict-items") def visit_for(self, node: astroid.For) -> None: diff --git a/tests/functional/c/consider/consider_using_maxsplit_arg.py b/tests/functional/c/consider/consider_using_maxsplit_arg.py new file mode 100644 index 0000000000..d12d7d00e2 --- /dev/null +++ b/tests/functional/c/consider/consider_using_maxsplit_arg.py @@ -0,0 +1,77 @@ +"""Emit a message for accessing first/last element of string.split""" +# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods,invalid-name,redefined-builtin + +# Test subscripting .split() +get_first = '1,2,3'.split(',')[0] # [consider-using-maxsplit-arg] +get_last = '1,2,3'[::-1].split(',')[0] # [consider-using-maxsplit-arg] + +SEQ = '1,2,3' +get_first = SEQ.split(',')[0] # [consider-using-maxsplit-arg] +get_last = SEQ.split(',')[-1] # [consider-using-maxsplit-arg] +get_first = SEQ.rsplit(',')[0] # [consider-using-maxsplit-arg] +get_last = SEQ.rsplit(',')[-1] # [consider-using-maxsplit-arg] + +get_mid = SEQ.split(',')[1] # This is okay +get_mid = SEQ.split(',')[-2] # This is okay + + +# Test varying maxsplit argument -- all these will be okay +## str.split() tests +good_split = '1,2,3'.split(sep=',', maxsplit=1)[-1] # This is fine +good_split = '1,2,3'.split(sep=',', maxsplit=1)[0] # This is fine +good_split = '1,2,3'.split(sep=',', maxsplit=2)[-1] # This is fine +good_split = '1,2,3'.split(sep=',', maxsplit=2)[0] # This is fine +good_split = '1,2,3'.split(sep=',', maxsplit=2)[1] # This is fine + +## str.rsplit() tests +good_split = '1,2,3'.rsplit(sep=',', maxsplit=1)[-1] # This is fine +good_split = '1,2,3'.rsplit(sep=',', maxsplit=1)[0] # This is fine +good_split = '1,2,3'.rsplit(sep=',', maxsplit=2)[-1] # This is fine +good_split = '1,2,3'.rsplit(sep=',', maxsplit=2)[0] # This is fine +good_split = '1,2,3'.rsplit(sep=',', maxsplit=2)[1] # This is fine + + +# Tests on class attributes +class Foo(): + class_str = '1,2,3' + def __init__(self): + self.my_str = '1,2,3' + + def get_string(self) -> str: + return self.my_str + +# Class attributes +get_first = Foo.class_str.split(',')[0] # [consider-using-maxsplit-arg] +get_last = Foo.class_str.split(',')[-1] # [consider-using-maxsplit-arg] +get_first = Foo.class_str.rsplit(',')[0] # [consider-using-maxsplit-arg] +get_last = Foo.class_str.rsplit(',')[-1] # [consider-using-maxsplit-arg] + +get_mid = Foo.class_str.split(',')[1] +get_mid = Foo.class_str.split(',')[-2] + + +# Test with accessors +bar = Foo() +get_first = bar.get_string().split(',')[0] # [consider-using-maxsplit-arg] +get_last = bar.get_string().split(',')[-1] # [consider-using-maxsplit-arg] + +get_mid = bar.get_string().split(',')[1] +get_mid = bar.get_string().split(',')[-2] + + +# Test with iterating over strings +list_of_strs = ["a", "b", "c", "d", "e", "f"] +for s in list_of_strs: + print(s.split(" ")[0]) # [consider-using-maxsplit-arg] + print(s.split(" ")[-1]) # [consider-using-maxsplit-arg] + print(s.split(" ")[-2]) + + +# Test warning messages (matching and replacing .split / .rsplit) +class Bar(): + split = '1,2,3' + +print(Bar.split.split(",")[0]) # [consider-using-maxsplit-arg] (Error message should show Bar.split.partition) +print(Bar.split.split(",")[-1]) # [consider-using-maxsplit-arg] (Error message should show Bar.split.rpartition) +print(Bar.split.rsplit(",")[0]) # [consider-using-maxsplit-arg] (Error message should show Bar.split.partition) +print(Bar.split.rsplit(",")[-1]) # [consider-using-maxsplit-arg] (Error message should show Bar.split.rpartition) diff --git a/tests/functional/c/consider/consider_using_maxsplit_arg.txt b/tests/functional/c/consider/consider_using_maxsplit_arg.txt new file mode 100644 index 0000000000..623c5bc3e7 --- /dev/null +++ b/tests/functional/c/consider/consider_using_maxsplit_arg.txt @@ -0,0 +1,18 @@ +consider-using-maxsplit-arg:5:12::Consider using '1,2,3'.split(sep=',',maxsplit=1)[0] instead +consider-using-maxsplit-arg:6:11::"Consider using '1,2,3'[::-1].split(sep=',',maxsplit=1)[0] instead" +consider-using-maxsplit-arg:9:12::Consider using SEQ.split(sep=',',maxsplit=1)[0] instead +consider-using-maxsplit-arg:10:11::Consider using SEQ.rsplit(sep=',',maxsplit=1)[-1] instead +consider-using-maxsplit-arg:11:12::Consider using SEQ.split(sep=',',maxsplit=1)[0] instead +consider-using-maxsplit-arg:12:11::Consider using SEQ.rsplit(sep=',',maxsplit=1)[-1] instead +consider-using-maxsplit-arg:44:12::Consider using Foo.class_str.split(sep=',',maxsplit=1)[0] instead +consider-using-maxsplit-arg:45:11::Consider using Foo.class_str.rsplit(sep=',',maxsplit=1)[-1] instead +consider-using-maxsplit-arg:46:12::Consider using Foo.class_str.split(sep=',',maxsplit=1)[0] instead +consider-using-maxsplit-arg:47:11::Consider using Foo.class_str.rsplit(sep=',',maxsplit=1)[-1] instead +consider-using-maxsplit-arg:55:12::Consider using bar.get_string().split(sep=',',maxsplit=1)[0] instead +consider-using-maxsplit-arg:56:11::Consider using bar.get_string().rsplit(sep=',',maxsplit=1)[-1] instead +consider-using-maxsplit-arg:65:10::Consider using s.split(sep=' ',maxsplit=1)[0] instead +consider-using-maxsplit-arg:66:10::Consider using s.rsplit(sep=' ',maxsplit=1)[-1] instead +consider-using-maxsplit-arg:74:6::Consider using Bar.split.split(sep=',',maxsplit=1)[0] instead +consider-using-maxsplit-arg:75:6::Consider using Bar.split.rsplit(sep=',',maxsplit=1)[-1] instead +consider-using-maxsplit-arg:76:6::Consider using Bar.split.split(sep=',',maxsplit=1)[0] instead +consider-using-maxsplit-arg:77:6::Consider using Bar.split.rsplit(sep=',',maxsplit=1)[-1] instead diff --git a/tests/functional/c/consider/consider_using_str_partition.py b/tests/functional/c/consider/consider_using_str_partition.py deleted file mode 100644 index f241f64412..0000000000 --- a/tests/functional/c/consider/consider_using_str_partition.py +++ /dev/null @@ -1,81 +0,0 @@ -"""Emit a message for iteration through dict keys and subscripting dict with key.""" -# pylint: disable=line-too-long,missing-docstring,unsubscriptable-object,too-few-public-methods,invalid-name,redefined-builtin - -# Test subscripting .split() -get_first = '1,2,3'.split(',')[0] # [consider-using-str-partition] -get_last = '1,2,3'[::-1].split(',')[0] # [consider-using-str-partition] - -SEQ = '1,2,3' -get_first = SEQ.split(',')[0] # [consider-using-str-partition] -get_last = SEQ.split(',')[-1] # [consider-using-str-partition] -get_first = SEQ.rsplit(',')[0] # [consider-using-str-partition] -get_last = SEQ.rsplit(',')[-1] # [consider-using-str-partition] - -get_mid = SEQ.split(',')[1] # This is okay -get_mid = SEQ.split(',')[-2] # This is okay - - -# Test varying maxsplit argument -## str.split() tests -bad_split = '1,2,3'.split(sep=',', maxsplit=1)[1] # [consider-using-str-partition] - -good_split = '1,2,3'.split(sep=',', maxsplit=1)[-1] # This is fine -good_split = '1,2,3'.split(sep=',', maxsplit=1)[0] # This is fine -good_split = '1,2,3'.split(sep=',', maxsplit=2)[-1] # This is fine, we ignore cases where maxsplit > 1 -good_split = '1,2,3'.split(sep=',', maxsplit=2)[0] # This is fine, we ignore cases where maxsplit > 1 -good_split = '1,2,3'.split(sep=',', maxsplit=2)[1] # This is fine, we ignore cases where maxsplit > 1 - -## str.rsplit() tests -bad_split = '1,2,3'.rsplit(sep=',', maxsplit=1)[1] # [consider-using-str-partition] - -good_split = '1,2,3'.rsplit(sep=',', maxsplit=1)[-1] # This is fine -good_split = '1,2,3'.rsplit(sep=',', maxsplit=1)[0] # This is fine -good_split = '1,2,3'.rsplit(sep=',', maxsplit=2)[-1] # This is fine, we ignore cases where maxsplit > 1 -good_split = '1,2,3'.rsplit(sep=',', maxsplit=2)[0] # This is fine, we ignore cases where maxsplit > 1 -good_split = '1,2,3'.rsplit(sep=',', maxsplit=2)[1] # This is fine, we ignore cases where maxsplit > 1 - - -# Tests on class attributes -class Foo(): - class_str = '1,2,3' - def __init__(self): - self.my_str = '1,2,3' - - def get_string(self) -> str: - return self.my_str - -# Class attributes -get_first = Foo.class_str.split(',')[0] # [consider-using-str-partition] -get_last = Foo.class_str.split(',')[-1] # [consider-using-str-partition] -get_first = Foo.class_str.rsplit(',')[0] # [consider-using-str-partition] -get_last = Foo.class_str.rsplit(',')[-1] # [consider-using-str-partition] - -get_mid = Foo.class_str.split(',')[1] -get_mid = Foo.class_str.split(',')[-2] - - -# Test with accessors -bar = Foo() -get_first = bar.get_string().split(',')[0] # [consider-using-str-partition] -get_last = bar.get_string().split(',')[-1] # [consider-using-str-partition] - -get_mid = bar.get_string().split(',')[1] -get_mid = bar.get_string().split(',')[-2] - - -# Test with iterating over strings -list_of_strs = ["a", "b", "c", "d", "e", "f"] -for s in list_of_strs: - print(s.split(" ")[0]) # [consider-using-str-partition] - print(s.split(" ")[-1]) # [consider-using-str-partition] - print(s.split(" ")[-2]) - - -# Test warning messages (matching and replacing .split / .rsplit) -class Bar(): - split = '1,2,3' - -print(Bar.split.split(",")[0]) # [consider-using-str-partition] (Error message should show Bar.split.partition) -print(Bar.split.split(",")[-1]) # [consider-using-str-partition] (Error message should show Bar.split.rpartition) -print(Bar.split.rsplit(",")[0]) # [consider-using-str-partition] (Error message should show Bar.split.partition) -print(Bar.split.rsplit(",")[-1]) # [consider-using-str-partition] (Error message should show Bar.split.rpartition) diff --git a/tests/functional/c/consider/consider_using_str_partition.txt b/tests/functional/c/consider/consider_using_str_partition.txt deleted file mode 100644 index d8d36e67c7..0000000000 --- a/tests/functional/c/consider/consider_using_str_partition.txt +++ /dev/null @@ -1,20 +0,0 @@ -consider-using-str-partition:5:12::Consider using '1,2,3'.partition(',')[0] instead -consider-using-str-partition:6:11::"Consider using '1,2,3'[::-1].partition(',')[0] instead" -consider-using-str-partition:9:12::Consider using SEQ.partition(',')[0] instead -consider-using-str-partition:10:11::Consider using SEQ.rpartition(',')[-1] instead -consider-using-str-partition:11:12::Consider using SEQ.partition(',')[0] instead -consider-using-str-partition:12:11::Consider using SEQ.rpartition(',')[-1] instead -consider-using-str-partition:20:12::Consider using '1,2,3'.partition(,)[2] instead -consider-using-str-partition:29:12::Consider using '1,2,3'.rpartition(,)[2] instead -consider-using-str-partition:48:12::Consider using Foo.class_str.partition(',')[0] instead -consider-using-str-partition:49:11::Consider using Foo.class_str.rpartition(',')[-1] instead -consider-using-str-partition:50:12::Consider using Foo.class_str.partition(',')[0] instead -consider-using-str-partition:51:11::Consider using Foo.class_str.rpartition(',')[-1] instead -consider-using-str-partition:59:12::Consider using bar.get_string().partition(',')[0] instead -consider-using-str-partition:60:11::Consider using bar.get_string().rpartition(',')[-1] instead -consider-using-str-partition:69:10::Consider using s.partition(' ')[0] instead -consider-using-str-partition:70:10::Consider using s.rpartition(' ')[-1] instead -consider-using-str-partition:78:6::Consider using Bar.split.partition(',')[0] instead -consider-using-str-partition:79:6::Consider using Bar.split.rpartition(',')[-1] instead -consider-using-str-partition:80:6::Consider using Bar.split.partition(',')[0] instead -consider-using-str-partition:81:6::Consider using Bar.split.rpartition(',')[-1] instead