Repository navigation
fix(extract): resolve module.func values in Python dispatch tables (#1566) - #4773
nothariharan wants to merge 1 commit into
Conversation
|
Thanks for the pull request, @nothariharan. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
There was a problem hiding this comment.
Graphify reviewed this change.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; formal verification proved 0 of 2 changed function(s) (2 not verified).
Formal verification. PR-changed functions: 0/2 verified (0 proven, 0 may-equivalent, 0 distinguished) · 2 not verified (1 vacuous, 1 unsupported).
Not verified on this run: \_resolve\_python\_member\_calls (vacuous: never exercised), \_extract\_generic (unsupported).
Graphify review — findings
Adds indirect-call resolution for Python dispatch tables whose values are written as module.func (e.g. {"card": card.card_charge}), so these now produce INFERRED indirect_call edges to the imported module's function instead of being dropped. Resolution goes through the imported module only, targets real callables rather than classes used as values, and runs after direct calls so a real calls edge to the same target wins; an existing imports edge no longer hides the dispatch edge. Bumps _AST_CACHE_SCHEMA to 26, so cached Python extractions re-run once.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 4080 functions depend on the 809 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 1016 callers, 62 callees - worse:
walk_calls()— 1 callers, 37 callees - worse:
_scan_module_dispatch()— 1 callers, 7 callees
Verification — 4080 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 3829 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
238 of 454 test file(s) selected (52%) via static blast radius.
tests/test_affected_overrides.py— impacttests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_binary_media_words.py— impacttests/test_blade_extractor.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_c_include_resolution.py— impacttests/test_cache.py— impacttests/test_cache_long_paths.py— impacttests/test_cache_stale_import_target.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_charmap_encoding.py— impacttests/test_chunking.py— impacttests/test_cjs_module_extension.py— impacttests/test_cobol_extractor.py— impacttests/test_cpp_method_declarations.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cpp_template_type_params.py— impacttests/test_cpp_type_alias.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_external_call_guards.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_delegate_declarations.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_foreach_loop_variable.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_chain_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_overloads.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_tuple_type_refs.py— impacttests/test_csharp_type_resolution.py— impacttests/test_csharp_underscore_collisions.py— impacttests/test_csharp_var_call_return_type.py— impacttests/test_csharp_verbatim_interpolation.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_ejs_extractor.py— impacttests/test_elixir_defdelegate.py— impacttests/test_elixir_import_resolution.py— impact- … and 188 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_resolve\_python\_member\_calls.
The verifier did not have enough to check \_resolve\_python\_member\_calls, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: parameter `config` is annotated `LanguageConfig` — outside the synthesizable primitive/collection set
· 3 more finding(s) on lines outside this diff (see the check run).
…raphify-Labs#1566) A dict/list/set/tuple literal of bare function names already yields `indirect_call` edges, but the same table written with the module prefix, the usual shape of a plugin registry: from payments import card, cash PROVIDERS = {"card": card.card_charge, "cash": cash.cash_charge} got no edge at all, because only bare identifiers counted as values. Such a value now becomes an `indirect` member raw call, and the Python member-call resolver binds it through its module arm only: the receiver must be a module imported into the caller's own file, holding exactly one callable of that name. It is emitted as the INFERRED `indirect_call` (0.85, context `collection`) the bare-name path uses, never to a class used as a value, and never when a parameter or local shadows the module name. A module-level table resolves from the file node. The AST cache schema goes to 26 so cached Python extractions re-run.
e4d2306 to
28934b6
Compare
What does this PR do?
Part of #1566. A dispatch table of bare function names already gets
indirect_calledges. The same table written with the module prefix, the usual shape of a plugin registry, got no edge at all, because only bare identifiers counted as table values:On v8 neither
pay()norcheckout.pylinks tocard_chargeorcash_charge. This came up in a user report about a plugin-style payments module.How it resolves. A
module.funcvalue in a dict, list, set or tuple literal becomes anindirectmember raw call. The existing Python member-call resolver then binds it, through its module arm only (#1883): the receiver must be a module imported into the caller's own file, and that module must hold exactly one callable of that name. The edge is the same INFERREDindirect_call(0.85, contextcollection) that the bare-name path emits.It does not guess:
card.CardGateway, indirect_call callable guard admits classes → false edges for select(Model), except tuples, and getattr literals #2137).card.card_charge()call to the same target keeps the precisecallsedge, whether it comes before or after the table. Indirect raw calls resolve after direct ones.from payments.card import card_charge(animportsedge to the same symbol) does not hide the edge. Indirect edges only dedup againstcalls/indirect_callpairs, as the generic resolver'scall_like_pairsdoes.A module-level table resolves from the file node.
_file_node()returnsNonefor a file node, so the module arm treats an indirect caller with no parent and with import edges as the file itself. That special case covers onlyindirectraw calls. Direct module-level calls (card.card_charge(1)at top level) still get no edge, as on v8. Changing that would add edges outside this PR's scope._AST_CACHE_SCHEMAgoes to 26 so cached Python extractions re-run.Type of change
Verification & Invariants
Invariant: an
indirect_callis emitted only to a callable the caller's own imports pin down to exactly one definition. Otherwise nothing is emitted.tests/test_indirect_dispatch_module_attr.py(10). On v8, the 4 positive tests fail: function-level table, module-level registry, same-name-in-two-modules, and the file-levelimportsedge. The 6 guard tests pass on both: class value, shadowing parameter, unimported module, direct call not duplicated, table-before-call ordering, and both spellings giving one edge.graphify update --no-cluster:tests/fixtures: 849 nodes / 970 edges, identical.graphify/package as a corpus: 4,787 nodes / 10,144 edges, identical. That code has manylocal.attrdict values (path.name,st.st_size,imported.local_name), and none gained an edge.pay()andcheckout.pytocard_charge/cash_charge), nothing removed.callsedge, and a file-levelimportsedge suppressed the indirect edge. Both have tests.Known pre-existing gaps, not changed here. The new path inherits these from the bare-name path's shared guard. Each one reproduces on v8 with bare names:
except ... as(or a module-levelwith) is not treated as shadowing, soexcept E as card_charge: [card_charge]still gets an edge.isinstancetuple counts as a table value.indirect_call, a later directmodule.func()call to the same target loses itscallsedge.module.funcpassed as a call argument, assigned, or returned (pool.submit(card.card_charge)) is still unsupported. Only table values are handled here.How was this tested?
Windows 11, Python 3.12.
Graphify-specific checklist