[red-knot] Add support for relative imports#12910
Conversation
|
(I'm expecting this to have a significant impact on the |
|
|
I'll let @MichaReiser review this, since I helped write it :) You'll have to update the expected number of errors in the benchmark before we'll get to see benchmark results. And it looks like Clippy has a complaint. |
MichaReiser
left a comment
There was a problem hiding this comment.
This looks great. Again, much simpler than I expected and nice that the file_to_module provides everything that we need.
6a1a69a to
54ab1f0
Compare
CodSpeed Performance ReportMerging #12910 will degrade performances by 29.01%Comparing Summary
Benchmarks breakdown
|
fc9b5eb to
5138cc3
Compare
| let result = db.check_file(*parser).unwrap(); | ||
|
|
||
| assert_eq!(result.len(), 111); | ||
| assert_eq!(result.len(), 29); |
| fn relative_module_name(&self, tail: Option<&str>, level: NonZeroU32) -> Option<ModuleName> { | ||
| let Some(module) = file_to_module(self.db, self.file) else { | ||
| tracing::debug!("Failed to resolve file {:?} to a module", self.file); | ||
| return None; | ||
| }; | ||
| let mut level = level.get(); | ||
| if module.kind().is_package() { | ||
| level -= 1; | ||
| } | ||
| let mut module_name = module.name().to_owned(); | ||
| for _ in 0..level { | ||
| module_name = module_name.parent()?; | ||
| } | ||
| if let Some(tail) = tail { | ||
| if let Some(valid_tail) = ModuleName::new(tail) { | ||
| module_name.extend(&valid_tail); | ||
| } else { | ||
| tracing::debug!("Failed to resolve relative import due to invalid syntax"); | ||
| return None; | ||
| } | ||
| } | ||
| Some(module_name) | ||
| } |
There was a problem hiding this comment.
Are these tracing calls the kind of thing you were thinking of @MichaReiser? I also added a tracing::trace!() call to infer_import_from_definition.
| /// module_name.extend(&ModuleName::new_static("baz.eggs.ham").unwrap()); | ||
| /// assert_eq!(&module_name, "foo.bar.baz.eggs.ham"); | ||
| /// ``` | ||
| pub fn extend(&mut self, other: &ModuleName) { |
There was a problem hiding this comment.
Nice, I also had a thought late last night that this is the name we should have used for this method, so glad you all settled on the same thing 😆
Summary
This PR adds support for relative import statements, e.g.
from .foo import bar,from . import bar, orfrom ...foo import bar.Test Plan
Several tests added to
infer.rs.Co-authored-by: Carl Meyer carl@astral.sh