Skip to content

Commit 069e168

Browse files
committed
ci: simplify the pool-props lint without changing behavior
Fold the pool-props check into one recursive _pool_props_violations that walks each scope once: it collects the caps and the constructions in the same pass and recurses into nested scopes, replacing the separate _capped_variables and _assigned_pool_props helpers and the double walk. Add a _report helper so the two rule outputs no longer duplicate the print/bullet/hint block. Same rules, same messages; all lint tests pass.
1 parent 0035e30 commit 069e168

1 file changed

Lines changed: 63 additions & 67 deletions

File tree

‎ci/tools/check_mempool_hygiene.py‎

Lines changed: 63 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ def _opted_out(lines: list[str], node: ast.AST) -> bool:
9797

9898

9999
def _walk_scope(scope: ast.AST) -> Iterator[ast.AST]:
100-
"""Yield the nodes of ``scope`` without entering nested functions, lambdas, or classes."""
100+
"""Yield ``scope``'s descendants, yielding but not entering nested function/lambda/class bodies."""
101101
pending = list(ast.iter_child_nodes(scope))
102102
while pending:
103103
node = pending.pop()
@@ -106,55 +106,49 @@ def _walk_scope(scope: ast.AST) -> Iterator[ast.AST]:
106106
pending.extend(ast.iter_child_nodes(node))
107107

108108

109-
def _assigned_pool_props(node: ast.AST) -> tuple[str, str] | None:
110-
"""Return ``(variable, type name)`` for ``variable = <PoolPropsType>(...)``."""
111-
if not (isinstance(node, ast.Assign) and len(node.targets) == 1 and isinstance(node.value, ast.Call)):
112-
return None
113-
target = node.targets[0]
114-
callee = _callee_name(node.value)
115-
if isinstance(target, ast.Name) and callee in POOL_PROPS_TYPES:
116-
return target.id, callee
117-
return None
109+
def _assign_targets(node: ast.AST) -> list[ast.expr]:
110+
"""The assignment targets of ``node``, or ``[]`` if it is not an assignment."""
111+
if isinstance(node, ast.Assign):
112+
return node.targets
113+
if isinstance(node, (ast.AugAssign, ast.AnnAssign)):
114+
return [node.target]
115+
return []
118116

119117

