fix(ingestion): resolve module.symbol() calls through submodule bindings - #1733
Conversation
|
✅ Health of changed files: 7.9 → 8.3 (+0.4) 📋 At a glance Files & modules (2)
✅ Health gate: passed 🔎 More signals (2)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (2 with dependents)"]
f_packages_core_src_repowise_core_ingestion_resolvers_python_py[".../resolvers/python.py 🔥"]:::changed
f_packages_core_src_repowise_core_ingestion_import_index_py[".../ingestion/import_index.py"]:::changed
end
f_packages_core_src_repowise_core_ingestion_graph_builder_py[".../graph/builder.py"]
f_packages_core_src_repowise_core_ingestion_resolvers_python_py --> f_packages_core_src_repowise_core_ingestion_graph_builder_py
f_packages_core_src_repowise_core_ingestion_resolvers___init___py[".../resolvers/__init__.py"]
f_packages_core_src_repowise_core_ingestion_resolvers_python_py --> f_packages_core_src_repowise_core_ingestion_resolvers___init___py
f_packages_core_src_repowise_core_ingestion_call_resolver_py[".../ingestion/call_resolver.py"]
f_packages_core_src_repowise_core_ingestion_import_index_py --> f_packages_core_src_repowise_core_ingestion_call_resolver_py
f_packages_core_src_repowise_core_ingestion_graph__resolvers_py[".../graph/_resolvers.py"]
f_packages_core_src_repowise_core_ingestion_import_index_py --> f_packages_core_src_repowise_core_ingestion_graph__resolvers_py
f_packages_core_src_repowise_core_ingestion_heritage_resolver_py[".../ingestion/heritage_resolver.py"]
f_packages_core_src_repowise_core_ingestion_import_index_py --> f_packages_core_src_repowise_core_ingestion_heritage_resolver_py
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (5 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (2)
👀 Suggested reviewers @RaghavChamadiya 📊 See the full report for this PR |
`from pkg import submodule` bound `submodule` to the package `__init__.py` (the fan-out's first target), so a later `submodule.symbol()` call resolved against a file that declares nothing and the call was missed — dead-code then reported the symbol as safe-to-delete despite a real caller (repowise-dev#1193). Two-part fix: - `resolve_python_import_all` now points each submodule binding's source_file at the submodule file it resolves to, not the package init. - `build_import_name_maps` only back-fills a binding's source_file when it is unset, so the submodule pin survives. Adds regression tests for the submodule-binding pin and the non-submodule (package re-export) case.
b363886 to
5666ef8
Compare
|
Rebased this onto the latest main — the CI failure was just a stale test file on my branch (the |
|
Thanks @sloemo01. The two-part framing in the description is what made this quick to check, because the interesting part of this fix is not either edit on its own, it is that they have to happen in that order. The diagnosis is right and it is a level below where I would have looked first. I traced the ordering, because the Recording one thing rather than asking for a change. Not a blocker and not for this PR: the guard also makes Nothing to change. Waiting on the same workflow-run approval as #1731 and #1732. |
Fixes #1193
Problem
from pkg import submoduleboundsubmoduleto the package__init__.py(the fan-out's first target), so a latersubmodule.symbol()call resolved against a file that declares nothing and the call was missed. Dead-code then reported the symbol as safe-to-delete despite a real caller.Fix
Two-part:
resolve_python_import_allnow points each submodule binding'ssource_fileat the submodule file it resolves to, not the package init.build_import_name_mapsonly back-fills a binding'ssource_filewhen it is unset, so the submodule pin survives.Verified end-to-end:
from sensors import foo; foo.check()now resolves tosensors/foo.py::check(0.88).Test
Adds regression tests for the submodule-binding pin and the non-submodule (package re-export) case.
test_python_resolver_fanout.py: 9 passed; fulltests/unit/ingestion+tests/unit/dead_code: 2855 passed (1 pre-existing env failure unrelated to this change).