AST Parsing and Rewriting

## One dispatch mechanism, two contracts `ast.NodeVisitor` and `ast.NodeTransformer` share their dispatch. You call `visitor.visit(tree)`; `visit` looks up a method named `visit_` plus the node's class name - `visit_Call`, `visit_FunctionDef`, `visit_Constant` - and calls it. If no such method exists, `generic_visit` runs instead, and `generic_visit` is the part that actually walks into the node's children. `ast.NodeTransformer` subclasses `ast.NodeVisitor`; it overrides `generic_visit` so that the children it walks are also *replaced* by whatever the child visits return. So the mechanism is the same and the **contract on the return value** is what differs. ## NodeVisitor: read-only, accumulate on self With a plain visitor the return value goes nowhere useful - the traversal ignores it. The idiom is therefore to accumulate state on the instance: ```python class CallCounter(ast.NodeVisitor): def __init__(self): self.calls = [] def visit_Call(self, node): if isinstance(node.func, ast.Name): self.calls.append((node.func.id, node.lineno)) self.generic_visit(node) ``` That is the shape of essentially every hand-written linter rule: match a node type, decide, record a finding with its position, keep walking. ## NodeTransformer: the return value is the edit With a transformer, whatever a `visit_X` method returns takes the old node's place in its parent: - **return `node`** - keep it (possibly after mutating its fields in place); - **return a different node** - substitute it; - **return `None`** - delete the node from its parent's list; - **return a list of nodes** - splice them in, but only where the parent field holds a *list of statements*. Returning a list where a single expression is expected produces a broken tree, not a syntax error, and the failure surfaces later at `compile()`. Deletion is the sharp edge: `None` deletes silently, so a transformer that forgets to return the node on its non-matching branch quietly strips code. Make the last line of every `visit_` method an explicit `return node`. ## The generic_visit trap The single most common bug in both classes is forgetting `self.generic_visit(node)`. Once you define `visit_FunctionDef`, *your method owns that subtree*: nothing below it is visited unless you descend yourself. A rule that flags nested calls will silently report only the outermost one; a transformer will rewrite the top of a nested structure and leave the inside untouched. In a transformer, call `self.generic_visit(node)` **before** you build the replacement if you want the children rewritten first (bottom-up), and remember it returns the node with its children already processed. Related: visiting is single-pass, so an edit that creates a node your own visitor would have matched is *not* revisited unless you run the transformer again over the result. Codemods that must reach a fixed point loop until the tree stops changing. ## Choosing between them Use a visitor when the output is information - findings, a symbol table, a call graph, metrics. Use a transformer when the output is a tree you will hand to `compile()` or `ast.unparse()`. Wanting both usually means two passes: collect first, decide, then rewrite, which is easier to reason about than deciding and editing in one traversal. A detail worth carrying: a rewritten tree is not ready to compile. New nodes you construct have no position fields, and `compile()` rejects that with a `TypeError` until you run `ast.fix_missing_locations()` or `ast.copy_location()` over the result. ## Node types are grammar, and grammar changes Because dispatch is by class name, a visitor method whose name no longer matches any node class simply never fires - it does not raise. Every literal has been an `ast.Constant` since Python 3.8, so handlers named for the old per-literal node classes are dead code on a modern interpreter. In the other direction, `visit_Match` is meaningless before 3.10 and essential after it if your rule must see pattern-matching code. Test that a visitor actually fires; a silent zero is indistinguishable from a clean file. ## Testing a pass Because both classes fail silently - a handler that never fires, a subtree never entered, a node quietly deleted - the test for an AST pass is not "it did not crash". Assert on the collected findings for a fixture that contains a nested, an aliased and a negative case, and for a transformer assert on `ast.unparse()` of the result so the shape of the rewrite is visible in the test. A pass that returns zero findings should be indistinguishable in your suite from one that was never called; if it is not, you have no evidence the rule works.