120-
def _capped_variables(scope: ast.AST) -> set[str]:
121-
"""Names with ``name.max_size`` or ``name.maxSize`` assigned directly in ``scope``."""
122-
capped = set()
118+
def _pool_props_violations(path: Path, scope: ast.AST, lines: list[str]) -> list[Violation]:
119+
"""Report each ``var = PoolPropsType(...)`` with no ``var.max_size`` set in the same scope.
120+
121+
Recurses so a cap only counts in the scope that built the props; a cap on a
122+
same-named variable elsewhere does not excuse it.
123+
"""
124+
found: list[Violation] = []
125+
constructed: list[tuple[ast.Assign, str, str]] = [] # (node, variable, type name)
126+
capped: set[str] = set()
123127
for node in _walk_scope(scope):
124-
if isinstance(node, ast.Assign):
125-
targets = node.targets
126-
elif isinstance(node, (ast.AugAssign, ast.AnnAssign)):
127-
targets = [node.target]
128-
else:
128+
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
129+
found += _pool_props_violations(path, node, lines)
129130
continue
130-
capped.update(
131-
target.value.id
132-
for target in targets
133-
if isinstance(target, ast.Attribute)
134-
and target.attr in POOL_PROPS_CAP_ATTRIBUTES
135-
and isinstance(target.value, ast.Name)
136-
)
137-
return capped
138-
139-
140-
def _pool_props_violations(path: Path, tree: ast.AST, lines: list[str]) -> list[Violation]:
141-
found = []
142-
scopes = [tree, *(n for n in ast.walk(tree) if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)))]
143-
for scope in scopes:
144-
capped = _capped_variables(scope)
145-
for node in _walk_scope(scope):
146-
assigned = _assigned_pool_props(node)
147-
if assigned is None:
148-
continue
149-
variable, type_name = assigned
150-
if variable not in capped and not _opted_out(lines, node):
151-
found.append(
152-
Violation(
153-
RULE_PROPS,
154-
f"{path.as_posix()}:{node.lineno}: {type_name} assigned to '{variable}' "
155-
f"without {variable}.max_size / {variable}.maxSize",
156-
)
131+
for target in _assign_targets(node):
132+
if (
133+
isinstance(target, ast.Attribute)
134+
and target.attr in POOL_PROPS_CAP_ATTRIBUTES
135+
and isinstance(target.value, ast.Name)
136+
):
137+
capped.add(target.value.id)
138+
if isinstance(node, ast.Assign) and len(node.targets) == 1 and isinstance(node.value, ast.Call):
139+
(target,) = node.targets
140+
type_name = _callee_name(node.value)
141+
if isinstance(target, ast.Name) and type_name in POOL_PROPS_TYPES:
142+
constructed.append((node, target.id, type_name))
143+
for node, variable, type_name in constructed:
144+
if variable not in capped and not _opted_out(lines, node):
145+
found.append(
146+
Violation(
147+
RULE_PROPS,
148+
f"{path.as_posix()}:{node.lineno}: {type_name} assigned to '{variable}' "
149+
f"without {variable}.max_size / {variable}.maxSize",
157150
)
151+
)
158152
return found
159153

160154

@@ -189,6 +183,16 @@ def violations_in(path: Path) -> list[str]:
189183
return [violation.message for violation in _violations(path)]
190184

191185

186+
def _report(header: str, messages: list[str], hint: str) -> None:
187+
"""Print ``header``, the bulleted ``messages``, and ``hint`` to stderr; a no-op if empty."""
188+
if not messages:
189+
return
190+
print(header, file=sys.stderr)
191+
for message in messages:
192+
print(f" - {message}", file=sys.stderr)
193+
print(hint, file=sys.stderr)
194+
195+
192196
def main(argv: list[str] | None = None) -> int:
193197
parser = argparse.ArgumentParser(description=__doc__)
194198
parser.add_argument(
@@ -206,28 +210,20 @@ def main(argv: list[str] | None = None) -> int:
206210
if not violations:
207211
return 0
208212

209-
resource_messages = [v.message for v in violations if v.rule == RULE_RESOURCE]
210-
props_messages = [v.message for v in violations if v.rule == RULE_PROPS]
211-
if resource_messages:
212-
print("error: memory pools created by tests must set max_size:", file=sys.stderr)
213-
for message in resource_messages:
214-
print(f" - {message}", file=sys.stderr)
215-
print(
216-
f"Use the suite-wide POOL_SIZE from cuda_core/tests/helpers/constants.py, or annotate a\n"
217-
f"deliberate exception with a '# {OPT_OUT_MARKER}: <reason>' comment.\n"
218-
f"See cuda_core/tests/AGENTS.md.",
219-
file=sys.stderr,
220-
)
221-
if props_messages:
222-
print("error: pool properties created by tests must set max_size / maxSize:", file=sys.stderr)
223-
for message in props_messages:
224-
print(f" - {message}", file=sys.stderr)
225-
print(
226-
f"Set max_size (v2 bindings) or maxSize (other bindings) on the props object you constructed,\n"
227-
f"using POOL_SIZE from cuda_python_test_helpers.memory, or annotate a deliberate exception\n"
228-
f"with a '# {OPT_OUT_MARKER}: <reason>' comment. See cuda_bindings_13/AGENTS.md.",
229-
file=sys.stderr,
230-
)
213+
_report(
214+
"error: memory pools created by tests must set max_size:",
215+
[v.message for v in violations if v.rule == RULE_RESOURCE],
216+
f"Use the suite-wide POOL_SIZE from cuda_core/tests/helpers/constants.py, or annotate a\n"
217+
f"deliberate exception with a '# {OPT_OUT_MARKER}: <reason>' comment.\n"
218+
f"See cuda_core/tests/AGENTS.md.",
219+
)
220+
_report(
221+
"error: pool properties created by tests must set max_size / maxSize:",
222+
[v.message for v in violations if v.rule == RULE_PROPS],
223+
f"Set max_size (v2 bindings) or maxSize (other bindings) on the props object you constructed,\n"
224+
f"using POOL_SIZE from cuda_python_test_helpers.memory, or annotate a deliberate exception\n"
225+
f"with a '# {OPT_OUT_MARKER}: <reason>' comment. See cuda_bindings_13/AGENTS.md.",
226+
)
231227
return 1
232228

233229

0 commit comments

Comments
 (0)