From 4c45ccae4b07a805abc3611679c92d133d07fd33 Mon Sep 17 00:00:00 2001
From: Pang Yu Shao
Date: Wed, 12 May 2021 22:44:26 +0800
Subject: [PATCH] Improved on help text logic and accompanying tests
---
.../refactoring/recommendation_checker.py | 26 ++++++++--
.../consider/consider_using_str_partition.py | 12 +++++
.../consider/consider_using_str_partition.txt | 52 +++++++++++--------
3 files changed, 62 insertions(+), 28 deletions(-)
diff --git a/pylint/checkers/refactoring/recommendation_checker.py b/pylint/checkers/refactoring/recommendation_checker.py
index af7993dc4ea..a012cb52160 100644
--- a/pylint/checkers/refactoring/recommendation_checker.py
+++ b/pylint/checkers/refactoring/recommendation_checker.py
@@ -259,16 +259,26 @@ def visit_subscript(self, node: astroid.Subscript) -> None:
if isinstance(value, (astroid.UnaryOp, astroid.Const)):
const = utils.safe_infer(value)
const = cast(astroid.Const, const)
- p = re.compile(r"r*split")
+ p = re.compile(
+ r"tilpsr*\."
+ ) # Match and replace in reverse to ensure last occurance replaced
if const.value == -1:
- help_text = p.sub("rpartition", subscripted_object.as_string())
+ help_text = p.sub(
+ ".rpartition"[::-1],
+ subscripted_object.as_string()[::-1],
+ count=1,
+ )[::-1]
self.add_message(
"consider-using-str-partition",
node=node,
args=(help_text, -1),
)
elif const.value == 0:
- help_text = p.sub("partition", subscripted_object.as_string())
+ help_text = p.sub(
+ ".partition"[::-1],
+ subscripted_object.as_string()[::-1],
+ count=1,
+ )[::-1]
self.add_message(
"consider-using-str-partition",
node=node,
@@ -294,8 +304,14 @@ def visit_subscript(self, node: astroid.Subscript) -> None:
# Further check for argument within len(), and compare that to object being split
arg_within_len = value.left.args[0].name
if arg_within_len == node.value.name:
- p = re.compile(r"r*split")
- help_text = p.sub("rpartition", subscripted_object.as_string())
+ p = re.compile(
+ r"tilpsr*\."
+ ) # Match and replace in reverse to ensure last occurance replaced
+ help_text = p.sub(
+ ".rpartition"[::-1],
+ subscripted_object.as_string()[::-1],
+ count=1,
+ )[::-1]
self.add_message(
"consider-using-str-partition",
node=node,
diff --git a/tests/functional/c/consider/consider_using_str_partition.py b/tests/functional/c/consider/consider_using_str_partition.py
index 226b5dd8ac6..541cff64434 100644
--- a/tests/functional/c/consider/consider_using_str_partition.py
+++ b/tests/functional/c/consider/consider_using_str_partition.py
@@ -3,6 +3,9 @@
from collections.abc import Iterable
from typing import Any
+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]
@@ -68,6 +71,15 @@ def get_string(self) -> str:
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)
+
# Test with user-defined len function
def len(x: Iterable[Any]) -> str:
return f"Hello, world! {x[2]}"
diff --git a/tests/functional/c/consider/consider_using_str_partition.txt b/tests/functional/c/consider/consider_using_str_partition.txt
index 1899d22294d..25e400b2940 100644
--- a/tests/functional/c/consider/consider_using_str_partition.txt
+++ b/tests/functional/c/consider/consider_using_str_partition.txt
@@ -1,23 +1,29 @@
-consider-using-str-partition:7:12::Consider using SEQ.partition(',')[0] instead
-consider-using-str-partition:8:11::Consider using SEQ.rpartition(',')[-1] 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:17:12::Consider using SEQ.partition(',')[0] instead
-consider-using-str-partition:18:11::Consider using SEQ.rpartition(',')[-1] instead
-consider-using-str-partition:19:11::Consider using SEQ.rpartition(',')[-1] instead
-consider-using-str-partition:22:12::Consider using SEQ.partition(',')[0] instead
-consider-using-str-partition:23:11::Consider using SEQ.rpartition(',')[-1] instead
-consider-using-str-partition:24:11::Consider using SEQ.rpartition(',')[-1] instead
-consider-using-str-partition:26:6::Consider using SEQ.partition(',')[0] instead
-consider-using-str-partition:26:17::Consider using SEQ.rpartition(',')[-1] instead
-consider-using-str-partition:43:12::Consider using Foo.class_str.partition(',')[0] instead
-consider-using-str-partition:44:11::Consider using Foo.class_str.rpartition(',')[-1] instead
-consider-using-str-partition:45:12::Consider using Foo.class_str.partition(',')[0] instead
-consider-using-str-partition:46:11::Consider using Foo.class_str.rpartition(',')[-1] instead
-consider-using-str-partition:52:12::Consider using Foo.class_str.partition(',')[0] instead
-consider-using-str-partition:53:11::Consider using Foo.class_str.rpartition(',')[-1] instead
-consider-using-str-partition:54:11::Consider using Foo.class_str.rpartition(',')[-1] instead
-consider-using-str-partition:58:12::Consider using bar.get_string().partition(',')[0] instead
-consider-using-str-partition:59:11::Consider using bar.get_string().rpartition(',')[-1] instead
-consider-using-str-partition:67:10::Consider using s.partition(' ')[0] instead
-consider-using-str-partition:68:10::Consider using s.rpartition(' ')[-1] instead
+consider-using-str-partition:6:12::Consider using '1,2,3'.partition(',')[0] instead
+consider-using-str-partition:7:11::"Consider using '1,2,3'[::-1].partition(',')[0] instead"
+consider-using-str-partition:10:12::Consider using SEQ.partition(',')[0] instead
+consider-using-str-partition:11:11::Consider using SEQ.rpartition(',')[-1] instead
+consider-using-str-partition:12:12::Consider using SEQ.partition(',')[0] instead
+consider-using-str-partition:13:11::Consider using SEQ.rpartition(',')[-1] instead
+consider-using-str-partition:20:12::Consider using SEQ.partition(',')[0] instead
+consider-using-str-partition:21:11::Consider using SEQ.rpartition(',')[-1] instead
+consider-using-str-partition:22:11::Consider using SEQ.rpartition(',')[-1] instead
+consider-using-str-partition:25:12::Consider using SEQ.partition(',')[0] instead
+consider-using-str-partition:26:11::Consider using SEQ.rpartition(',')[-1] instead
+consider-using-str-partition:27:11::Consider using SEQ.rpartition(',')[-1] instead
+consider-using-str-partition:29:6::Consider using SEQ.partition(',')[0] instead
+consider-using-str-partition:29:17::Consider using SEQ.rpartition(',')[-1] instead
+consider-using-str-partition:46:12::Consider using Foo.class_str.partition(',')[0] instead
+consider-using-str-partition:47:11::Consider using Foo.class_str.rpartition(',')[-1] 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:55:12::Consider using Foo.class_str.partition(',')[0] instead
+consider-using-str-partition:56:11::Consider using Foo.class_str.rpartition(',')[-1] instead
+consider-using-str-partition:57:11::Consider using Foo.class_str.rpartition(',')[-1] instead
+consider-using-str-partition:61:12::Consider using bar.get_string().partition(',')[0] instead
+consider-using-str-partition:62:11::Consider using bar.get_string().rpartition(',')[-1] instead
+consider-using-str-partition:70:10::Consider using s.partition(' ')[0] instead
+consider-using-str-partition:71: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