Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] Fix relative imports in src.root #15990

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Feb 6, 2025

Summary

Fixes #15989

Red Knot failed to resolve relative imports if the importing module is located at a search path root.

The issue was that the module resolver returned an Err(TooManyDots) as soon as the parent of the current module is None (which is the case for a module at the search path root).
However, this is incorrect if there's a tail (a module name).

Test Plan

Added mdtests

@MichaReiser MichaReiser force-pushed the micha/fix-relative-imports-in-src-root branch from d3dd1ee to 8f08246 Compare February 6, 2025 09:25
@MichaReiser MichaReiser added bug Something isn't working red-knot Multi-file analysis & type inference labels Feb 6, 2025
@MichaReiser MichaReiser marked this pull request as ready for review February 6, 2025 09:25
Comment on lines 1 to 2
from crates.ruff_linter.resources.test.cpython.Lib.typing import reveal_type

Copy link
Member

Choose a reason for hiding this comment

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

not sure what this is doing here :P

Suggested change
from crates.ruff_linter.resources.test.cpython.Lib.typing import reveal_type

Copy link
Member Author

Choose a reason for hiding this comment

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

lol, Pycharm tried to be clever

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching this

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Nice -- well diagnosed!

@MichaReiser MichaReiser force-pushed the micha/fix-relative-imports-in-src-root branch from 8f08246 to a1e265d Compare February 6, 2025 14:04
@MichaReiser MichaReiser enabled auto-merge (squash) February 6, 2025 14:06
@MichaReiser MichaReiser merged commit 5588c75 into main Feb 6, 2025
20 checks passed
@MichaReiser MichaReiser deleted the micha/fix-relative-imports-in-src-root branch February 6, 2025 14:08
@carljm
Copy link
Contributor

carljm commented Feb 6, 2025

I think this change is not correct? Python does not actually allow this at runtime. It seems like a bug in pyright if it allows it.

If I create these two files:

lib.py:

x = 1

main.py:

from .lib import x

The result of python3 -c 'import main' is ImportError: attempted relative import with no known parent package

The tomllib modules actually exist within a tomllib package, so the relative imports are fine; they are not meant to be top-level modules.

@MichaReiser
Copy link
Member Author

Hmm true. I'll revert tomorrow and add an explicit test for it. I didn't consider testing it, I trusted Pyright but it seems I shouldn't have.

MichaReiser added a commit that referenced this pull request Feb 6, 2025
MichaReiser added a commit that referenced this pull request Feb 7, 2025
…e root of a search path (#16001)

## Summary

This PR reverts the behavior changes from
#15990

But it isn't just a revert, it also:

* Adds a test covering this specific behavior
* Preserves the improvement to use `saturating_sub` in the package case
to avoid overflows in the case of invalid syntax
* Use `ancestors` instead of a `for` loop

## Test Plan

Added test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[red-knot] Relative imports in the root directory
3 participants