fix(registry): resolve aliased direct-symbol imports in import_map (#875)#979
Conversation
Signed-off-by: LA-10 <lashuha@uwaterloo.com>
|
On where the fix lives, I went with making the resolver handle the value shape build_import_map already produces. The other option would be normalizing at construction instead (store the module QN plus the original leaf name, so the append rule holds for everything). That's arguably cleaner, but it means touching build_import_map in both pass_calls.c and pass_parallel.c and changing the contract at pipeline.h:180. Happy to redo it that way if you'd rather. |
|
Thanks for digging into the import-map shape. I have triaged this as a 0.9.1-rc bug fix for #875. During review we will compare the local resolver fix with the alternative of normalizing build_import_map, and we should keep the LSP alias miss as a separate follow-up so this PR stays focused. |
|
Merged — thank you! The asymmetry analysis (module-valued vs symbol-valued import-map entries) was spot-on, the four-line fix lands exactly at the root cause, and the end-to-end verification on the issue's repro repo is the gold standard. Much appreciated. |
The DeusData#979 direct-hit early return in resolve_import_map fires before suffix handling, so any imported_symbol.method() call whose base symbol is itself an indexed node resolved to the base node instead of base.method — the exact mis-resolution the neighboring comment warns about. On django this degraded the graph by ~11K edges (CALLS 74,685->72,175, TESTS 38,032->29,236); auth.login => Signal.send became an edge onto the user_logged_in Variable node. Take the direct hit only for suffix-less callees — precisely the aliased direct-symbol import case (DeusData#875) the early return was added for. Verified on django: CALLS 74,708 (baseline + the 23 genuine DeusData#979 wins), TESTS 38,035; the DeusData#979 bare-alias test stays green. Reproduce-first: resolve_import_map_alias_with_suffix_hits_method is RED on the unguarded code (returns the bare symbol QN) and GREEN with the guard. Closes DeusData#1000 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
What does this PR do?
trace_pathreturns no inbound caller for a function called through an import alias (from m import f as g;g()). No CALLS edge is emitted at all.Root cause (
registry.c,resolve_import_map):build_import_mapstoreskey = local_name→value = the IMPORTS edge's target QN. Plain imports target the Module node (value = module QN); aliased direct-symbol imports target the Function node (value = full symbol QN). The bare-callee branch assumes the former and always appends:For
_scan_bashthat yields...security_scan.scan_bash._scan_bashmatches nothing, edge dropped. Plain imports work becausemodule + "." + namereconstructs the real QN. That asymmetry is why only aliases break, and whyresolve_import_map_bare_function(module-valuedvals[]) never caught it.Fix: if
resolvedis itself a registered symbol, it is the target return it before appending. Cannot regress plain/dotted imports:pass_definitions.c:337registers only never Module, so module-valued entries can't match.resolve_import_mapis shared by the sequential and parallel pipelines, so one change covers both.Issue #875
Verification
tests/test_registry.c::resolve_import_map_bare_alias— RED on unmodified clone, GREEN with fix. Existing import_map tests stay green.End-to-end on
willow-2.0(the issue's public repro),trace_path(scan_bash, inbound):_scan_shell_fragment,check_kart_task(bothcore/kart_task_scan.py)main(willow/fylgja/events/pre_tool.py) — the production callerCall-site identity preserved, per the triage note:
Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)