Skip to content

Commit 1237de7

Browse files
worksbyfridayclaude
andcommitted
Tighten _is_simple_expr to only allow names and literals
As dosisod correctly pointed out, member access (@Property), unary operators (__neg__), and binary operators (__add__) can all have side effects via dunder methods. Only bare names and literals are truly safe for eager evaluation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e657110 commit 1237de7

2 files changed

Lines changed: 9 additions & 19 deletions

File tree

refurb/checks/logical/use_in.py

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,14 @@
22

33
from mypy.nodes import (
44
BytesExpr,
5-
CallExpr,
65
ComparisonExpr,
76
ComplexExpr,
87
Expression,
98
FloatExpr,
10-
IndexExpr,
119
IntExpr,
12-
MemberExpr,
1310
NameExpr,
1411
OpExpr,
1512
StrExpr,
16-
UnaryExpr,
1713
)
1814

1915
from refurb.checks.common import (
@@ -57,26 +53,14 @@ class ErrorInfo(Error):
5753
def _is_simple_expr(node: Expression) -> bool:
5854
"""Check if an expression is simple enough to be safely eagerly evaluated.
5955
60-
Simple expressions are those that cannot raise exceptions or have side
61-
effects when evaluated, making them safe for use in `in` tuple checks
62-
where short-circuit evaluation is lost.
56+
Only names and literals are considered safe. Member access, unary
57+
operators, and binary operators are excluded because @property and
58+
dunder methods (__neg__, __add__, etc.) can have side effects.
6359
"""
6460
match node:
6561
case NameExpr() | IntExpr() | StrExpr() | BytesExpr() | FloatExpr() | ComplexExpr():
6662
return True
6763

68-
case MemberExpr(expr=expr):
69-
return _is_simple_expr(expr)
70-
71-
case UnaryExpr(expr=expr):
72-
return _is_simple_expr(expr)
73-
74-
case OpExpr(left=left, right=right):
75-
return _is_simple_expr(left) and _is_simple_expr(right)
76-
77-
case IndexExpr() | CallExpr():
78-
return False
79-
8064
return False
8165

8266

test/data/err_108.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,9 @@ class C:
3434
d = {"a": 1}
3535
_ = x == "abc" or x == d["a"]
3636
_ = x == len(x) or x == "abc"
37+
38+
# member access, unary, and binary ops as non-common operands should not match
39+
# because @property and dunder methods can have side effects
40+
_ = x == c.y or x == "abc"
41+
_ = x == -y or x == 1
42+
_ = x == (y + x) or x == 2

0 commit comments

Comments
 (0)