From dbad1eafdfac00902f67bf5c454b29706375a1cb Mon Sep 17 00:00:00 2001 From: dosisod <39638017+dosisod@users.noreply.github.com> Date: Tue, 9 Jan 2024 23:14:53 -0800 Subject: [PATCH] Improve error message output: * Stringify args for FURB123 * Use double quotes for byte exprs --- refurb/checks/common.py | 38 ++++++++++++++++++- .../checks/readability/no_unnecessary_cast.py | 35 ++++++++--------- test/data/bug_cast.txt | 2 +- test/data/bug_type_reassignment.txt | 2 +- test/data/err_123.txt | 34 ++++++++--------- test/data/err_181.txt | 2 +- test/data/err_182.txt | 32 ++++++++-------- test/data/inline_comments.txt | 14 +++---- test/data/stringify.py | 2 + test/data/stringify.txt | 1 + 10 files changed, 100 insertions(+), 62 deletions(-) create mode 100644 test/data/stringify.py create mode 100644 test/data/stringify.txt diff --git a/refurb/checks/common.py b/refurb/checks/common.py index 1105326..2d83e51 100644 --- a/refurb/checks/common.py +++ b/refurb/checks/common.py @@ -7,9 +7,11 @@ BytesExpr, CallExpr, ComparisonExpr, + ComplexExpr, DictExpr, DictionaryComprehension, Expression, + FloatExpr, ForStmt, GeneratorExpr, IndexExpr, @@ -26,6 +28,7 @@ SliceExpr, StarExpr, Statement, + StrExpr, TupleExpr, UnaryExpr, ) @@ -291,12 +294,43 @@ def _stringify(node: Node) -> str: case BytesExpr(value=value): # TODO: use same formatting as source line - return repr(value.encode()) + value = value.replace('"', r"\"") + + return f'b"{value}"' case IntExpr(value=value): # TODO: use same formatting as source line return str(value) + case ComplexExpr(value=value): + # TODO: use same formatting as source line + return str(value) + + case FloatExpr(value=value): + return str(value) + + case StrExpr(value=value): + value = value.replace('"', r"\"") + + return f'"{value}"' + + case DictExpr(items=items): + parts: list[str] = [] + + for k, v in items: + if k: + parts.append(f"{stringify(k)}: {stringify(v)}") + + else: + parts.append(f"**{stringify(v)}") + + return f"{{{', '.join(parts)}}}" + + case TupleExpr(items=items): + inner = ", ".join(stringify(x) for x in items) + + return f"({inner})" + case CallExpr(): name = _stringify(node.callee) @@ -308,7 +342,7 @@ def _stringify(node: Node) -> str: return f"{_stringify(left)} {op} {_stringify(right)}" case ComparisonExpr(): - parts: list[str] = [] + parts = [] for op, operand in zip(node.operators, node.operands): parts.extend((_stringify(operand), op)) diff --git a/refurb/checks/readability/no_unnecessary_cast.py b/refurb/checks/readability/no_unnecessary_cast.py index f8dd34f..b504d60 100644 --- a/refurb/checks/readability/no_unnecessary_cast.py +++ b/refurb/checks/readability/no_unnecessary_cast.py @@ -16,6 +16,7 @@ Var, ) +from refurb.checks.common import stringify from refurb.error import Error @@ -59,16 +60,16 @@ class ErrorInfo(Error): FUNC_NAMES = { - "builtins.bool": (None, "x"), - "builtins.bytes": (BytesExpr, "x"), - "builtins.complex": (ComplexExpr, "x"), - "builtins.dict": (DictExpr, "x.copy()"), - "builtins.float": (FloatExpr, "x"), - "builtins.int": (IntExpr, "x"), - "builtins.list": (ListExpr, "x.copy()"), - "builtins.str": (StrExpr, "x"), - "builtins.tuple": (TupleExpr, "x"), - "tuple[]": (TupleExpr, "x"), + "builtins.bool": (None, ""), + "builtins.bytes": (BytesExpr, ""), + "builtins.complex": (ComplexExpr, ""), + "builtins.dict": (DictExpr, ".copy()"), + "builtins.float": (FloatExpr, ""), + "builtins.int": (IntExpr, ""), + "builtins.list": (ListExpr, ".copy()"), + "builtins.str": (StrExpr, ""), + "builtins.tuple": (TupleExpr, ""), + "tuple[]": (TupleExpr, ""), } @@ -86,13 +87,9 @@ def check(node: CallExpr, errors: list[Error]) -> None: args=[arg], arg_kinds=[arg_kind], ) if arg_kind != ArgKind.ARG_STAR2 and fullname in FUNC_NAMES: - node_type, msg = FUNC_NAMES[fullname] + node_type, suffix = FUNC_NAMES[fullname] - if type(arg) == node_type: - if isinstance(arg, DictExpr | ListExpr): - msg = "x" - - elif is_boolean_literal(arg) and name == "bool": + if (type(arg) == node_type) or (is_boolean_literal(arg) and name == "bool"): pass else: @@ -106,4 +103,8 @@ def check(node: CallExpr, errors: list[Error]) -> None: case _: return - errors.append(ErrorInfo.from_node(node, f"Replace `{name}(x)` with `{msg}`")) + expr = stringify(arg) + + msg = f"Replace `{name}({stringify(arg)})` with `{expr}{suffix}`" + + errors.append(ErrorInfo.from_node(node, msg)) diff --git a/test/data/bug_cast.txt b/test/data/bug_cast.txt index a0d2f86..f3b7218 100644 --- a/test/data/bug_cast.txt +++ b/test/data/bug_cast.txt @@ -1 +1 @@ -test/data/bug_cast.py:7:11 [FURB123]: Replace `int(x)` with `x` +test/data/bug_cast.py:7:11 [FURB123]: Replace `int(0)` with `0` diff --git a/test/data/bug_type_reassignment.txt b/test/data/bug_type_reassignment.txt index cdb9515..c865471 100644 --- a/test/data/bug_type_reassignment.txt +++ b/test/data/bug_type_reassignment.txt @@ -1 +1 @@ -test/data/bug_type_reassignment.py:18:6 [FURB123]: Replace `str(x)` with `x` +test/data/bug_type_reassignment.py:18:6 [FURB123]: Replace `str(x2)` with `x2` diff --git a/test/data/err_123.txt b/test/data/err_123.txt index a03b8ca..a662c5c 100644 --- a/test/data/err_123.txt +++ b/test/data/err_123.txt @@ -1,17 +1,17 @@ -test/data/err_123.py:3:5 [FURB123]: Replace `bool(x)` with `x` -test/data/err_123.py:4:5 [FURB123]: Replace `bytes(x)` with `x` -test/data/err_123.py:5:5 [FURB123]: Replace `complex(x)` with `x` -test/data/err_123.py:6:5 [FURB123]: Replace `dict(x)` with `x` -test/data/err_123.py:7:5 [FURB123]: Replace `float(x)` with `x` -test/data/err_123.py:8:5 [FURB123]: Replace `list(x)` with `x` -test/data/err_123.py:9:5 [FURB123]: Replace `str(x)` with `x` -test/data/err_123.py:10:5 [FURB123]: Replace `tuple(x)` with `x` -test/data/err_123.py:11:5 [FURB123]: Replace `int(x)` with `x` -test/data/err_123.py:14:5 [FURB123]: Replace `bool(x)` with `x` -test/data/err_123.py:17:5 [FURB123]: Replace `bytes(x)` with `x` -test/data/err_123.py:20:5 [FURB123]: Replace `complex(x)` with `x` -test/data/err_123.py:23:5 [FURB123]: Replace `dict(x)` with `x.copy()` -test/data/err_123.py:26:5 [FURB123]: Replace `float(x)` with `x` -test/data/err_123.py:29:5 [FURB123]: Replace `list(x)` with `x.copy()` -test/data/err_123.py:32:5 [FURB123]: Replace `str(x)` with `x` -test/data/err_123.py:35:5 [FURB123]: Replace `tuple(x)` with `x` +test/data/err_123.py:3:5 [FURB123]: Replace `bool(True)` with `True` +test/data/err_123.py:4:5 [FURB123]: Replace `bytes(b"hello world")` with `b"hello world"` +test/data/err_123.py:5:5 [FURB123]: Replace `complex(1j)` with `1j` +test/data/err_123.py:6:5 [FURB123]: Replace `dict({"a": 1})` with `{"a": 1}.copy()` +test/data/err_123.py:7:5 [FURB123]: Replace `float(123.456)` with `123.456` +test/data/err_123.py:8:5 [FURB123]: Replace `list([1, 2, 3])` with `[1, 2, 3].copy()` +test/data/err_123.py:9:5 [FURB123]: Replace `str("hello world")` with `"hello world"` +test/data/err_123.py:10:5 [FURB123]: Replace `tuple((1, 2, 3))` with `(1, 2, 3)` +test/data/err_123.py:11:5 [FURB123]: Replace `int(123)` with `123` +test/data/err_123.py:14:5 [FURB123]: Replace `bool(a)` with `a` +test/data/err_123.py:17:5 [FURB123]: Replace `bytes(b)` with `b` +test/data/err_123.py:20:5 [FURB123]: Replace `complex(c)` with `c` +test/data/err_123.py:23:5 [FURB123]: Replace `dict(d)` with `d.copy()` +test/data/err_123.py:26:5 [FURB123]: Replace `float(e)` with `e` +test/data/err_123.py:29:5 [FURB123]: Replace `list(f)` with `f.copy()` +test/data/err_123.py:32:5 [FURB123]: Replace `str(g)` with `g` +test/data/err_123.py:35:5 [FURB123]: Replace `tuple(t)` with `t` diff --git a/test/data/err_181.txt b/test/data/err_181.txt index 5453515..9936911 100644 --- a/test/data/err_181.txt +++ b/test/data/err_181.txt @@ -13,6 +13,6 @@ test/data/err_181.py:30:1 [FURB181]: Replace `sha512().digest().hex()` with `sha test/data/err_181.py:31:1 [FURB181]: Replace `shake_128().digest(10).hex()` with `shake_128().hexdigest(10)` test/data/err_181.py:32:1 [FURB181]: Replace `shake_256().digest(10).hex()` with `shake_256().hexdigest(10)` test/data/err_181.py:34:1 [FURB181]: Replace `hashlib.sha256().digest().hex()` with `hashlib.sha256().hexdigest()` -test/data/err_181.py:36:1 [FURB181]: Replace `sha256(b'text').digest().hex()` with `sha256(b'text').hexdigest()` +test/data/err_181.py:36:1 [FURB181]: Replace `sha256(b"text").digest().hex()` with `sha256(b"text").hexdigest()` test/data/err_181.py:38:1 [FURB181]: Replace `hash_algo().digest().hex()` with `hash_algo().hexdigest()` test/data/err_181.py:41:1 [FURB181]: Replace `h.digest().hex()` with `h.hexdigest()` diff --git a/test/data/err_182.txt b/test/data/err_182.txt index 6ccec2d..d7feab0 100644 --- a/test/data/err_182.txt +++ b/test/data/err_182.txt @@ -1,16 +1,16 @@ -test/data/err_182.py:19:1 [FURB182]: Replace `h1 = blake2b(); h1.update(b'data')` with `h1 = blake2b(b'data')` -test/data/err_182.py:22:1 [FURB182]: Replace `h2 = blake2s(); h2.update(b'data')` with `h2 = blake2s(b'data')` -test/data/err_182.py:25:1 [FURB182]: Replace `h3 = md5(); h3.update(b'data')` with `h3 = md5(b'data')` -test/data/err_182.py:28:1 [FURB182]: Replace `h4 = sha1(); h4.update(b'data')` with `h4 = sha1(b'data')` -test/data/err_182.py:31:1 [FURB182]: Replace `h5 = sha224(); h5.update(b'data')` with `h5 = sha224(b'data')` -test/data/err_182.py:34:1 [FURB182]: Replace `h6 = sha256(); h6.update(b'data')` with `h6 = sha256(b'data')` -test/data/err_182.py:37:1 [FURB182]: Replace `h7 = sha384(); h7.update(b'data')` with `h7 = sha384(b'data')` -test/data/err_182.py:40:1 [FURB182]: Replace `h8 = sha3_224(); h8.update(b'data')` with `h8 = sha3_224(b'data')` -test/data/err_182.py:43:1 [FURB182]: Replace `h9 = sha3_256(); h9.update(b'data')` with `h9 = sha3_256(b'data')` -test/data/err_182.py:46:1 [FURB182]: Replace `h10 = sha3_384(); h10.update(b'data')` with `h10 = sha3_384(b'data')` -test/data/err_182.py:49:1 [FURB182]: Replace `h11 = sha3_512(); h11.update(b'data')` with `h11 = sha3_512(b'data')` -test/data/err_182.py:52:1 [FURB182]: Replace `h12 = sha512(); h12.update(b'data')` with `h12 = sha512(b'data')` -test/data/err_182.py:55:1 [FURB182]: Replace `h13 = shake_128(); h13.update(b'data')` with `h13 = shake_128(b'data')` -test/data/err_182.py:58:1 [FURB182]: Replace `h14 = shake_256(); h14.update(b'data')` with `h14 = shake_256(b'data')` -test/data/err_182.py:61:1 [FURB182]: Replace `h15 = hashlib.sha256(); h15.update(b'data')` with `h15 = hashlib.sha256(b'data')` -test/data/err_182.py:64:1 [FURB182]: Replace `h16 = hash_algo(); h16.update(b'data')` with `h16 = hash_algo(b'data')` +test/data/err_182.py:19:1 [FURB182]: Replace `h1 = blake2b(); h1.update(b"data")` with `h1 = blake2b(b"data")` +test/data/err_182.py:22:1 [FURB182]: Replace `h2 = blake2s(); h2.update(b"data")` with `h2 = blake2s(b"data")` +test/data/err_182.py:25:1 [FURB182]: Replace `h3 = md5(); h3.update(b"data")` with `h3 = md5(b"data")` +test/data/err_182.py:28:1 [FURB182]: Replace `h4 = sha1(); h4.update(b"data")` with `h4 = sha1(b"data")` +test/data/err_182.py:31:1 [FURB182]: Replace `h5 = sha224(); h5.update(b"data")` with `h5 = sha224(b"data")` +test/data/err_182.py:34:1 [FURB182]: Replace `h6 = sha256(); h6.update(b"data")` with `h6 = sha256(b"data")` +test/data/err_182.py:37:1 [FURB182]: Replace `h7 = sha384(); h7.update(b"data")` with `h7 = sha384(b"data")` +test/data/err_182.py:40:1 [FURB182]: Replace `h8 = sha3_224(); h8.update(b"data")` with `h8 = sha3_224(b"data")` +test/data/err_182.py:43:1 [FURB182]: Replace `h9 = sha3_256(); h9.update(b"data")` with `h9 = sha3_256(b"data")` +test/data/err_182.py:46:1 [FURB182]: Replace `h10 = sha3_384(); h10.update(b"data")` with `h10 = sha3_384(b"data")` +test/data/err_182.py:49:1 [FURB182]: Replace `h11 = sha3_512(); h11.update(b"data")` with `h11 = sha3_512(b"data")` +test/data/err_182.py:52:1 [FURB182]: Replace `h12 = sha512(); h12.update(b"data")` with `h12 = sha512(b"data")` +test/data/err_182.py:55:1 [FURB182]: Replace `h13 = shake_128(); h13.update(b"data")` with `h13 = shake_128(b"data")` +test/data/err_182.py:58:1 [FURB182]: Replace `h14 = shake_256(); h14.update(b"data")` with `h14 = shake_256(b"data")` +test/data/err_182.py:61:1 [FURB182]: Replace `h15 = hashlib.sha256(); h15.update(b"data")` with `h15 = hashlib.sha256(b"data")` +test/data/err_182.py:64:1 [FURB182]: Replace `h16 = hash_algo(); h16.update(b"data")` with `h16 = hash_algo(b"data")` diff --git a/test/data/inline_comments.txt b/test/data/inline_comments.txt index 12fcded..4f9c726 100644 --- a/test/data/inline_comments.txt +++ b/test/data/inline_comments.txt @@ -1,7 +1,7 @@ -test/data/inline_comments.py:18:5 [FURB123]: Replace `int(x)` with `x` -test/data/inline_comments.py:19:5 [FURB123]: Replace `int(x)` with `x` -test/data/inline_comments.py:20:5 [FURB123]: Replace `int(x)` with `x` -test/data/inline_comments.py:21:5 [FURB123]: Replace `int(x)` with `x` -test/data/inline_comments.py:22:5 [FURB123]: Replace `int(x)` with `x` -test/data/inline_comments.py:23:5 [FURB123]: Replace `str(x)` with `x` -test/data/inline_comments.py:24:5 [FURB123]: Replace `str(x)` with `x` +test/data/inline_comments.py:18:5 [FURB123]: Replace `int(0)` with `0` +test/data/inline_comments.py:19:5 [FURB123]: Replace `int(0)` with `0` +test/data/inline_comments.py:20:5 [FURB123]: Replace `int(0)` with `0` +test/data/inline_comments.py:21:5 [FURB123]: Replace `int(0)` with `0` +test/data/inline_comments.py:22:5 [FURB123]: Replace `int(0)` with `0` +test/data/inline_comments.py:23:5 [FURB123]: Replace `str("# noqa: FURB123 ")` with `"# noqa: FURB123 "` +test/data/inline_comments.py:24:5 [FURB123]: Replace `str("# noqa: FURB123 ")` with `"# noqa: FURB123 "` diff --git a/test/data/stringify.py b/test/data/stringify.py new file mode 100644 index 0000000..f83df77 --- /dev/null +++ b/test/data/stringify.py @@ -0,0 +1,2 @@ +# test double star in dict expr +_ = dict({**{}}) diff --git a/test/data/stringify.txt b/test/data/stringify.txt new file mode 100644 index 0000000..b6eafc0 --- /dev/null +++ b/test/data/stringify.txt @@ -0,0 +1 @@ +test/data/stringify.py:2:5 [FURB123]: Replace `dict({**{}})` with `{**{}}.copy()`