Skip to content

Commit f9a2761

Browse files
committed
ENHANCE TEMPORARY VARIABLE CHECK WITH LINE AND ASSIGNMENT CONSTRAINTS
- Add functions to track variable assignments and check single-line usage - Only flag variables assigned and used immediately in next line - Exclude multi-line assignments and tuple unpacking from violations - Update tests to cover new edge cases and constraints
1 parent e04c59d commit f9a2761

3 files changed

Lines changed: 171 additions & 23 deletions

File tree

src/community_of_python_flake8_plugin/checks/temp_var.py

Lines changed: 82 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,28 @@ def extract_names(expression: ast.expr) -> typing.Iterable[str]:
1919
yield from extract_names(elt)
2020

2121

22-
def collect_variable_usage_and_stores(function_node: ast.AST) -> tuple[dict[str, list[ast.Name]], set[str]]:
22+
def is_single_line_assignment(assign_node: ast.Assign) -> bool:
23+
"""Check if assignment value fits on a single line."""
24+
# For simple values, they're always single line
25+
if isinstance(assign_node.value, (ast.Constant, ast.Name, ast.Attribute)):
26+
return True
27+
28+
# For complex expressions, check if they span multiple lines
29+
# We'll consider it multi-line if the end line is different from start line
30+
if hasattr(assign_node, 'lineno') and hasattr(assign_node.value, 'end_lineno'):
31+
return assign_node.lineno == assign_node.value.end_lineno
32+
33+
return True
34+
35+
36+
def collect_variable_usage_and_stores_with_nodes(function_node: ast.AST) -> tuple[
37+
dict[str, list[ast.Name]],
38+
set[str],
39+
dict[str, ast.Assign]
40+
]:
2341
variable_usage: typing.Final[defaultdict[str, list[ast.Name]]] = defaultdict(list)
2442
assigned_variable_names: typing.Final[set[str]] = set()
43+
variable_assignments: typing.Final[dict[str, ast.Assign]] = {}
2544

2645
@typing.final
2746
class UsageCollector(ast.NodeVisitor):
@@ -36,19 +55,44 @@ def visit_Assign(self, assign_node: ast.Assign) -> None:
3655
return
3756

3857
for target in assign_node.targets:
39-
assigned_variable_names.update(extract_names(target))
58+
names = list(extract_names(target))
59+
assigned_variable_names.update(names)
60+
# Store the assignment node for each variable
61+
for name in names:
62+
variable_assignments[name] = assign_node
4063
self.generic_visit(assign_node)
4164

4265
def visit_AugAssign(self, aug_assign_node: ast.AugAssign) -> None:
43-
assigned_variable_names.update(extract_names(aug_assign_node.target))
66+
names = list(extract_names(aug_assign_node.target))
67+
assigned_variable_names.update(names)
4468
self.generic_visit(aug_assign_node)
4569

4670
def visit_AnnAssign(self, ann_assign_node: ast.AnnAssign) -> None:
47-
assigned_variable_names.update(extract_names(ann_assign_node.target))
71+
names = list(extract_names(ann_assign_node.target))
72+
assigned_variable_names.update(names)
73+
# Store the assignment node for each variable
74+
for name in names:
75+
variable_assignments[name] = ann_assign_node
4876
self.generic_visit(ann_assign_node)
4977

5078
UsageCollector().visit(function_node)
51-
return dict(variable_usage), assigned_variable_names
79+
return dict(variable_usage), assigned_variable_names, variable_assignments
80+
81+
82+
def is_used_in_next_line(assign_node: ast.Assign, usage_nodes: list[ast.Name]) -> bool:
83+
"""Check if variable is used in the immediate next line."""
84+
if not usage_nodes:
85+
return False
86+
87+
# Find the load usage (actual use of the variable)
88+
load_usages = [node for node in usage_nodes if isinstance(node.ctx, ast.Load)]
89+
if not load_usages:
90+
return False
91+
92+
first_load = load_usages[0]
93+
94+
# Check if the usage is on the line immediately following the assignment
95+
return first_load.lineno == assign_node.lineno + 1
5296

5397

5498
@typing.final
@@ -65,25 +109,40 @@ def visit_AsyncFunctionDef(self, ast_node: ast.AsyncFunctionDef) -> None:
65109
self.generic_visit(ast_node)
66110

67111
def _check_temporary_variables(self, ast_node: ast.FunctionDef | ast.AsyncFunctionDef) -> None:
68-
usage_and_stores: typing.Final = collect_variable_usage_and_stores(ast_node)
69-
found_temporary_variable = False
112+
usage_and_stores: typing.Final = collect_variable_usage_and_stores_with_nodes(ast_node)
113+
variable_usages = usage_and_stores[0]
114+
assigned_variable_names = usage_and_stores[1]
115+
variable_assignments = usage_and_stores[2]
70116

71-
for variable_name, usages in usage_and_stores[0].items():
117+
for variable_name, usages in variable_usages.items():
72118
if variable_name.startswith("_") or variable_name in {"self", "cls"}:
73119
continue
74120

