Skip to content

Commit

Permalink
Improve error message output
Browse files Browse the repository at this point in the history
  • Loading branch information
dosisod committed Jan 12, 2024
1 parent 585c8a2 commit 389e0ff
Show file tree
Hide file tree
Showing 13 changed files with 86 additions and 73 deletions.
8 changes: 6 additions & 2 deletions refurb/checks/builtin/list_extend.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ class ErrorInfo(Error):

name = "use-list-extend"
code = 113
msg: str = "Use `x.extend(...)` instead of repeatedly calling `x.append()`"
categories = ("list",)


Expand Down Expand Up @@ -73,7 +72,12 @@ def check_stmts(stmts: list[Statement], errors: list[Error]) -> None:
)
) if str(ty).startswith("builtins.list["):
if not last.did_error and name == last.name:
errors.append(ErrorInfo(last.line, last.column))
old = f"{name}.append(...); {name}.append(...)"
new = f"{name}.extend((..., ...))"
msg = f"Replace `{old}` with `{new}`"

errors.append(ErrorInfo(last.line, last.column, msg))

last.did_error = True

last.name = name
Expand Down
8 changes: 5 additions & 3 deletions refurb/checks/builtin/no_is_type_none.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from mypy.nodes import CallExpr, ComparisonExpr, NameExpr

from refurb.checks.common import is_type_none_call
from refurb.checks.common import is_type_none_call, stringify
from refurb.error import Error


Expand Down Expand Up @@ -41,12 +41,14 @@ def check(node: ComparisonExpr, errors: list[Error]) -> None:
case ComparisonExpr(
operators=["is" | "is not" | "==" | "!=" as oper],
operands=[
CallExpr(callee=NameExpr(fullname="builtins.type")),
CallExpr(callee=NameExpr(fullname="builtins.type"), args=[arg]),
rhs,
],
) if is_type_none_call(rhs):
new = "is" if oper in {"is", "=="} else "is not"

msg = f"Replace `type(x) {oper} type(None)` with `x {new} None`"
expr = stringify(arg)

msg = f"Replace `type({expr}) {oper} type(None)` with `{expr} {new} None`"

errors.append(ErrorInfo.from_node(node, msg))
15 changes: 7 additions & 8 deletions refurb/checks/builtin/no_slice_copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from mypy.nodes import AssignmentStmt, DelStmt, IndexExpr, MypyFile, RefExpr, SliceExpr, Var

from refurb.checks.common import stringify
from refurb.error import Error
from refurb.visitor import TraverserVisitor

Expand Down Expand Up @@ -29,7 +30,6 @@ class ErrorInfo(Error):

name = "no-slice-copy"
code = 145
msg: str = "Replace `x[:]` with `x.copy()`"
categories = ("readability",)


Expand Down Expand Up @@ -57,8 +57,6 @@ def visit_del_stmt(self, node: DelStmt) -> None:
self.accept(node.expr)

def visit_index_expr(self, node: IndexExpr) -> None:
index = node.index

match node.base:
case RefExpr(node=Var(type=ty)):
if not str(ty).startswith(SEQUENCE_BUILTINS):
Expand All @@ -67,11 +65,12 @@ def visit_index_expr(self, node: IndexExpr) -> None:
case _:
return

if (
isinstance(index, SliceExpr)
and index.begin_index is index.end_index is index.stride is None
):
self.errors.append(ErrorInfo.from_node(node))
match node.index:
case SliceExpr(begin_index=None, end_index=None, stride=None):
base = stringify(node.base)
msg = f"Replace `{base}[:]` with `{base}.copy()`"

self.errors.append(ErrorInfo.from_node(node, msg))


def check(node: MypyFile, errors: list[Error]) -> None:
Expand Down
15 changes: 9 additions & 6 deletions refurb/checks/builtin/use_int_base_zero.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from mypy.nodes import ArgKind, CallExpr, IndexExpr, IntExpr, RefExpr, SliceExpr

from refurb.checks.common import stringify
from refurb.error import Error


