Fix cross-file inheritance and ghost type references in AST extractor - #1231
Fix cross-file inheritance and ghost type references in AST extractor#1231MananJain39 wants to merge 2 commits into
Conversation
superclass references as SymbolUseFacts
safishamsi
left a comment
There was a problem hiding this comment.
Review: Request Changes
Root issue: incomplete fix — ghost node/edge persists
The inherits direction targets a real bug (ambiguous cross-file base classes), but the fix is incomplete. It adds a parallel canonical edge via the deferred resolution pipeline while leaving the original ghost node (empty source_file) and ghost edge created at extract.py:2413-2423 in the graph. Because (source, relation) differs in target, nx.DiGraph in build.py:300 does not collapse them — both survive. Issue #1186's duplicate-node symptom is not resolved.
Required fix: suppress the eager bare-fallback at extract.py:2413-2423 for Python when the base is import-bound, rather than adding a parallel edge.
Real defect: fn_nid scoping bug
The #return type block (diff ~lines 55-65) is outside the if node.type == "function_definition" guard but references fn_nid, which is only defined inside it. For any non-function node with a return_type field this raises UnboundLocalError or silently attaches return-type refs to the wrong function using a stale value from the previous iteration.
Annotation half is redundant and uses wrong context
The existing path at extract.py:2978-2997 (using _python_collect_type_refs) already handles cross-file annotation edges correctly and is covered by test_python_import_resolution.py::test_python_parameter_return_and_generic_contexts. The PR's hand-rolled walker:
- Emits
context="annotation"which is not inREFERENCE_CONTEXTS(extract.py:139-141) - Skips
_python_type_ref_ok/_PY_STDLIB_BLOCKLISTchecks - Misses subscripted generics (
list[Payload],Optional[X]) that the recursive_python_collect_type_refshandles - Misses
typed_default_parameternode type
Recommend: drop the annotation half entirely and reuse the existing path, or route through _python_type_ref_ok with context="parameter_type"/"return_type".
No tests
No test exercises the ambiguous cross-file base scenario or confirms ghost node removal. Please add a test verifying: (1) the canonical inherits edge resolves to the correct imported class, and (2) no ghost node with empty source_file remains in the graph.
|
Thanks for the detailed review @safishamsi — you're right that the current change only adds the deferred canonical edge path without suppressing the original eager fallback emission, so the ghost node/edge can still survive in parallel. I'll rework the inheritance fix to suppress the eager fallback path for import-bound Python base references rather than layering an additional edge on top. You're also correct about the annotation half:
I'll drop the annotation portion and focus the PR on the missing cross-file inheritance resolution path only. I'll also add regression coverage for:
|
Summary
Fixes #1186
This PR fixes missing cross-file Python inheritance edges and unresolved type references by routing them through the deferred symbol-resolution pipeline instead of resolving them immediately during AST traversal.
Changes
class_definitionnodes as_SymbolUseFact(relation="inherits"). This restores properinheritsedges for project-internal cross-file bases instead of downgrading them to genericusesedges on import lines.function_definitionnodes as_SymbolUseFact(relation="references"). This prevents the creation of duplicate fallback ("ghost") nodes for imported types.Result
Cross-file references now successfully leverage the import-aware resolution post-pass, ensuring canonical edge generation and eliminating duplicate unresolved nodes.