mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-26 07:57:57 +00:00
fix(blocking-io): trace self/cls attribute chains and local aliases in the call graph (#4200)
* fix(blocking-io): trace self/cls attribute chains and local aliases in the call graph
_record_call_ref only recorded a call-graph edge for bare-name calls and
literal self./cls. single-hop attribute calls (self.flush()). Any other
receiver shape fell through the "." not in call_name fallback and was
silently dropped from the graph -- including a deeper self./cls.
attribute chain (self.store.flush()), a local variable holding a
self./cls. attribute (store = self.store; store.flush()), or a
parameter used directly as a receiver. A real blocking call reachable
from async code only through one of those shapes never surfaced as a
finding, the opposite (and more dangerous) failure mode from the
duplicate-helper-name over-report this detector already documents.
Trace those shapes back to a self./cls. attribute or a parameter,
within the same function only, and resolve them through the same
bare-method-name fallback already used for receivers that cannot be
resolved to a name at all -- no new false-positive risk beyond what
that existing fallback already accepts.
* fix(blocking-io): narrow alias tracking to fix three scope-creep bugs
The alias/receiver tracking this detector added reused dotted_name(),
which intentionally unwraps ast.Call/ast.Subscript for blocking-call
pattern matching elsewhere in this module. Reusing it for alias
extraction let a Call or Subscript result inherit its base's
alias-worthiness, so factory().flush(), client = factory(); client.flush(),
and client = clients[0]; client.flush() were all incorrectly treated as
calls on a traced receiver. Add _simple_receiver_name(), a restricted
Name/Attribute-only extractor, and use it wherever a receiver/alias is
extracted instead of dotted_name().
Alias state also only ever grew: _record_local_receiver_alias_targets
never removed a name once traced, so a later reassignment to a
non-traceable value (client = NonBlockingClient()) left the name
aliased forever, still exposing unrelated same-named methods.
Reassigning now resolves traceability from scratch and kills the name
when the new value isn't traceable.
Separately, if/else branches had no isolation: with no visit_If
override, body and orelse shared one mutable alias set, so an alias
added in one leaked into the other and the result depended on which
branch was textually first. Add a visit_If override that snapshots
aliases before the branch, resets between body and orelse, and unions
their exit states afterwards -- a conservative, order-independent
may-alias join. Scoped to ast.If only; ast.Try/ast.Match keep the
previous unisolated traversal (different, more complex control-flow
semantics, out of scope here).
Finally, _visit_function pushed the new function's context before
visiting decorator_list/args/returns, but those expressions run at
definition time in the enclosing scope, not the function body. A
default value referencing an outer name that happens to match one of
the function's own parameter names (receiver = Store(); async def
route(receiver=receiver.flush())) was misattributed to route itself.
Visit decorators, parameter defaults/annotations, the return
annotation, and PEP 695 type-parameter bounds before pushing the new
function's context so they resolve against the enclosing scope.
Real-scanner output against the actual backend tree is unchanged
(41/41 findings, byte-identical JSON) -- these were latent
false-positive/negative risks in shapes the current codebase doesn't
happen to contain, not active miscounts.
* fix(blocking-io): fix three more alias-tracking and definition-time bugs
_record_local_receiver_alias_targets ran before the assignment's own
value was visited, so an assignment's RHS was analyzed against the
alias state *after* the target had already been updated/killed for
this same statement. Python evaluates the RHS before binding the
target: with `client = self.store` followed by
`client = client.flush()`, the second statement's target update killed
`client`'s alias before its own RHS (`client.flush()`) was visited, so
that call silently disappeared from the graph. visit_Assign and
visit_AnnAssign now visit the RHS first and only update the target's
alias afterward, matching Python's own evaluate-then-bind order.
_simple_receiver_name still returned the trailing attribute name
whenever its recursive parent lookup came back unsupported (a Call or
Subscript), instead of refusing the whole chain -- so `factory().client`
and `clients[0].client` both collapsed to plain "client", which, when
"client" was also a traced parameter or local alias, incorrectly linked
`factory().client.flush()` to an unrelated same-file `Store.flush`.
Return None instead of falling back to `node.attr`, so an unsupported
node anywhere in the chain makes the whole receiver unresolved rather
than a truncated suffix of it.
Finally, _visit_function's enclosing-scope walk of decorators,
defaults, annotations, and type_params recursed into every
subexpression uniformly, including ones that don't actually execute at
definition time: a lambda's body, a bare generator expression's
element/later-for clauses, annotations postponed by `from __future__
import annotations`, and PEP 695 type-parameter bounds (always
evaluated lazily, in their own hidden function, only if something like
T.__bound__ is ever accessed). Add visit_Lambda/visit_GeneratorExp
overrides that stop at exactly the eager subset (a lambda's own
parameter defaults; a generator's outermost iterable), skip parameter/
return annotations entirely once a `postponed_annotations` flag is set
by the future import, and drop the type_params walk instead of moving
it to the enclosing scope.
Real-scanner output against the actual backend tree is unchanged
(41/41 findings, byte-identical JSON) -- these were latent risks in
shapes the current codebase doesn't happen to contain, not active
miscounts.
* fix(blocking-io): preserve eager traversal for immediately invoked lambdas and consumed generators
visit_Lambda/visit_GeneratorExp (added last round to stop treating merely
created lambda/generator objects as executing at definition time) were
unconditional, so they also suppressed bodies that genuinely execute right
away: an immediately invoked lambda ((lambda: ...)()) and a generator
expression passed directly to an eager-consuming builtin (list/set/tuple/
frozenset/dict/sorted).
visit_Call now marks a Lambda used as its own func, or a GeneratorExp passed
as the sole argument to one of those builtins, by node identity before
generic_visit runs. visit_Lambda/visit_GeneratorExp check that marker and,
on a match, visit the node fully instead of applying the lazy walk. A
lambda/generator that is merely created, stored, passed as a callback, or
invoked later through a variable is unaffected and stays lazy.
* fix(blocking-io): scope lambda/generator laziness to definition-time expressions only
visit_Lambda/visit_GeneratorExp were unconditional overrides, so they
suppressed lambda bodies and generator elements everywhere the visitor
reached one, not only inside another function's definition-time
expressions (decorators, parameter defaults/annotations, return
annotation) where that suppression is actually needed. In ordinary
function-body code this caused real false negatives: a lambda stored in
a local and called through that name (callback = lambda: os.listdir(".");
callback()), a generator reduced by sum/any/all/min/max, a bare
lambda/generator that is merely created, and a generator wrapped in
another lazy iterator like map(...) all went unscanned, even though none
of them are definition-time expressions at all.
The previous fix for this (an id()-keyed marker set covering exactly two
eager shapes -- an immediately invoked lambda, and a generator passed
directly to a fixed list of eager-consuming builtins) narrowed the
suppression back down, but only for those two shapes, and the underlying
eager-consumer builtin set itself excluded true reducers (sum/any/all/
min/max) that consume their generator argument just as eagerly as list/
set/etc. Both are instances of the same problem: enumerating every shape
in which a lambda or generator happens to be invoked/consumed piecemeal
inside an AST visitor, which is unbounded in the general case.
Replace both mechanisms with a single boolean,
_in_definition_time_expression, set only while _visit_function walks
another function's own decorators/defaults/annotations/return
annotation. visit_Lambda/visit_GeneratorExp apply their lazy
(defaults-only/outermost-iterable-only) walk only while it is set;
everywhere else they fall through to a full generic_visit, scanning
lambda bodies and generator elements unconditionally -- the same
conservative, over-report-rather-than-infer stance this file already
takes for reachability elsewhere.
This removes EAGER_ITERABLE_CONSUMER_NAMES and the two identity-marker
sets entirely rather than growing them further. The one shape this
newly gives up on -- an immediately invoked lambda or eagerly consumed
generator used as another function's decorator/default/annotation value
-- is now an explicit, narrow, documented limitation (see
backend/AGENTS.md): definition-time expressions never scan a nested
lambda body or generator element, full stop, regardless of whether it
happens to be invoked right there.
Targeted suite (test_detect_blocking_io_static.py +
test_scan_changed_blocking_io.py + test_detector_repo_root.py +
blocking_io/test_gate_smoke.py): 65/66 pass, the one failure a
pre-existing Windows path-separator comparison unrelated to this file.
Full backend suite: identical 64 pre-existing failures on both the
pre-fix and post-fix commit, confirmed by diffing the two failure lists
directly -- zero regressions. Real scanner against the actual backend
tree: 41/41, byte-identical JSON before and after -- these were latent
risks in shapes the current codebase doesn't happen to contain, not
active miscounts.
This commit is contained in:
parent
20debf9cc7
commit
40c4ec32f4
@ -111,8 +111,83 @@ detect-blocking-io` resolve to the same repo-root path). JSON findings include
|
||||
`code` for model-assisted or manual review. `priority` is a deterministic
|
||||
review ordering from operation type, not proof of a bug. Bare-name same-file
|
||||
calls are resolved by function name, so duplicate helper names in one file can
|
||||
conservatively over-report async reachability. It is intentionally
|
||||
informational and is not run from CI in this round.
|
||||
conservatively over-report async reachability. The call graph also resolves
|
||||
multi-hop `self.`/`cls.` attribute chains (`self.store.flush()`) and local
|
||||
variables or parameters traced back — within the same function only — to a
|
||||
`self.`/`cls.` attribute (`store = self.store; store.flush()`); both fall back
|
||||
to the same bare-method-name resolution as an unresolvable receiver, so they
|
||||
share its over-report risk rather than adding a new kind. Deeper cross-function
|
||||
or cross-module aliasing is out of scope and stays an unreported false
|
||||
negative.
|
||||
|
||||
That same-function alias tracing is deliberately narrower than the symbolic
|
||||
names `dotted_name()` builds for blocking-call pattern matching elsewhere in
|
||||
this module: receiver/alias extraction uses a restricted extractor that only
|
||||
recognizes `Name`/`Attribute` chains, so a `Call` or `Subscript` result (e.g.
|
||||
`factory().flush()`, or `client = factory(); client.flush()` /
|
||||
`client = clients[0]; client.flush()`) is never treated as inheriting its
|
||||
base's alias-worthiness — including when the unsupported node is buried
|
||||
deeper in the chain (`factory().client.flush()`, `clients[0].client.flush()`):
|
||||
an unrecognized shape anywhere in the chain makes the whole receiver
|
||||
unresolved, it never falls back to just the chain's trailing attribute name,
|
||||
or that name alone could still collide with an unrelated traced parameter or
|
||||
local alias. Reassigning a traced name to a non-traceable value (anything
|
||||
other than a `self.`/`cls.` attribute or an already-traced name) kills its
|
||||
alias instead of leaving it traceable, so a stale alias from an earlier
|
||||
assignment cannot keep exposing an unrelated same-named method after the
|
||||
variable is reassigned to something else; the assignment's right-hand side is
|
||||
always analyzed against the alias state as it stood *before* this kill-or-add
|
||||
update, matching Python's own evaluate-then-bind order, so
|
||||
`client = client.flush()` still resolves that call against `client`'s prior
|
||||
(pre-reassignment) alias instead of the state after it's gone. `if`/`else`
|
||||
branches get isolated alias state — an alias added in one branch cannot leak
|
||||
into the other — and the state after the whole `if` is the union of what each
|
||||
branch produced (a conservative may-alias join), so the result no longer
|
||||
depends on which branch is textually `body` vs. `orelse`. This branch
|
||||
isolation is deliberately scoped to `ast.If` only; `ast.Try`/`ast.Match` have
|
||||
different, more complex control-flow semantics and keep the older unisolated
|
||||
traversal. Finally, a function's decorators and parameter defaults are
|
||||
analyzed in the *enclosing* scope rather than the new function's own, and
|
||||
parameter/return annotations get the same enclosing-scope treatment unless
|
||||
the module postpones annotation evaluation (`from __future__ import
|
||||
annotations`), in which case they are skipped entirely, in either scope —
|
||||
those expressions run at definition time, before the function has ever been
|
||||
called (or, when postponed, never run at all), so a call there is never
|
||||
attributed to the function being defined (it moves to whatever scope actually
|
||||
contains the `def`, e.g. the enclosing function, or disappears if that scope
|
||||
is module/class level and therefore never async-reachable). PEP 695
|
||||
type-parameter bounds are not visited in either scope: CPython evaluates each
|
||||
one lazily, in its own hidden function, only if something like `T.__bound__`
|
||||
is actually accessed, never as part of running the `def` statement itself.
|
||||
A `lambda`'s body and a bare generator expression's element/filters/later
|
||||
`for` clauses are excluded from traversal ONLY while walking another
|
||||
function's own definition-time expressions (decorators, parameter defaults/
|
||||
annotations, return annotation): there, we know structurally that the
|
||||
enclosing `def` statement is executing right now, and neither a lambda body
|
||||
nor a generator's element runs just because the lambda/generator object is
|
||||
created — only a lambda's own parameter defaults and a generator's
|
||||
outermost iterable are genuinely eager at that moment. This exclusion is
|
||||
absolute and has no exceptions: even a lambda that is immediately invoked at
|
||||
its own definition site (`(lambda: ...)()`), or a generator passed directly
|
||||
to an eager-consuming builtin, is still excluded when it appears inside
|
||||
another function's decorator/default/annotation — a narrow, intentional
|
||||
limitation given how rarely a definition-time expression contains an
|
||||
executed call at all, preferred over special-casing specific shapes there.
|
||||
|
||||
Everywhere else — module level, class bodies, and ordinary function-body
|
||||
statements — a lambda body or generator expression's element is scanned
|
||||
unconditionally, the same conservative, over-report-rather-than-infer stance
|
||||
this file already takes for reachability elsewhere (the `ast.If` may-alias
|
||||
union, the bare-name call-graph resolution). This file does not attempt to
|
||||
distinguish a lambda that is invoked immediately, invoked later through a
|
||||
stored variable, passed as a callback, or never called at all, nor a
|
||||
generator that is consumed by an eager builtin (`list`, `sum`, `any`, etc.),
|
||||
wrapped in another lazy iterator (`map`, `filter`), or never consumed —
|
||||
telling these apart in the general case would mean inferring evaluation
|
||||
order and consumption across arbitrary code rather than reading a fixed,
|
||||
structural fact, so none of them are special-cased; all are scanned the
|
||||
same way. This is intentionally informational and is not run from CI in
|
||||
this round.
|
||||
|
||||
For a diff-scoped view of the same findings, `scripts/scan_changed_blocking_io.py`
|
||||
(repo root) reports findings on the added lines of `git diff <base>...HEAD`
|
||||
|
||||
@ -293,6 +293,32 @@ def dotted_name(node: ast.AST | None) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
def _simple_receiver_name(node: ast.AST | None) -> str | None:
|
||||
"""Like `dotted_name`, but only for Name/Attribute chains.
|
||||
|
||||
`dotted_name` intentionally unwraps `ast.Call` and `ast.Subscript` to build a
|
||||
symbolic name for blocking-call pattern matching (`visit_Call` /
|
||||
`_blocking_rule` / `_sync_http_client_factory_base`). Reusing that for
|
||||
receiver/alias tracking is wrong: it would make a Call or Subscript result
|
||||
inherit its base's alias-worthiness (e.g. treating `factory()` as if it
|
||||
were the traced name `factory`), which this restricted extractor refuses
|
||||
to do by simply not recognizing those node shapes at all -- including when
|
||||
one is buried further down the chain (`factory().client`,
|
||||
`clients[0].client`): an unsupported node anywhere in the chain makes the
|
||||
whole receiver None, it never falls back to just the trailing attribute
|
||||
name, or that trailing name alone could still collide with an unrelated
|
||||
traced parameter or local alias of the same name.
|
||||
"""
|
||||
if isinstance(node, ast.Name):
|
||||
return node.id
|
||||
if isinstance(node, ast.Attribute):
|
||||
parent = _simple_receiver_name(node.value)
|
||||
if parent is None:
|
||||
return None
|
||||
return f"{parent}.{node.attr}"
|
||||
return None
|
||||
|
||||
|
||||
def relative_to_repo(path: Path, repo_root: Path = REPO_ROOT) -> str:
|
||||
try:
|
||||
return path.resolve().relative_to(repo_root.resolve()).as_posix()
|
||||
@ -314,6 +340,7 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
self.relative_path = relative_path
|
||||
self.source_lines = source_lines
|
||||
self.import_aliases: dict[str, str] = {}
|
||||
self.postponed_annotations = False
|
||||
self.class_stack: list[str] = []
|
||||
self.function_stack: list[_FunctionContext] = []
|
||||
self.module_context = _FunctionContext("<module>", None, False)
|
||||
@ -325,7 +352,19 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
self.functions_by_name: dict[str, list[str]] = defaultdict(list)
|
||||
self.call_refs: dict[str, list[_CallRef]] = defaultdict(list)
|
||||
self.path_like_name_stack: list[set[str]] = []
|
||||
self.local_receiver_alias_stack: list[set[str]] = []
|
||||
self.potential_findings: list[_PotentialFinding] = []
|
||||
# True only while walking ANOTHER function's own decorators,
|
||||
# parameter defaults/annotations, or return annotation (see
|
||||
# `_visit_function`) -- expressions that run at definition time, in
|
||||
# the enclosing scope, before the function being defined has ever
|
||||
# been called. `visit_Lambda`/`visit_GeneratorExp` apply their
|
||||
# defaults-only/outermost-iterable-only lazy traversal only while
|
||||
# this is set; it is always restored to `False` before `_visit_function`
|
||||
# returns, and no definition-time expression can itself contain a
|
||||
# nested `def`/`class` statement (only expressions), so this never
|
||||
# needs to be a stack.
|
||||
self._in_definition_time_expression = False
|
||||
|
||||
@property
|
||||
def current_function(self) -> _FunctionContext | None:
|
||||
@ -348,6 +387,15 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
|
||||
if node.module is None:
|
||||
return
|
||||
if node.module == "__future__" and any(alias.name == "annotations" for alias in node.names):
|
||||
# `from __future__ import annotations` (PEP 563) makes CPython skip
|
||||
# evaluating parameter/return annotations at runtime entirely --
|
||||
# they are kept as unevaluated strings -- so a call written in one
|
||||
# never actually runs at definition time in this module, in either
|
||||
# scope. This is always visited before any function def that could
|
||||
# use it: the statement is required to appear before any other
|
||||
# code in the file (`ast.parse` itself rejects it elsewhere).
|
||||
self.postponed_annotations = True
|
||||
for alias in node.names:
|
||||
local_name = alias.asname or alias.name
|
||||
self.import_aliases[local_name] = f"{node.module}.{alias.name}"
|
||||
@ -366,14 +414,28 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
self._visit_function(node, is_async=True)
|
||||
|
||||
def visit_Assign(self, node: ast.Assign) -> None:
|
||||
# Visit the RHS before recording anything about the assignment's own
|
||||
# target(s): Python evaluates the value before binding it, so a call
|
||||
# in the RHS (e.g. `client = client.flush()`) must see the receiver-
|
||||
# alias state as it stood immediately BEFORE this assignment, not
|
||||
# after. Updating/killing the target's alias first would make the
|
||||
# target's own old alias disappear before the RHS that still runs
|
||||
# under it gets a chance to be resolved.
|
||||
self.visit(node.value)
|
||||
self._record_sync_http_client_targets(node.value, node.targets)
|
||||
self.generic_visit(node)
|
||||
self._record_local_receiver_alias_targets(node.value, node.targets)
|
||||
for target in node.targets:
|
||||
self.visit(target)
|
||||
|
||||
def visit_AnnAssign(self, node: ast.AnnAssign) -> None:
|
||||
self._record_path_like_annotation(node.annotation, [node.target])
|
||||
if node.value is not None:
|
||||
# Same RHS-before-target-update ordering as visit_Assign above.
|
||||
self.visit(node.value)
|
||||
self._record_sync_http_client_targets(node.value, [node.target])
|
||||
self.generic_visit(node)
|
||||
self._record_local_receiver_alias_targets(node.value, [node.target])
|
||||
self.visit(node.target)
|
||||
self.visit(node.annotation)
|
||||
|
||||
def visit_With(self, node: ast.With) -> None:
|
||||
temporary_clients: dict[str, str | None] = {}
|
||||
@ -397,6 +459,33 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
else:
|
||||
current_clients[name] = previous
|
||||
|
||||
def visit_If(self, node: ast.If) -> None:
|
||||
# `ast.If` is the only branching construct given isolated, merged alias
|
||||
# state: `body` and `orelse` are mutually exclusive at runtime, so an
|
||||
# alias added in one must not leak into the other, but a name aliased in
|
||||
# either branch might still be aliased after the `if` (a conservative
|
||||
# may-alias join) -- and the result must not depend on which branch is
|
||||
# textually `body` vs `orelse`. `ast.Try`/`ast.Match` have different,
|
||||
# more complex control-flow semantics (exception edges, multiple
|
||||
# mutually exclusive case bodies) and are deliberately out of scope
|
||||
# here; they keep the prior unisolated `generic_visit` behavior.
|
||||
self.visit(node.test)
|
||||
if not self.local_receiver_alias_stack:
|
||||
for statement in node.body:
|
||||
self.visit(statement)
|
||||
for statement in node.orelse:
|
||||
self.visit(statement)
|
||||
return
|
||||
before = set(self.current_local_receiver_aliases)
|
||||
for statement in node.body:
|
||||
self.visit(statement)
|
||||
after_body = set(self.current_local_receiver_aliases)
|
||||
self.local_receiver_alias_stack[-1] = set(before)
|
||||
for statement in node.orelse:
|
||||
self.visit(statement)
|
||||
after_orelse = self.current_local_receiver_aliases
|
||||
self.local_receiver_alias_stack[-1] = after_body | after_orelse
|
||||
|
||||
def visit_Call(self, node: ast.Call) -> None:
|
||||
current = self.current_context
|
||||
call_name = self._canonical_name(dotted_name(node.func))
|
||||
@ -405,6 +494,58 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
self._record_blocking_candidate(node, call_name, current)
|
||||
self.generic_visit(node)
|
||||
|
||||
def visit_Lambda(self, node: ast.Lambda) -> None:
|
||||
# A lambda's parameter defaults run eagerly when the lambda object
|
||||
# itself is created (same timing as a regular function's defaults),
|
||||
# but its body only runs later, whenever/if the lambda is actually
|
||||
# called -- possibly in a completely different scope, possibly
|
||||
# never. That distinction is only meaningful while walking ANOTHER
|
||||
# function's definition-time expressions (see `_visit_function`):
|
||||
# there, we know structurally that the enclosing `def` statement is
|
||||
# executing right now, and a nested lambda's body is categorically
|
||||
# not part of that execution, no matter how (or whether) the lambda
|
||||
# is later used. Everywhere else -- module level, class bodies, and
|
||||
# ordinary function-body statements -- a lambda is scanned
|
||||
# unconditionally, like any other expression: this file does not
|
||||
# attempt to prove whether/when a lambda sitting in a variable,
|
||||
# passed as a callback, invoked immediately, or invoked later through
|
||||
# a stored name is actually called. That is the same conservative,
|
||||
# over-report-rather-than-infer stance this file already takes for
|
||||
# reachability elsewhere (e.g. `visit_If`'s may-alias union, or the
|
||||
# bare-name call-graph resolution in `_record_call_ref`).
|
||||
if not self._in_definition_time_expression:
|
||||
self.generic_visit(node)
|
||||
return
|
||||
for default in node.args.defaults:
|
||||
self.visit(default)
|
||||
for default in node.args.kw_defaults:
|
||||
if default is not None:
|
||||
self.visit(default)
|
||||
|
||||
def visit_GeneratorExp(self, node: ast.GeneratorExp) -> None:
|
||||
# Same principle as `visit_Lambda` above: only the outermost `for`'s
|
||||
# iterable is evaluated eagerly, to build the generator object; the
|
||||
# element expression, any `if` filters, and any additional `for`
|
||||
# clauses live inside the generator's own frame and only run once/if
|
||||
# it is iterated -- which may never happen. This is only meaningful
|
||||
# while walking another function's definition-time expressions;
|
||||
# everywhere else a generator expression is scanned unconditionally,
|
||||
# regardless of what (if anything) it is later passed to. This file
|
||||
# does not distinguish a builtin that consumes its argument eagerly
|
||||
# (`list(...)`, `sum(...)`) from one that wraps it in another lazy
|
||||
# iterator (`map(...)`) or leaves it unconsumed in a variable --
|
||||
# telling those apart in general means inferring evaluation order
|
||||
# across arbitrary code, not reading a fixed, structural fact. List/
|
||||
# set/dict comprehensions always run their implicit scope immediately
|
||||
# as part of building the result, regardless of context, so they are
|
||||
# left fully eager unconditionally and are not given a matching
|
||||
# override.
|
||||
if not self._in_definition_time_expression:
|
||||
self.generic_visit(node)
|
||||
return
|
||||
if node.generators:
|
||||
self.visit(node.generators[0].iter)
|
||||
|
||||
def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef, *, is_async: bool) -> None:
|
||||
qualname = ".".join((*self.class_stack, node.name)) if self.class_stack else node.name
|
||||
class_name = self.class_stack[-1] if self.class_stack else None
|
||||
@ -413,14 +554,57 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
self.functions_by_name[node.name].append(qualname)
|
||||
if class_name is not None:
|
||||
self.class_methods[class_name].add(node.name)
|
||||
|
||||
# Decorators and parameter defaults run at definition time in the
|
||||
# ENCLOSING scope, not inside the function body -- visit them before
|
||||
# pushing this function's own context. Otherwise a call/receiver there
|
||||
# (e.g. a default value referencing an outer variable that happens to
|
||||
# share a parameter's name) gets misattributed to the function being
|
||||
# defined instead of to whatever actually executes it. Parameter and
|
||||
# return annotations get the same enclosing-scope treatment unless
|
||||
# this module postpones annotation evaluation (`from __future__
|
||||
# import annotations`), in which case they never execute at runtime at
|
||||
# all, in either scope, so they are skipped entirely. PEP 695
|
||||
# type-parameter bounds (`node.type_params`) are not visited at all,
|
||||
# in either scope: CPython evaluates each one lazily, in its own
|
||||
# hidden function, only when something like `T.__bound__` is actually
|
||||
# accessed -- never as part of running this `def` statement.
|
||||
# `_in_definition_time_expression` marks this whole traversal so
|
||||
# `visit_Lambda`/`visit_GeneratorExp` know a lambda body or generator
|
||||
# element reached here is not eager either -- purely because of
|
||||
# where it sits, not because of its own shape. It is always restored
|
||||
# to `False` below before this function's own body is visited.
|
||||
self._in_definition_time_expression = True
|
||||
for decorator in node.decorator_list:
|
||||
self.visit(decorator)
|
||||
self._visit_definition_time_arguments(node.args)
|
||||
if node.returns is not None and not self.postponed_annotations:
|
||||
self.visit(node.returns)
|
||||
self._in_definition_time_expression = False
|
||||
|
||||
self.function_stack.append(context)
|
||||
self.sync_http_client_stack.append({})
|
||||
self.path_like_name_stack.append(set(_path_like_argument_names(node.args, self._canonical_name)))
|
||||
self.generic_visit(node)
|
||||
self.local_receiver_alias_stack.append(set(_all_argument_names(node.args)))
|
||||
for statement in node.body:
|
||||
self.visit(statement)
|
||||
self.local_receiver_alias_stack.pop()
|
||||
self.path_like_name_stack.pop()
|
||||
self.sync_http_client_stack.pop()
|
||||
self.function_stack.pop()
|
||||
|
||||
def _visit_definition_time_arguments(self, arguments: ast.arguments) -> None:
|
||||
for default in arguments.defaults:
|
||||
self.visit(default)
|
||||
for default in arguments.kw_defaults:
|
||||
if default is not None:
|
||||
self.visit(default)
|
||||
if self.postponed_annotations:
|
||||
return
|
||||
for argument in _iter_arguments(arguments):
|
||||
if argument.annotation is not None:
|
||||
self.visit(argument.annotation)
|
||||
|
||||
def _canonical_name(self, name: str | None) -> str | None:
|
||||
if name is None:
|
||||
return None
|
||||
@ -437,15 +621,36 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
return
|
||||
if not isinstance(node.func, ast.Attribute):
|
||||
return
|
||||
receiver = dotted_name(node.func.value)
|
||||
receiver = _simple_receiver_name(node.func.value)
|
||||
if receiver in {"self", "cls"}:
|
||||
self.call_refs[current.qualname].append(_CallRef(node.func.attr, current.class_name, self_method=True))
|
||||
return
|
||||
if self._is_traceable_same_function_receiver(receiver):
|
||||
# Multi-hop self./cls. attribute chains (self.store.flush()) and local
|
||||
# variables/parameters traced back -- within this same function only --
|
||||
# to a self./cls. attribute or a parameter (store = self.store;
|
||||
# store.flush()) cannot be resolved to a specific class without full
|
||||
# type inference. Fall back to the same conservative same-file,
|
||||
# bare-method-name resolution already used below for receivers that
|
||||
# cannot be resolved to a name at all, rather than dropping the edge.
|
||||
self.call_refs[current.qualname].append(_CallRef(node.func.attr, current.class_name, self_method=False))
|
||||
return
|
||||
# Keep same-module direct calls through canonical aliases out of the call graph.
|
||||
# External calls are handled as blocking candidates instead.
|
||||
if "." not in call_name:
|
||||
self.call_refs[current.qualname].append(_CallRef(call_name, current.class_name, self_method=False))
|
||||
|
||||
def _is_traceable_same_function_receiver(self, receiver: str | None) -> bool:
|
||||
# True when `receiver` is a self./cls.-rooted attribute chain, or a name
|
||||
# traced -- within the current function only -- back to one of those or
|
||||
# to a parameter. Deliberately no cross-function/cross-module alias or
|
||||
# type inference; see `local_receiver_alias_stack` and
|
||||
# `_record_local_receiver_alias_targets`.
|
||||
if receiver is None:
|
||||
return False
|
||||
root = receiver.split(".", 1)[0]
|
||||
return root in {"self", "cls"} or root in self.current_local_receiver_aliases
|
||||
|
||||
def _record_blocking_candidate(self, node: ast.Call, call_name: str, current: _FunctionContext) -> None:
|
||||
rule = self._blocking_rule(node, call_name)
|
||||
if rule is None:
|
||||
@ -512,6 +717,10 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
def current_path_like_names(self) -> set[str]:
|
||||
return self.path_like_name_stack[-1] if self.path_like_name_stack else set()
|
||||
|
||||
@property
|
||||
def current_local_receiver_aliases(self) -> set[str]:
|
||||
return self.local_receiver_alias_stack[-1] if self.local_receiver_alias_stack else set()
|
||||
|
||||
def _record_path_like_annotation(self, annotation: ast.AST, targets: Iterable[ast.AST]) -> None:
|
||||
if not self.path_like_name_stack or not _is_path_annotation(annotation, self._canonical_name):
|
||||
return
|
||||
@ -526,6 +735,29 @@ class BlockingIOStaticVisitor(ast.NodeVisitor):
|
||||
for name in _iter_assigned_names(target):
|
||||
current_clients[name] = client_base
|
||||
|
||||
def _record_local_receiver_alias_targets(self, value: ast.AST, targets: Iterable[ast.AST]) -> None:
|
||||
# Every assignment to a name previously in `current_local_receiver_aliases`
|
||||
# must resolve that name's traceability from scratch -- not only add new
|
||||
# traceable names -- or a stale alias from an earlier, unrelated value
|
||||
# would keep exposing same-named blocking methods (dead code below this
|
||||
# point) forever. A non-traceable value (an unrecognized shape, or a
|
||||
# Call/Subscript result -- see `_simple_receiver_name`) therefore kills
|
||||
# the name instead of leaving it untouched.
|
||||
if not self.local_receiver_alias_stack:
|
||||
return
|
||||
current_aliases = self.current_local_receiver_aliases
|
||||
assigned_names = [name for target in targets for name in _iter_assigned_names(target)]
|
||||
if not assigned_names:
|
||||
return
|
||||
dotted = _simple_receiver_name(value)
|
||||
root = dotted.split(".", 1)[0] if dotted is not None else None
|
||||
is_traceable = root is not None and (root in {"self", "cls"} or root in current_aliases)
|
||||
for name in assigned_names:
|
||||
if is_traceable:
|
||||
current_aliases.add(name)
|
||||
else:
|
||||
current_aliases.discard(name)
|
||||
|
||||
def _sync_http_client_factory_base(self, node: ast.AST) -> str | None:
|
||||
if not isinstance(node, ast.Call):
|
||||
return None
|
||||
@ -573,17 +805,30 @@ def _is_path_annotation(annotation: ast.AST | None, canonical_name: Callable[[st
|
||||
return False
|
||||
|
||||
|
||||
def _path_like_argument_names(arguments: ast.arguments, canonical_name: Callable[[str | None], str | None]) -> Iterable[str]:
|
||||
def _iter_arguments(arguments: ast.arguments) -> Iterable[ast.arg]:
|
||||
candidates = [*arguments.posonlyargs, *arguments.args, *arguments.kwonlyargs]
|
||||
if arguments.vararg is not None:
|
||||
candidates.append(arguments.vararg)
|
||||
if arguments.kwarg is not None:
|
||||
candidates.append(arguments.kwarg)
|
||||
for argument in candidates:
|
||||
yield from candidates
|
||||
|
||||
|
||||
def _path_like_argument_names(arguments: ast.arguments, canonical_name: Callable[[str | None], str | None]) -> Iterable[str]:
|
||||
for argument in _iter_arguments(arguments):
|
||||
if _is_path_annotation(argument.annotation, canonical_name):
|
||||
yield argument.arg
|
||||
|
||||
|
||||
def _all_argument_names(arguments: ast.arguments) -> Iterable[str]:
|
||||
# Every parameter name of a function, unfiltered by annotation -- used to
|
||||
# seed same-function receiver-alias tracing (see
|
||||
# `local_receiver_alias_stack`) so a parameter used directly as a call
|
||||
# receiver (e.g. a constructor-injected dependency) is traceable too.
|
||||
for argument in _iter_arguments(arguments):
|
||||
yield argument.arg
|
||||
|
||||
|
||||
def _iter_assigned_names(target: ast.AST) -> Iterable[str]:
|
||||
if isinstance(target, ast.Name):
|
||||
yield target.id
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Loading…
x
Reference in New Issue
Block a user