Skip to content

Commit

Permalink
Merge pull request #373 from gauge-sh/fix-relative-import-handling
Browse files Browse the repository at this point in the history
Fix handling relative imports at import depth > 1
  • Loading branch information
emdoyle authored Oct 17, 2024
2 parents cbc2ebf + c6142e1 commit 6035b1a
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 40 deletions.
3 changes: 3 additions & 0 deletions python/tests/example/valid/domain_two/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,7 @@
from domain_four import ok
from domain_three import x

from .. import domain_three
y = domain_three

__all__ = ["x", "ok"]
Empty file.
3 changes: 3 additions & 0 deletions python/tests/example/valid/domain_two/some_file.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from . import other

__all__ = ["other"]
31 changes: 31 additions & 0 deletions python/tests/test_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,34 @@ def test_file_outside_source_root(temp_project, tmp_path):
("file1.c", 3),
]
assert result == expected


def test_relative_import_from_parent(temp_project):
# Create a nested directory structure
(temp_project / "parent" / "child").mkdir(parents=True, exist_ok=True)

# Create a file in the parent directory
parent_file_content = """
def parent_function():
pass
"""
create_temp_file(temp_project / "parent", "parent_module.py", parent_file_content)

# Create a file in the child directory with a relative import from the parent
child_file_content = """
from .. import parent_module
def child_function():
parent_module.parent_function()
"""
create_temp_file(
temp_project / "parent" / "child", "child_module.py", child_file_content
)

result = get_project_imports(
[str(temp_project)],
str(temp_project / "parent" / "child" / "child_module.py"),
ignore_type_checking_imports=True,
)
expected = [("parent.parent_module", 2)]
assert result == expected
85 changes: 45 additions & 40 deletions src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,44 +151,53 @@ impl<'a> ImportVisitor<'a> {
let mut imports = NormalizedImports::new();

let import_depth: usize = import_statement.level.try_into().unwrap();
let num_paths_to_strip = if self.is_package {
import_depth.saturating_sub(1)
} else {
import_depth
};

let mod_path = match &self.file_mod_path {
Some(mod_path) => mod_path,
None => "",
};
// If our current file mod path is None, we are not within the source root
// so we assume that relative imports are also not within the source root
if mod_path.is_empty() && import_depth > 0 {
return imports;
};

let base_path_parts: Vec<&str> = mod_path.split('.').collect();
let base_path_parts = if num_paths_to_strip > 0 {
base_path_parts[..base_path_parts.len() - num_paths_to_strip].to_vec()
} else {
base_path_parts
};

let base_mod_path = if let Some(ref module) = import_statement.module {
if import_depth > 0 {
// For relative imports (level > 0), adjust the base module path
let num_paths_to_strip = if self.is_package {
import_depth - 1

// base_mod_path becomes the current file's mod path
// minus the paths_to_strip (due to level of import)
// plus the module we are importing from
if base_path_parts.is_empty() {
module.to_string()
} else {
import_depth
};

// If our current file mod path is None, we are not within the source root
// so we assume that relative imports are also not within the source root
match &self.file_mod_path {
None => return imports, // early return from the outer function
Some(mod_path) => {
let base_path_parts: Vec<&str> = mod_path.split('.').collect();
let base_path_parts = if num_paths_to_strip > 0 {
base_path_parts[..base_path_parts.len() - num_paths_to_strip].to_vec()
} else {
base_path_parts
};

if base_path_parts.is_empty() {
module.to_string()
} else {
// base_mod_path is the current file's mod path
// minus the paths_to_strip (due to level of import)
// plus the module we are importing from
format!("{}.{}", base_path_parts.join("."), module)
}
}
format!("{}.{}", base_path_parts.join("."), module)
}
} else {
module.to_string()
}
} else {
// We are importing from the current package ('.')
String::new()
// We are importing from the current package ('.') or a parent ('..' or more)
// We have already stripped parts from the current file's mod path based on the import depth,
// so we just need to join the remaining parts with a '.'
if base_path_parts.is_empty() {
// This means we are looking at a current package import outside of a source root
return imports;
}
base_path_parts.join(".")
};

let line_no = self
Expand All @@ -207,23 +216,19 @@ impl<'a> ImportVisitor<'a> {
}

for name in &import_statement.names {
let local_mod_path = format!(
"{}{}.{}",
".".repeat(import_depth),
import_statement.module.as_deref().unwrap_or(""),
name.asname.as_deref().unwrap_or(name.name.as_ref())
);
if let Some(ignored) = ignored_modules {
if ignored.contains(&local_mod_path) {
if ignored.contains(
&name
.asname
.as_deref()
.unwrap_or(name.name.as_ref())
.to_string(),
) {
continue; // This import is ignored by a directive
}
}

let global_mod_path = match import_statement.module {
Some(_) => format!("{}.{}", base_mod_path, name.name.as_str()),
None => name.name.to_string(),
};

let global_mod_path = format!("{}.{}", base_mod_path, name.name.as_str());
imports.push(NormalizedImport {
module_path: global_mod_path,
line_no: self
Expand Down

0 comments on commit 6035b1a

Please sign in to comment.