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] consider fixing spans related to relative imports #15977

Open
BurntSushi opened this issue Feb 5, 2025 · 1 comment
Open

[red-knot] consider fixing spans related to relative imports #15977

BurntSushi opened this issue Feb 5, 2025 · 1 comment
Labels
bug Something isn't working diagnostics Related to reporting of diagnostics. red-knot Multi-file analysis & type inference

Comments

@BurntSushi
Copy link
Member

Description

This issue originally sprouted from here: #15976 (comment)

Consider this Python program:

from ...zqzqzqzq import hi

Running red-knot at present gives you this:

$ run-red-knot -q pr1 -- check
error: lint:unresolved-import
 --> /home/andrew/astral/ruff/play/diags/play/test.py:1:9
  |
1 | from ...zqzqzqzq import hi
  |         ^^^^^^^^ Cannot resolve import `...zqzqzqzq`
  |

The range here is a little inconsistent with the actual message. The range is only on the module name, but the message includes the preceding .... Indeed, if we futz with the Python program a bit more, we can somewhat observe what's going wrong:

from . . . zqzqzqzq import hi

Gives this:

$ run-red-knot -q pr1 -- check
error: lint:unresolved-import
 --> /home/andrew/astral/ruff/play/diags/play/test.py:1:12
  |
1 | from . . . zqzqzqzq import hi
  |            ^^^^^^^^ Cannot resolve import `...zqzqzqzq`
  |

Here, the diagnostic message is showing something that does not match the actual source code.

I believe the underlying problem is the AST definition of a from import:

/// See also [ImportFrom](https://docs.python.org/3/library/ast.html#ast.ImportFrom)
#[derive(Clone, Debug, PartialEq)]
pub struct StmtImportFrom {
pub range: TextRange,
pub module: Option<Identifier>,
pub names: Vec<Alias>,
pub level: u32,
}

Namely, it normalizes the level based on the preceding dots. But in doing so, this drops the range associated with the dots.

@BurntSushi BurntSushi added the bug Something isn't working label Feb 5, 2025
@BurntSushi
Copy link
Member Author

An alternative is to change the diagnostic message to make it in sync with the range. The range isn't arguably wrong per se, but I would guess that including the dots is more semantically correct? (Because the dots may be what leads to a module import being unresolvable.)

@BurntSushi BurntSushi added the diagnostics Related to reporting of diagnostics. label Feb 5, 2025
@AlexWaygood AlexWaygood added the red-knot Multi-file analysis & type inference label Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working diagnostics Related to reporting of diagnostics. red-knot Multi-file analysis & type inference
Projects
None yet
Development

No branches or pull requests

2 participants