Skip to content

Commit

Permalink
Use module path resolver for relative autofix (#4027)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Apr 19, 2023
1 parent 7fa1da2 commit 0d84517
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 67 deletions.
2 changes: 1 addition & 1 deletion crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,7 @@ where
stmt,
*level,
module.as_deref(),
self.module_path.as_ref(),
self.module_path.as_deref(),
&self.settings.flake8_tidy_imports.ban_relative_imports,
)
{
Expand Down
95 changes: 29 additions & 66 deletions crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize};

use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Violation};
use ruff_macros::{derive_message_formats, violation, CacheKey};
use ruff_python_ast::helpers::{create_stmt, from_relative_import, unparse_stmt};
use ruff_python_ast::helpers::{create_stmt, resolve_imported_module_path, unparse_stmt};
use ruff_python_ast::source_code::Stylist;
use ruff_python_ast::types::Range;
use ruff_python_stdlib::identifiers::is_identifier;
Expand Down Expand Up @@ -90,74 +90,37 @@ fn fix_banned_relative_import(
stmt: &Stmt,
level: Option<usize>,
module: Option<&str>,
module_path: Option<&Vec<String>>,
module_path: Option<&[String]>,
stylist: &Stylist,
) -> Option<Edit> {
// Only fix is the module path is known.
if let Some(mut parts) = module_path.cloned() {
if level? >= parts.len() {
return None;
}

// Remove relative level from module path.
for _ in 0..level? {
parts.pop();
}

let module_name = if let Some(module) = module {
let call_path = from_relative_import(&parts, module);
// Empty indicates an invalid module.
if call_path.is_empty() {
return None;
}
// Require import to be a valid module:
// https://python.org/dev/peps/pep-0008/#package-and-module-names
if !call_path.iter().all(|part| is_identifier(part)) {
return None;
}
call_path.as_slice().join(".")
} else if parts.len() > 1 {
let module = parts.pop().unwrap();
let call_path = from_relative_import(&parts, &module);
// Empty indicates an invalid module.
if call_path.is_empty() {
return None;
}
// Require import to be a valid module:
// https://python.org/dev/peps/pep-0008/#package-and-module-names
if !call_path.iter().all(|part| is_identifier(part)) {
return None;
}
call_path.as_slice().join(".")
} else {
// Require import to be a valid module:
// https://python.org/dev/peps/pep-0008/#package-and-module-names
if !parts.iter().all(|part| is_identifier(part)) {
return None;
}
parts.join(".")
};

let StmtKind::ImportFrom { names, .. } = &stmt.node else {
panic!("Expected StmtKind::ImportFrom");
};
let content = unparse_stmt(
&create_stmt(StmtKind::ImportFrom {
module: Some(module_name),
names: names.clone(),
level: Some(0),
}),
stylist,
);
let Some(module_path) = resolve_imported_module_path(level, module, module_path) else {
return None;
};

Some(Edit::replacement(
content,
stmt.location,
stmt.end_location.unwrap(),
))
} else {
None
// Require import to be a valid module:
// https://python.org/dev/peps/pep-0008/#package-and-module-names
if !module_path.split('.').all(is_identifier) {
return None;
}

let StmtKind::ImportFrom { names, .. } = &stmt.node else {
panic!("Expected StmtKind::ImportFrom");
};
let content = unparse_stmt(
&create_stmt(StmtKind::ImportFrom {
module: Some(module_path.to_string()),
names: names.clone(),
level: Some(0),
}),
stylist,
);

Some(Edit::replacement(
content,
stmt.location,
stmt.end_location.unwrap(),
))
}

/// TID252
Expand All @@ -166,7 +129,7 @@ pub fn banned_relative_import(
stmt: &Stmt,
level: Option<usize>,
module: Option<&str>,
module_path: Option<&Vec<String>>,
module_path: Option<&[String]>,
strictness: &Strictness,
) -> Option<Diagnostic> {
let strictness_level = match strictness {
Expand Down Expand Up @@ -197,9 +160,9 @@ pub fn banned_relative_import(
mod tests {
use std::path::Path;

use crate::assert_messages;
use anyhow::Result;

use crate::assert_messages;
use crate::registry::Rule;
use crate::settings::Settings;
use crate::test::test_path;
Expand Down

0 comments on commit 0d84517

Please sign in to comment.