Expand Down Expand Up @@ -56,6 +57,7 @@ def check(node: CallExpr, errors: list[Error]) -> None:
callee=RefExpr(fullname="builtins.int"),
args=[
IndexExpr(
base=index_base,
index=SliceExpr(
begin_index=IntExpr(value=2),
end_index=None,
Expand All @@ -69,9 +71,10 @@ def check(node: CallExpr, errors: list[Error]) -> None:
):
kw = "base=" if arg_kinds[1] == ArgKind.ARG_NAMED else ""

errors.append(
ErrorInfo.from_node(
node,
f"Replace `int(x[2:], {kw}{base})` with `int(x, {kw}0)`",
)
)
index_base_expr = stringify(index_base)

old = f"int({index_base_expr}[2:], {kw}{base})"
new = f"int({index_base_expr}, {kw}0)"
msg = f"Replace `{old}` with `{new}`"

errors.append(ErrorInfo.from_node(node, msg))
12 changes: 9 additions & 3 deletions refurb/checks/pattern_matching/simplify_as_builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ def check(node: AsPattern, errors: list[Error]) -> None:
positionals=[],
keyword_keys=[],
keyword_values=[],
)
) if fullname in BUILTIN_PATTERN_CLASSES:
errors.append(ErrorInfo.from_node(node, f"Replace `{name}() as x` with `{name}(x)`"))
),
name=as_name,
) if as_name and fullname in BUILTIN_PATTERN_CLASSES:
old = f"{name}() as {as_name.name}"
new = f"{name}({as_name.name})"

msg = f"Replace `{old}` with `{new}`"

errors.append(ErrorInfo.from_node(node, msg))
22 changes: 10 additions & 12 deletions refurb/checks/readability/no_or_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
Var,
)

from refurb.checks.common import extract_binary_oper
from refurb.checks.common import extract_binary_oper, stringify
from refurb.error import Error


Expand Down Expand Up @@ -49,38 +49,31 @@ def is_markdown_header(line: str) -> bool:

def check(node: OpExpr, errors: list[Error]) -> None:
match extract_binary_oper("or", node):
case (NameExpr(node=Var(type=ty)), arg):
case (NameExpr(node=Var(type=ty)) as lhs, arg):
match arg:
case CallExpr(callee=NameExpr(name=name, fullname=fullname), args=[]):
expr = f"{name}()"
case CallExpr(callee=NameExpr(fullname=fullname), args=[]):
pass

case ListExpr(items=[]):
fullname = "builtins.list"
expr = "[]"

case DictExpr(items=[]):
fullname = "builtins.dict"
expr = "{}"

case TupleExpr(items=[]):
fullname = "builtins.tuple"
expr = "()"

case StrExpr(value=""):
fullname = "builtins.str"
expr = '""'

case BytesExpr(value=""):
fullname = "builtins.bytes"
expr = 'b""'

case IntExpr(value=0):
fullname = "builtins.int"
expr = "0"

case NameExpr(fullname="builtins.False"):
fullname = "builtins.bool"
expr = "False"

case _:
return
Expand All @@ -89,4 +82,9 @@ def check(node: OpExpr, errors: list[Error]) -> None:

# Must check fullname for compatibility with older Mypy versions
if fullname and type_name.startswith(fullname):
errors.append(ErrorInfo.from_node(node, f"Replace `x or {expr}` with `x`"))
lhs_expr = stringify(lhs)
rhs_expr = stringify(arg)

msg = f"Replace `{lhs_expr} or {rhs_expr}` with `{lhs_expr}`"

