Skip to content

Classify autoimport project sources by path containment - #922

Open
marlon-costa-dc wants to merge 3 commits into
python-rope:masterfrom
marlon-costa-dc:pr/autoimport-project-containment
Open

marlon-costa-dc wants to merge 3 commits into
python-rope:masterfrom
marlon-costa-dc:pr/autoimport-project-containment

Conversation

@marlon-costa-dc

Copy link
Copy Markdown
Contributor

get_package_source() classifies a package as project source when the project address is a substring of the package path (project.address in str(package)). A sibling directory such as <project>-other, or a path that leaves the project through .., is therefore indexed as project source by autoimport.

This resolves the package path and requires it to lie under the project root (Path.is_relative_to).

Regression tests cover a sibling directory whose name extends the project's and a path escaping through ..; both return Source.PROJECT on master and Source.UNKNOWN with this change. Tested locally on Python 3.11, 3.13 and 3.14. On 3.10 the project fixture cannot create its nested venv on my machine, for these tests and for the existing ones alike.

@codecov

codecov Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.42%. Comparing base (2a29095) to head (7ce1e8f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #922   +/-   ##
=======================================
  Coverage   95.41%   95.42%           
=======================================
  Files         134      134           
  Lines       26802    26816   +14     
=======================================
+ Hits        25574    25588   +14     
  Misses       1228     1228           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Marlon Costa and others added 2 commits November 4, 2026 23:15
get_package_source treated a package as part of the project when the
project address was a substring of its path, so a sibling directory such
as "<project>-other", or a path leaving the project through "..", was
indexed as project source. Resolve the package path and require it to lie
under the project root.

(cherry picked from commit 5dcf5ee, project containment only)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mcepl
mcepl force-pushed the pr/autoimport-project-containment branch from 4596756 to 3133dbc Compare November 4, 2026 22:15

@mcepl mcepl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed head 3133dbc618bb9abfe627d70f06faa8a1a980e3e4. I found no blocking correctness issue and approve this change.

The substring test confuses path spelling with ancestry: a sibling named <project>-other and a path escaping through .. can both be classified as project source. Resolving both paths before is_relative_to() fixes that distinction. The API is available on the minimum supported Python 3.10, and the default non-strict resolution still permits classification of paths that do not yet exist.

I checked the callers in get_package_tuple() and SQLite autoimport package discovery. The change is limited to source classification; it does not rewrite package paths or module names. Builtin classification retains priority, and external packages still fall through to the existing site-packages, standard-library and unknown classifications.

Validation on Linux with Python 3.13.16:

  • python3 -m pytest -q ropetest/contrib/autoimport ropetest/contrib/autoimporttest.py: 72 passed.
  • Both new regression tests fail with Source.PROJECT instead of Source.UNKNOWN when I substitute the base commit's get_package_source() implementation in memory. They pass on this head.
  • Fifteen additional probes passed, covering the project root and children, a nonexistent child, a relative path, an internal .. path, sibling and parent escapes, symlinks into and out of the project, a project opened through a symlink, builtin precedence, and site-packages, stdlib, unknown and no-project cases.
  • The symlink probes confirm the resolved-path policy: a link inside the project pointing outside is not project source, while an external link pointing into the project is project source.
  • The diff passes git diff --check.

The new tests cover the reported false positives, and the existing positive project-source test continues to pass. Committed symlink regression tests would be useful follow-up coverage for the resolved-path policy, but I do not consider them a blocker here.

Packages inside ignored directories under the project root remain a separate classification issue tracked by #814. This PR does not claim to fix that, and it should not need to expand its scope to do so.

I ran the autoimport tests rather than the full suite locally. At review time, the reported CI checks were passing apart from a pending Python 3.13 macOS job; wait for that job before merging.

@mcepl
mcepl requested a review from lieryan November 4, 2026 22:24

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants