Skip to content

Commit d44dc34

Browse files
authored
use-walrus-if codemod can handle unused variables (#477)
* use-walrus-if codemod can handle unused variables * cleanup
1 parent 58e1a6f commit d44dc34

File tree

2 files changed

+67
-5
lines changed

2 files changed

+67
-5
lines changed

src/core_codemods/use_walrus_if.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from libcst._position import CodeRange
77
from libcst.metadata import ParentNodeProvider, ScopeProvider
88

9+
from codemodder.codemods.utils_mixin import NameResolutionMixin
910
from core_codemods.api import Metadata, Reference, ReviewGuidance, SimpleCodemod
1011

1112
FoundAssign = namedtuple("FoundAssign", ["assign", "target", "value"])
@@ -17,7 +18,7 @@ def pairwise(iterable):
1718
return zip(a, b)
1819

1920

20-
class UseWalrusIf(SimpleCodemod):
21+
class UseWalrusIf(SimpleCodemod, NameResolutionMixin):
2122
metadata = Metadata(
2223
name="use-walrus-if",
2324
summary="Use Assignment Expression (Walrus) In Conditional",
@@ -76,6 +77,16 @@ def _filter_if(self, node: cst.CSTNode) -> cst.BaseExpression | None:
7677
return test
7778
return None
7879

80+
def _single_access(self, original_node: cst.IfExp) -> bool:
81+
match original_node.test:
82+
case cst.Name():
83+
access = self.find_accesses(original_node.test)
84+
case cst.UnaryOperation():
85+
access = self.find_accesses(original_node.test.expression)
86+
case _:
87+
access = self.find_accesses(original_node.test.left)
88+
return len(access) == 1
89+
7990
def on_visit(self, node: cst.CSTNode) -> Optional[bool]:
8091
if len(node.children) < 2:
8192
return super().on_visit(node)
@@ -87,7 +98,6 @@ def on_visit(self, node: cst.CSTNode) -> Optional[bool]:
8798
continue
8899

89100
assign, target, value = found_assign
90-
91101
match if_test:
92102
# If test can be a comparison expression
93103
case cst.Comparison(
@@ -130,16 +140,24 @@ def leave_If(self, original_node, updated_node):
130140
if (result := self._if_stack.pop()) is not None:
131141
position, named_expr = result
132142
self.add_change_from_position(position, self.change_description)
143+
144+
# If a variable has a single access, it means it's only assigned and not used again.
145+
# In this case, do not use a walrus named expr to prevent unused variable warnings.
146+
# Instead, move the variable's rhs directly into the if statement.
147+
new_expression = (
148+
named_expr.value if self._single_access(original_node) else named_expr
149+
)
150+
133151
match updated_node.test:
134152
case cst.Name():
135-
return updated_node.with_changes(test=named_expr)
153+
return updated_node.with_changes(test=new_expression)
136154
case cst.UnaryOperation():
137155
return updated_node.with_changes(
138-
test=updated_node.test.with_changes(expression=named_expr)
156+
test=updated_node.test.with_changes(expression=new_expression)
139157
)
140158
case _:
141159
return updated_node.with_changes(
142-
test=updated_node.test.with_changes(left=named_expr)
160+
test=updated_node.test.with_changes(left=new_expression)
143161
)
144162

145163
return original_node

tests/codemods/test_walrus_if.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,3 +225,47 @@ def test_multiple_walrus_changes(self, tmpdir):
225225
assert changes[0].lineNumber == 2
226226
assert changes[1].lineNumber == 5
227227
assert changes[2].lineNumber == 8
228+
229+
def test_no_walrus_if_unused_variable(self, tmpdir):
230+
input_code = """
231+
test = (
232+
1
233+
if True
234+
else 0
235+
)
236+
if test:
237+
print("var will not be reused")
238+
"""
239+
expected_output = """
240+
if (
241+
1
242+
if True
243+
else 0
244+
):
245+
print("var will not be reused")
246+
"""
247+
self.run_and_assert(tmpdir, input_code, expected_output)
248+
249+
def test_no_walrus_if_not(self, tmpdir):
250+
input_code = """
251+
val = do_something()
252+
if not val:
253+
print("something")
254+
"""
255+
expected_output = """
256+
if not do_something():
257+
print("something")
258+
"""
259+
self.run_and_assert(tmpdir, input_code, expected_output)
260+
261+
def test_no_walrus_if(self, tmpdir):
262+
input_code = """
263+
val = do_something()
264+
if val is None:
265+
print("hi")
266+
"""
267+
expected_output = """
268+
if do_something() is None:
269+
print("hi")
270+
"""
271+
self.run_and_assert(tmpdir, input_code, expected_output)

0 commit comments

Comments
 (0)