Skip to content

Commit

Permalink
Add use-isinstance-bool check
Browse files Browse the repository at this point in the history
  • Loading branch information
dosisod committed Mar 4, 2024
1 parent 43c63c8 commit 85ef890
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 0 deletions.
21 changes: 21 additions & 0 deletions docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -2414,4 +2414,25 @@ def normalize_phone_number(phone_number: str) -> int:
digits = filter(str.isdigit, phone_number)

return int("".join(digits))
```

## FURB191: `use-isinstance-bool`

Categories: `readability`

Don't check if a value is `True` or `False` using `in`, use an
`isinstance()` call.

Bad:

```python
if value in {True, False}:
pass
```

Good:

```python
if isinstance(value, bool):
pass
```
69 changes: 69 additions & 0 deletions refurb/checks/readability/use_isinstance_bool.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
from dataclasses import dataclass

from mypy.nodes import ComparisonExpr, Expression, ListExpr, NameExpr, SetExpr, TupleExpr

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


@dataclass
class ErrorInfo(Error):
"""
Don't check if a value is `True` or `False` using `in`, use an
`isinstance()` call.
Bad:
```
if value in {True, False}:
pass
```
Good:
```
if isinstance(value, bool):
pass
```
"""

name = "use-isinstance-bool"
code = 191
categories = ("readability",)


# TODO: move to common
def is_true(expr: Expression) -> bool:
match expr:
case NameExpr(fullname="builtins.True"):
return True

return False


def is_false(expr: Expression) -> bool:
match expr:
case NameExpr(fullname="builtins.False"):
return True

return False


def check(node: ComparisonExpr, errors: list[Error]) -> None:
match node:
case ComparisonExpr(
operators=["in" | "not in" as op],
operands=[
lhs,
SetExpr(items=[t, f]) | TupleExpr(items=[t, f]) | ListExpr(items=[t, f]),
],
) if (is_true(t) and is_false(f)) or (is_false(t) and is_true(f)):
old = stringify(node)
new = f"isinstance({stringify(lhs)}, bool)"

if op == "not in":
new = f"not {new}"

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

errors.append(ErrorInfo.from_node(node, msg))
54 changes: 54 additions & 0 deletions test/data/err_191.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
b = True

# these should match

if b in {True, False}: pass
if b in {False, True}: pass
if b in [True, False]: pass # noqa: FURB109
if b in [False, True]: pass # noqa: FURB109
if b in (True, False): pass
if b in (False, True): pass

_ = [x for x in [] if b in (True, False)] # noqa: FURB109
_ = {x for x in [] if b in (True, False)} # noqa: FURB109
_ = (x for x in [] if b in (True, False)) # noqa: FURB109
_ = {k: v for k, v in {}.items() if b in (True, False)}

_ = 1 if b in (True, False) else 2

match b:
case _ if b in (True, False):
pass

while b in {True, False}:
pass

assert b in {True, False}

if b not in {True, False}: pass


# these should not
if b in {True}: pass # noqa: FURB171
if b in [True]: pass # noqa: FURB109, FURB171
if b in (True,): pass # noqa: FURB171

if b in {True, True}: pass

for x in [True, False]: pass # noqa: FURB109

_ = [x for x in [True, False]] # noqa: FURB109
_ = {x for x in [True, False]} # noqa: FURB109
_ = (x for x in [True, False]) # noqa: FURB109

# TODO: suggest `isinstance(x, bool) or y`
if b in {True, False, 123}: pass
if b in {False, True, 123}: pass
if b in [True, False, 123]: pass # noqa: FURB109
if b in [False, True, 123]: pass # noqa: FURB109
if b in (True, False, 123): pass
if b in (False, True, 123): pass

if b == {True, False}: pass
if b == {False, True}: pass
if b == [True, False]: pass
15 changes: 15 additions & 0 deletions test/data/err_191.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
test/data/err_191.py:5:4 [FURB191]: Replace `b in {True, False}` with `isinstance(b, bool)`
test/data/err_191.py:6:4 [FURB191]: Replace `b in {False, True}` with `isinstance(b, bool)`
test/data/err_191.py:7:4 [FURB191]: Replace `b in [True, False]` with `isinstance(b, bool)`
test/data/err_191.py:8:4 [FURB191]: Replace `b in [False, True]` with `isinstance(b, bool)`
test/data/err_191.py:9:4 [FURB191]: Replace `b in (True, False)` with `isinstance(b, bool)`
test/data/err_191.py:10:4 [FURB191]: Replace `b in (False, True)` with `isinstance(b, bool)`
test/data/err_191.py:12:23 [FURB191]: Replace `b in (True, False)` with `isinstance(b, bool)`
test/data/err_191.py:13:23 [FURB191]: Replace `b in (True, False)` with `isinstance(b, bool)`
test/data/err_191.py:14:23 [FURB191]: Replace `b in (True, False)` with `isinstance(b, bool)`
test/data/err_191.py:15:37 [FURB191]: Replace `b in (True, False)` with `isinstance(b, bool)`
test/data/err_191.py:17:10 [FURB191]: Replace `b in (True, False)` with `isinstance(b, bool)`
test/data/err_191.py:20:15 [FURB191]: Replace `b in (True, False)` with `isinstance(b, bool)`
test/data/err_191.py:23:7 [FURB191]: Replace `b in {True, False}` with `isinstance(b, bool)`
test/data/err_191.py:26:8 [FURB191]: Replace `b in {True, False}` with `isinstance(b, bool)`
test/data/err_191.py:28:4 [FURB191]: Replace `b not in {True, False}` with `not isinstance(b, bool)`

0 comments on commit 85ef890

Please sign in to comment.