From ad1284d684277bed1defe0765d31fbfd182dc9ec Mon Sep 17 00:00:00 2001
From: Pang Yu Shao
Date: Thu, 20 May 2021 12:49:25 +0800
Subject: [PATCH] Reworked logic based on review
---
ChangeLog | 2 +-
doc/whatsnew/2.9.rst | 2 +-
.../refactoring/recommendation_checker.py | 40 +++++----
.../c/consider/consider_using_maxsplit_arg.py | 77 ++++++++++++++++++
.../consider/consider_using_maxsplit_arg.txt | 18 +++++
.../consider/consider_using_str_partition.py | 81 -------------------
.../consider/consider_using_str_partition.txt | 20 -----
7 files changed, 115 insertions(+), 125 deletions(-)
create mode 100644 tests/functional/c/consider/consider_using_maxsplit_arg.py
create mode 100644 tests/functional/c/consider/consider_using_maxsplit_arg.txt
delete mode 100644 tests/functional/c/consider/consider_using_str_partition.py
delete mode 100644 tests/functional/c/consider/consider_using_str_partition.txt
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