errors.append(ErrorInfo.from_node(node, msg))
12 changes: 6 additions & 6 deletions test/data/err_113.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
test/data/err_113.py:6:1 [FURB113]: Use `x.extend(...)` instead of repeatedly calling `x.append()`
test/data/err_113.py:13:1 [FURB113]: Use `x.extend(...)` instead of repeatedly calling `x.append()`
test/data/err_113.py:18:1 [FURB113]: Use `x.extend(...)` instead of repeatedly calling `x.append()`
test/data/err_113.py:24:5 [FURB113]: Use `x.extend(...)` instead of repeatedly calling `x.append()`
test/data/err_113.py:29:5 [FURB113]: Use `x.extend(...)` instead of repeatedly calling `x.append()`
test/data/err_113.py:37:5 [FURB113]: Use `x.extend(...)` instead of repeatedly calling `x.append()`
test/data/err_113.py:6:1 [FURB113]: Replace `nums.append(...); nums.append(...)` with `nums.extend((..., ...))`
test/data/err_113.py:13:1 [FURB113]: Replace `nums.append(...); nums.append(...)` with `nums.extend((..., ...))`
test/data/err_113.py:18:1 [FURB113]: Replace `nums.append(...); nums.append(...)` with `nums.extend((..., ...))`
test/data/err_113.py:24:5 [FURB113]: Replace `nums.append(...); nums.append(...)` with `nums.extend((..., ...))`
test/data/err_113.py:29:5 [FURB113]: Replace `nums.append(...); nums.append(...)` with `nums.extend((..., ...))`
test/data/err_113.py:37:5 [FURB113]: Replace `nums.append(...); nums.append(...)` with `nums.extend((..., ...))`
20 changes: 10 additions & 10 deletions test/data/err_143.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
test/data/err_143.py:14:5 [FURB143]: Replace `x or set()` with `x`
test/data/err_143.py:15:5 [FURB143]: Replace `x or frozenset()` with `x`
test/data/err_143.py:16:5 [FURB143]: Replace `x or []` with `x`
test/data/err_143.py:17:5 [FURB143]: Replace `x or {}` with `x`
test/data/err_143.py:18:5 [FURB143]: Replace `x or ()` with `x`
test/data/err_143.py:19:5 [FURB143]: Replace `x or ()` with `x`
test/data/err_143.py:20:5 [FURB143]: Replace `x or ""` with `x`
test/data/err_143.py:21:5 [FURB143]: Replace `x or b""` with `x`
test/data/err_143.py:22:5 [FURB143]: Replace `x or False` with `x`
test/data/err_143.py:23:5 [FURB143]: Replace `x or 0` with `x`
test/data/err_143.py:14:5 [FURB143]: Replace `s or set()` with `s`
test/data/err_143.py:15:5 [FURB143]: Replace `f or frozenset()` with `f`
test/data/err_143.py:16:5 [FURB143]: Replace `l or []` with `l`
test/data/err_143.py:17:5 [FURB143]: Replace `d or {}` with `d`
test/data/err_143.py:18:5 [FURB143]: Replace `t or ()` with `t`
test/data/err_143.py:19:5 [FURB143]: Replace `t2 or ()` with `t2`
test/data/err_143.py:20:5 [FURB143]: Replace `r or ""` with `r`
test/data/err_143.py:21:5 [FURB143]: Replace `b or b""` with `b`
test/data/err_143.py:22:5 [FURB143]: Replace `n or False` with `n`
test/data/err_143.py:23:5 [FURB143]: Replace `i or 0` with `i`
6 changes: 3 additions & 3 deletions test/data/err_145.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
test/data/err_145.py:7:5 [FURB145]: Replace `x[:]` with `x.copy()`
test/data/err_145.py:8:5 [FURB145]: Replace `x[:]` with `x.copy()`
test/data/err_145.py:9:5 [FURB145]: Replace `x[:]` with `x.copy()`
test/data/err_145.py:7:5 [FURB145]: Replace `nums[:]` with `nums.copy()`
test/data/err_145.py:8:5 [FURB145]: Replace `t[:]` with `t.copy()`
test/data/err_145.py:9:5 [FURB145]: Replace `barray[:]` with `barray.copy()`
22 changes: 11 additions & 11 deletions test/data/err_158.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
test/data/err_158.py:5:14 [FURB158]: Replace `bool() as x` with `bool(x)`
test/data/err_158.py:6:14 [FURB158]: Replace `bytearray() as x` with `bytearray(x)`
test/data/err_158.py:7:14 [FURB158]: Replace `bytes() as x` with `bytes(x)`
test/data/err_158.py:8:14 [FURB158]: Replace `dict() as x` with `dict(x)`
test/data/err_158.py:9:14 [FURB158]: Replace `float() as x` with `float(x)`
test/data/err_158.py:10:14 [FURB158]: Replace `frozenset() as x` with `frozenset(x)`
test/data/err_158.py:11:14 [FURB158]: Replace `int() as x` with `int(x)`
test/data/err_158.py:12:14 [FURB158]: Replace `list() as x` with `list(x)`
test/data/err_158.py:13:14 [FURB158]: Replace `set() as x` with `set(x)`
test/data/err_158.py:14:14 [FURB158]: Replace `str() as x` with `str(x)`
test/data/err_158.py:15:14 [FURB158]: Replace `tuple() as x` with `tuple(x)`
test/data/err_158.py:5:14 [FURB158]: Replace `bool() as a` with `bool(a)`
test/data/err_158.py:6:14 [FURB158]: Replace `bytearray() as b` with `bytearray(b)`
test/data/err_158.py:7:14 [FURB158]: Replace `bytes() as c` with `bytes(c)`
test/data/err_158.py:8:14 [FURB158]: Replace `dict() as d` with `dict(d)`
test/data/err_158.py:9:14 [FURB158]: Replace `float() as e` with `float(e)`
test/data/err_158.py:10:14 [FURB158]: Replace `frozenset() as f` with `frozenset(f)`
test/data/err_158.py:11:14 [FURB158]: Replace `int() as g` with `int(g)`
test/data/err_158.py:12:14 [FURB158]: Replace `list() as h` with `list(h)`
test/data/err_158.py:13:14 [FURB158]: Replace `set() as i` with `set(i)`
test/data/err_158.py:14:14 [FURB158]: Replace `str() as j` with `str(j)`
test/data/err_158.py:15:14 [FURB158]: Replace `tuple() as k` with `tuple(k)`
10 changes: 5 additions & 5 deletions test/data/err_166.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
test/data/err_166.py:3:5 [FURB166]: Replace `int(x[2:], 2)` with `int(x, 0)`
test/data/err_166.py:4:5 [FURB166]: Replace `int(x[2:], 8)` with `int(x, 0)`
test/data/err_166.py:5:5 [FURB166]: Replace `int(x[2:], 16)` with `int(x, 0)`
test/data/err_166.py:8:5 [FURB166]: Replace `int(x[2:], 2)` with `int(x, 0)`
test/data/err_166.py:10:5 [FURB166]: Replace `int(x[2:], base=16)` with `int(x, base=0)`
test/data/err_166.py:3:5 [FURB166]: Replace `int("0b1010"[2:], 2)` with `int("0b1010", 0)`
test/data/err_166.py:4:5 [FURB166]: Replace `int("0o777"[2:], 8)` with `int("0o777", 0)`
test/data/err_166.py:5:5 [FURB166]: Replace `int("0xFFFF"[2:], 16)` with `int("0xFFFF", 0)`
test/data/err_166.py:8:5 [FURB166]: Replace `int(b[2:], 2)` with `int(b, 0)`
test/data/err_166.py:10:5 [FURB166]: Replace `int("0xFFFF"[2:], base=16)` with `int("0xFFFF", base=0)`
1 change: 1 addition & 0 deletions test/data/err_169.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@
_ = type(123) is type(456)
_ = type(123) is int
_ = int is type(None)
_ = type() is type(None) # type: ignore
8 changes: 4 additions & 4 deletions test/data/err_169.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
test/data/err_169.py:3:5 [FURB169]: Replace `type(x) is type(None)` with `x is None`
test/data/err_169.py:4:5 [FURB169]: Replace `type(x) == type(None)` with `x is None`
test/data/err_169.py:5:5 [FURB169]: Replace `type(x) is not type(None)` with `x is not None`
test/data/err_169.py:6:5 [FURB169]: Replace `type(x) != type(None)` with `x is not None`
test/data/err_169.py:3:5 [FURB169]: Replace `type(123) is type(None)` with `123 is None`
test/data/err_169.py:4:5 [FURB169]: Replace `type(123) == type(None)` with `123 is None`
test/data/err_169.py:5:5 [FURB169]: Replace `type(123) is not type(None)` with `123 is not None`
test/data/err_169.py:6:5 [FURB169]: Replace `type(123) != type(None)` with `123 is not None`

0 comments on commit 389e0ff

Please sign in to comment.