fix: resolve service-rooted absolute imports in multi-root repositories#624
Open
pallav10 wants to merge 1 commit into
Open
fix: resolve service-rooted absolute imports in multi-root repositories#624pallav10 wants to merge 1 commit into
pallav10 wants to merge 1 commit into
Conversation
resolve_absolute_import only tried <scan_root>/<module path> and <project_root>/<module path>. In repositories that contain nested service roots (Django projects under backend/<service>/, src/ layouts, monorepo subprojects), absolute imports are rooted at the service directory: 'from accounting.services import X' inside pulse-core/accounting/tasks.py resolves under pulse-core/, not the scan root. Those imports silently produced no graph edges, which cascaded into detector false positives: hundreds of live modules flagged orphaned (zero importers), import-based test-coverage mapping unable to mark modules as directly tested, and broken transitive coverage. resolve_absolute_import now also walks the importing file's ancestor directories (source_dir up to scan_root) as candidate package roots, keeping the existing scan-root-first then project-root order, and resolve_python_import passes the source directory through. Backward compatible: source_dir is optional and dynamic-import resolution is unchanged. Full test suite passes (3 pre-existing bash unused-import failures on main are unrelated).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
resolve_absolute_importonly tries<scan_root>/<module>and<project_root>/<module>. In repositories with nested service roots (e.g. a backend repo containing a Django service atpulse-core/plus a FastAPI service atapi-gateway/), absolute imports are rooted at the service directory:from accounting.services import Xinsidepulse-core/accounting/tasks.pyresolves underpulse-core/, not the scan root.Command:
desloppify scan --path .over such a repo (2,545 files, 545K LOC).Expected: import edges for service-rooted absolute imports.
Actual: those imports resolve to nothing, so the graph silently loses edges. Cascading effects observed on a real codebase:
Fix
resolve_absolute_importgains an optionalsource_dirparameter and walks the importing file's ancestor directories (up to the scan root) as candidate package roots, preserving the existing scan-root-first, project-root-last order.resolve_python_importpasses the source directory through. Dynamic-import resolution is unchanged (no source context).Added regression test
test_absolute_import_resolves_from_service_rootmodeling the Django service layout.python -m pytest desloppify/tests/ -q: 5661 passed, 151 skipped, 3 failed — the 3 failures arelang/common/test_bash_unused_imports.pycases that fail identically on main without this change.