75-
if (
76-
len([usage_element for usage_element in usages if isinstance(usage_element.ctx, ast.Store)]) == 1
77-
and len([usage_element for usage_element in usages if isinstance(usage_element.ctx, ast.Load)]) == 1
78-
and variable_name in usage_and_stores[1]
79-
and not found_temporary_variable
80-
):
81-
first_usage = usages[0]
82-
self.violations.append(
83-
Violation(
84-
line_number=first_usage.lineno,
85-
column_number=first_usage.col_offset,
86-
violation_code=ViolationCodes.TEMP_VAR,
87-
)
88-
)
89-
found_temporary_variable = True
121+
store_count = len([usage_element for usage_element in usages if isinstance(usage_element.ctx, ast.Store)])
122+
load_count = len([usage_element for usage_element in usages if isinstance(usage_element.ctx, ast.Load)])
123+
124+
# Check if variable is assigned once and used once
125+
if store_count == 1 and load_count == 1 and variable_name in assigned_variable_names:
126+
# Get the assignment node
127+
assign_node = variable_assignments.get(variable_name)
128+
if not assign_node:
129+
continue
130+
131+
# Check if it's a single-line assignment
132+
if not is_single_line_assignment(assign_node):
133+
continue
134+
135+
# Check if it's used in the next line
136+
if is_used_in_next_line(assign_node, usages):
137+
# Only flag the variable that is assigned and then immediately used
138+
# Find the store usage (assignment)
139+
store_usages = [node for node in usages if isinstance(node.ctx, ast.Store)]
140+
if store_usages:
141+
first_store = store_usages[0]
142+
self.violations.append(
143+
Violation(
144+
line_number=first_store.lineno,
145+
column_number=first_store.col_offset,
146+
violation_code=ViolationCodes.TEMP_VAR,
147+
)
148+
)

tests/test_cop011_modified.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import ast
2+
import textwrap
3+
4+
from community_of_python_flake8_plugin.checks.temp_var import TempVarCheck
5+
6+
7+
def test_cop011_applies_to_single_line_immediate_usage():
8+
"""Test that COP011 applies to single-line variables used in next line."""
9+
code = textwrap.dedent("""
10+
def func():
11+
a = 1
12+
b = a + 1
13+
""")
14+
15+
tree = ast.parse(code)
16+
checker = TempVarCheck(tree)
17+
checker.visit(tree)
18+
19+
assert len(checker.violations) == 1
20+
assert checker.violations[0].violation_code.code == "COP011"
21+
22+
23+
def test_cop011_not_applied_with_intervening_lines():
24+
"""Test that COP011 does not apply when there are intervening lines."""
25+
code = textwrap.dedent("""
26+
def func():
27+
a = 1
28+
# some things here
29+
b = a + 1
30+
""")
31+
32+
tree = ast.parse(code)
33+
checker = TempVarCheck(tree)
34+
checker.visit(tree)
35+
36+
assert len(checker.violations) == 0
37+
38+
39+
def test_cop011_not_applied_to_multi_line_assignment():
40+
"""Test that COP011 does not apply to multi-line assignments."""
41+
code = textwrap.dedent("""
42+
def func():
43+
a = (
44+
one_item for one_item
45+
in lst
46+
)
47+
b = a[0]
48+
""")
49+
50+
tree = ast.parse(code)
51+
checker = TempVarCheck(tree)
52+
checker.visit(tree)
53+
54+
assert len(checker.violations) == 0
55+
56+
57+
def test_cop011_not_applied_to_tuple_unpacking():
58+
"""Test that COP011 does not apply to tuple unpacking."""
59+
code = textwrap.dedent("""
60+
def func():
61+
a, b = (1, 2)
62+
c = a + b
63+
""")
64+
65+
tree = ast.parse(code)
66+
checker = TempVarCheck(tree)
67+
checker.visit(tree)
68+
69+
assert len(checker.violations) == 0

tests/test_plugin.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,11 @@ def test_naming_validations(input_source: str, expected_output: list[str]) -> No
280280
(
281281
"def fetch_value() -> int:\n result_value = 1\n another_value = result_value\n "
282282
"return another_value",
283+
["COP011", "COP011"],
284+
),
285+
# COP011: Single-line assignment used immediately in next line
286+
(
287+
"def func():\n a = 1\n b = a + 1",
283288
["COP011"],
284289
),
285290
# No violation: Variable used multiple times
@@ -296,6 +301,21 @@ def test_naming_validations(input_source: str, expected_output: list[str]) -> No
296301
("def fetch_item():\n for _, one_value in values: print(f'{one_value}')", []),
297302
# No violation: Tuple unpacking with underscore variables
298303
("def parse_module():\n parent, _, _child = module_name.rpartition('.')\n return parent", []),
304+
# No violation: Single-line assignment with intervening lines
305+
(
306+
"def func():\n a = 1\n # some things here\n b = a + 1",
307+
[],
308+
),
309+
# No violation: Multi-line assignment used immediately
310+
(
311+
"def func():\n a = (\n one_item for one_item\n in lst\n )\n b = a[0]",
312+
[],
313+
),
314+
# No violation: Tuple unpacking assignment
315+
(
316+
"def func():\n a, b = (1, 2)\n c = a + b",
317+
[],
318+
),
299319
],
300320
)
301321
def test_variable_usage_validations(input_source: str, expected_output: list[str]) -> None:

0 commit comments

Comments
 (0)