Skip to content

Commit

Permalink
Remove part of FURB131 (#239):
Browse files Browse the repository at this point in the history
This PR removes a portion of FURB131 which suggested replacing most instances
of `del` with `.pop()`. The reasoning behid it was not well founded, and in
most cases, it is better to just leave the `del` statememt as it is. In one
instance though, which is the `del x[:]` usage, it is more preferable to use
the `.clear()` method, as it is faster and more readable.

Closes #234.
  • Loading branch information
dosisod authored Mar 20, 2023
1 parent 4637180 commit 3f4b865
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 40 deletions.
16 changes: 6 additions & 10 deletions docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -757,21 +757,18 @@ if "key" in d:

Categories: `builtin` `readability`

The `del` statement has it's uses, but for the most part, it can be
replaced with a more flexible and expressive alternative.

With `dict` and `list` types you can remove a key/index by using the
`.pop()` method. If you want to remove all the elements in a `dict` or
`list`, use `.clear()` instead.
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.

Bad:

```python
names = {"key": "value"}
nums = [1, 2, 3]

del names["key"]
del nums[0]
del names[:]
del nums[:]
```

Expand All @@ -781,8 +778,7 @@ Good:
names = {"key": "value"}
nums = [1, 2, 3]

names.pop("key")
nums.pop(0)
names.clear()
nums.clear()
```
## FURB132: `use-set-discard`
Expand Down
33 changes: 8 additions & 25 deletions refurb/checks/builtin/no_del.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,18 @@
@dataclass
class ErrorInfo(Error):
"""
The `del` statement has it's uses, but for the most part, it can be
replaced with a more flexible and expressive alternative.
With `dict` and `list` types you can remove a key/index by using the
`.pop()` method. If you want to remove all the elements in a `dict` or
`list`, use `.clear()` instead.
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.
Bad:
```
names = {"key": "value"}
nums = [1, 2, 3]
del names["key"]
del nums[0]
del names[:]
del nums[:]
```
Expand All @@ -32,15 +29,15 @@ class ErrorInfo(Error):
names = {"key": "value"}
nums = [1, 2, 3]
names.pop("key")
nums.pop(0)
names.clear()
nums.clear()
```
"""

name = "no-del"
code = 131
categories = ["builtin", "readability"]
msg: str = "Replace `del x[:]` with `x.clear()`"


def check(node: DelStmt, errors: list[Error]) -> None:
Expand All @@ -50,18 +47,4 @@ def check(node: DelStmt, errors: list[Error]) -> None:
) if str(ty).startswith(("builtins.dict[", "builtins.list[")):
match index:
case SliceExpr(begin_index=None, end_index=None):
errors.append(
ErrorInfo.from_node(
node, "Replace `del x[:]` with `x.clear()`"
)
)

case SliceExpr():
pass

case _:
errors.append(
ErrorInfo.from_node(
node, "Replace `del x[y]` with `x.pop(y)`"
)
)
errors.append(ErrorInfo.from_node(node))
5 changes: 3 additions & 2 deletions test/data/err_131.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@

# these should match

del names["key"]
del nums[0]
del nums[:]


# these should not

del names["key"]
del nums[0]

x = 1
del x

Expand Down
4 changes: 1 addition & 3 deletions test/data/err_131.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
test/data/err_131.py:6:1 [FURB131]: Replace `del x[y]` with `x.pop(y)`
test/data/err_131.py:7:1 [FURB131]: Replace `del x[y]` with `x.pop(y)`
test/data/err_131.py:8:1 [FURB131]: Replace `del x[:]` with `x.clear()`
test/data/err_131.py:6:1 [FURB131]: Replace `del x[:]` with `x.clear()`

0 comments on commit 3f4b865

Please sign in to comment.