diff --git a/docs/checks.md b/docs/checks.md index 6bf25f4..d834e9d 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -833,32 +833,30 @@ if "key" in d: ... ``` -## FURB131: `no-del` +## FURB131: `use-clear` Categories: `builtin` `readability` -The `del` statement is commonly used for popping single elements from dicts -and lists, though a slice can be used to remove a range of elements -instead. When removing all elements via a slice, use the faster and more -succinct `.clear()` method instead. +Slice expressions can be used to replace part a list without reassigning +it. If you want to clear all the elements out of a list while maintaining +the same reference, don't use `del x[:]` or `x[:] = []`, use the faster +`x.clear()` method instead. Bad: ```python -names = {"key": "value"} nums = [1, 2, 3] -del names[:] del nums[:] +# or +nums[:] = [] ``` Good: ```python -names = {"key": "value"} nums = [1, 2, 3] -names.clear() nums.clear() ``` diff --git a/refurb/checks/builtin/no_del.py b/refurb/checks/builtin/no_del.py index 6fe35ad..3bd2543 100644 --- a/refurb/checks/builtin/no_del.py +++ b/refurb/checks/builtin/no_del.py @@ -1,6 +1,6 @@ from dataclasses import dataclass -from mypy.nodes import DelStmt, IndexExpr, NameExpr, SliceExpr, Var +from mypy.nodes import AssignmentStmt, DelStmt, IndexExpr, ListExpr, RefExpr, SliceExpr, Var from refurb.checks.common import stringify from refurb.error import Error @@ -9,46 +9,59 @@ @dataclass class ErrorInfo(Error): """ - The `del` statement is commonly used for popping single elements from dicts - and lists, though a slice can be used to remove a range of elements - instead. When removing all elements via a slice, use the faster and more - succinct `.clear()` method instead. + Slice expressions can be used to replace part a list without reassigning + it. If you want to clear all the elements out of a list while maintaining + the same reference, don't use `del x[:]` or `x[:] = []`, use the faster + `x.clear()` method instead. Bad: ``` - names = {"key": "value"} nums = [1, 2, 3] - del names[:] del nums[:] + # or + nums[:] = [] ``` Good: ``` - names = {"key": "value"} nums = [1, 2, 3] - names.clear() nums.clear() ``` """ - name = "no-del" + name = "use-clear" code = 131 categories = ("builtin", "readability") -def check(node: DelStmt, errors: list[Error]) -> None: +def check(node: DelStmt | AssignmentStmt, errors: list[Error]) -> None: match node: - case DelStmt(expr=IndexExpr(base=NameExpr(node=Var(type=ty)) as expr, index=index)) if str( - ty - ).startswith(("builtins.dict[", "builtins.list[")): - match index: - case SliceExpr(begin_index=None, end_index=None): - expr = stringify(expr) # type: ignore + case DelStmt( + expr=IndexExpr( + base=RefExpr(node=Var(type=ty)) as name, + index=SliceExpr(begin_index=None, end_index=None, stride=None), + ), + ) if str(ty).startswith("builtins.list["): + pass - msg = f"Replace `del {expr}[:]` with `{expr}.clear()`" + case AssignmentStmt( + lvalues=[ + IndexExpr( + base=RefExpr(node=Var(type=ty)) as name, + index=SliceExpr(begin_index=None, end_index=None, stride=None), + ), + ], + rvalue=ListExpr(items=[]), + ) if str(ty).startswith("builtins.list["): + pass - errors.append(ErrorInfo.from_node(node, msg)) + case _: + return + + msg = f"Replace `{stringify(node)}` with `{stringify(name)}.clear()`" + + errors.append(ErrorInfo.from_node(node, msg)) diff --git a/refurb/checks/common.py b/refurb/checks/common.py index de3c311..5ab9555 100644 --- a/refurb/checks/common.py +++ b/refurb/checks/common.py @@ -10,6 +10,7 @@ ComparisonExpr, ComplexExpr, ConditionalExpr, + DelStmt, DictExpr, DictionaryComprehension, Expression, @@ -420,6 +421,9 @@ def _stringify(node: Node) -> str: case ConditionalExpr(if_expr=if_true, cond=cond, else_expr=if_false): return f"{_stringify(if_true)} if {_stringify(cond)} else {_stringify(if_false)}" + case DelStmt(expr=expr): + return f"del {stringify(expr)}" + raise ValueError diff --git a/test/data/err_131.py b/test/data/err_131.py index 80bef79..d5ee130 100644 --- a/test/data/err_131.py +++ b/test/data/err_131.py @@ -1,17 +1,29 @@ -names = {"key": "value"} nums = [1, 2, 3] # these should match del nums[:] +nums[:] = [] + # these should not +names = {"key": "value"} + del names["key"] +del names[:] # type: ignore + del nums[0] x = 1 del x -del nums[1:2] +del nums[1:] +del nums[:1] +del nums[::1] + +nums[:] = [1, 2, 3] +nums[1:] = [] +nums[:1] = [] +nums[::1] = [] diff --git a/test/data/err_131.txt b/test/data/err_131.txt index a58bfbf..203efd1 100644 --- a/test/data/err_131.txt +++ b/test/data/err_131.txt @@ -1 +1,2 @@ -test/data/err_131.py:6:1 [FURB131]: Replace `del nums[:]` with `nums.clear()` +test/data/err_131.py:5:1 [FURB131]: Replace `del nums[:]` with `nums.clear()` +test/data/err_131.py:7:1 [FURB131]: Replace `nums[:] = []` with `nums.clear()`