Skip to content

Commit

Permalink
Make ImportFrom level just a u32 (#11170)
Browse files Browse the repository at this point in the history
  • Loading branch information
carljm committed Apr 27, 2024
1 parent 5994414 commit 845ba7c
Show file tree
Hide file tree
Showing 34 changed files with 100 additions and 182 deletions.
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/mod.rs
Expand Up @@ -464,7 +464,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
let level = *level;

// Mark the top-level module as "seen" by the semantic model.
if level.map_or(true, |level| level == 0) {
if level == 0 {
if let Some(module) = module.and_then(|module| module.split('.').next()) {
self.semantic.add_module(module);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/imports.rs
Expand Up @@ -39,7 +39,7 @@ fn extract_import_map(path: &Path, package: Option<&Path>, blocks: &[&Block]) ->
level,
range: _,
}) => {
let level = level.unwrap_or_default() as usize;
let level = *level as usize;
let module = if let Some(module) = module {
let module: &String = module.as_ref();
if level == 0 {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/importer/mod.rs
Expand Up @@ -405,7 +405,7 @@ impl<'a> Importer<'a> {
range: _,
}) = stmt
{
if level.map_or(true, |level| level == 0)
if *level == 0
&& name.as_ref().is_some_and(|name| name == module)
&& names.iter().all(|alias| alias.name.as_str() != "*")
{
Expand Down
Expand Up @@ -54,13 +54,11 @@ pub(crate) fn import(import_from: &Stmt, name: &str, asname: Option<&str>) -> Op
pub(crate) fn import_from(
import_from: &Stmt,
module: Option<&str>,
level: Option<u32>,
level: u32,
) -> Option<Diagnostic> {
// If level is not zero or module is none, return
if let Some(level) = level {
if level != 0 {
return None;
}
if level != 0 {
return None;
};

if let Some(module) = module {
Expand Down
Expand Up @@ -76,7 +76,7 @@ impl Violation for RelativeImports {

fn fix_banned_relative_import(
stmt: &Stmt,
level: Option<u32>,
level: u32,
module: Option<&str>,
module_path: Option<&[String]>,
generator: Generator,
Expand All @@ -99,7 +99,7 @@ fn fix_banned_relative_import(
TextRange::default(),
)),
names: names.clone(),
level: Some(0),
level: 0,
range: TextRange::default(),
};
let content = generator.stmt(&node.into());
Expand All @@ -113,7 +113,7 @@ fn fix_banned_relative_import(
pub(crate) fn banned_relative_import(
checker: &Checker,
stmt: &Stmt,
level: Option<u32>,
level: u32,
module: Option<&str>,
module_path: Option<&[String]>,
strictness: Strictness,
Expand All @@ -122,7 +122,7 @@ pub(crate) fn banned_relative_import(
Strictness::All => 0,
Strictness::Parents => 1,
};
if level? > strictness_level {
if level > strictness_level {
let mut diagnostic = Diagnostic::new(RelativeImports { strictness }, stmt.range());
if let Some(fix) =
fix_banned_relative_import(stmt, level, module, module_path, checker.generator())
Expand Down
Expand Up @@ -300,7 +300,7 @@ pub(crate) fn typing_only_runtime_import(
// Categorize the import, using coarse-grained categorization.
let import_type = match categorize(
&qualified_name.to_string(),
None,
0,
&checker.settings.src,
checker.package(),
checker.settings.isort.detect_same_package,
Expand Down
10 changes: 5 additions & 5 deletions crates/ruff_linter/src/rules/isort/categorize.rs
Expand Up @@ -92,7 +92,7 @@ enum Reason<'a> {
#[allow(clippy::too_many_arguments)]
pub(crate) fn categorize<'a>(
module_name: &str,
level: Option<u32>,
level: u32,
src: &[PathBuf],
package: Option<&Path>,
detect_same_package: bool,
Expand All @@ -104,14 +104,14 @@ pub(crate) fn categorize<'a>(
) -> &'a ImportSection {
let module_base = module_name.split('.').next().unwrap();
let (mut import_type, mut reason) = {
if matches!(level, None | Some(0)) && module_base == "__future__" {
if level == 0 && module_base == "__future__" {
(&ImportSection::Known(ImportType::Future), Reason::Future)
} else if no_sections {
(
&ImportSection::Known(ImportType::FirstParty),
Reason::NoSections,
)
} else if level.is_some_and(|level| level > 0) {
} else if level > 0 {
(
&ImportSection::Known(ImportType::LocalFolder),
Reason::NonZeroLevel,
Expand All @@ -133,7 +133,7 @@ pub(crate) fn categorize<'a>(
&ImportSection::Known(ImportType::FirstParty),
Reason::SourceMatch(src),
)
} else if matches!(level, None | Some(0)) && module_name == "__main__" {
} else if level == 0 && module_name == "__main__" {
(
&ImportSection::Known(ImportType::FirstParty),
Reason::KnownFirstParty,
Expand Down Expand Up @@ -191,7 +191,7 @@ pub(crate) fn categorize_imports<'a>(
for (alias, comments) in block.import {
let import_type = categorize(
&alias.module_name(),
None,
0,
src,
package,
detect_same_package,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/isort/mod.rs
Expand Up @@ -52,7 +52,7 @@ pub(crate) enum AnnotatedImport<'a> {
ImportFrom {
module: Option<&'a str>,
names: Vec<AnnotatedAliasData<'a>>,
level: Option<u32>,
level: u32,
atop: Vec<Comment<'a>>,
inline: Vec<Comment<'a>>,
trailing_comma: TrailingComma,
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/rules/isort/order.rs
Expand Up @@ -73,7 +73,7 @@ pub(crate) fn order_imports<'a>(
ModuleKey::from_module(
Some(alias.name),
alias.asname,
None,
0,
None,
ImportStyle::Straight,
settings,
Expand All @@ -90,7 +90,7 @@ pub(crate) fn order_imports<'a>(
Import((alias, _)) => ModuleKey::from_module(
Some(alias.name),
alias.asname,
None,
0,
None,
ImportStyle::Straight,
settings,
Expand All @@ -110,7 +110,7 @@ pub(crate) fn order_imports<'a>(
ModuleKey::from_module(
Some(alias.name),
alias.asname,
None,
0,
None,
ImportStyle::Straight,
settings,
Expand Down
10 changes: 4 additions & 6 deletions crates/ruff_linter/src/rules/isort/sorting.rs
Expand Up @@ -97,7 +97,7 @@ impl<'a> ModuleKey<'a> {
pub(crate) fn from_module(
name: Option<&'a str>,
asname: Option<&'a str>,
level: Option<u32>,
level: u32,
first_alias: Option<(&'a str, Option<&'a str>)>,
style: ImportStyle,
settings: &Settings,
Expand All @@ -106,13 +106,11 @@ impl<'a> ModuleKey<'a> {

let maybe_length = (settings.length_sort
|| (settings.length_sort_straight && style == ImportStyle::Straight))
.then_some(
name.map(str::width).unwrap_or_default() + level.unwrap_or_default() as usize,
);
.then_some(name.map(str::width).unwrap_or_default() + level as usize);

let distance = match level {
None | Some(0) => Distance::None,
Some(level) => match settings.relative_imports_order {
0 => Distance::None,
_ => match settings.relative_imports_order {
RelativeImportsOrder::ClosestToFurthest => Distance::Nearest(level),
RelativeImportsOrder::FurthestToClosest => Distance::Furthest(Reverse(level)),
},
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/isort/types.rs
Expand Up @@ -14,7 +14,7 @@ pub(crate) enum TrailingComma {
#[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq, Clone)]
pub(crate) struct ImportFromData<'a> {
pub(crate) module: Option<&'a str>,
pub(crate) level: Option<u32>,
pub(crate) level: u32,
}

#[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq)]
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pylint/rules/import_self.rs
Expand Up @@ -51,7 +51,7 @@ pub(crate) fn import_self(alias: &Alias, module_path: Option<&[String]>) -> Opti

/// PLW0406
pub(crate) fn import_from_self(
level: Option<u32>,
level: u32,
module: Option<&str>,
names: &[Alias],
module_path: Option<&[String]>,
Expand Down
Expand Up @@ -78,7 +78,7 @@ pub(crate) fn manual_from_import(
asname: None,
range: TextRange::default(),
}],
level: Some(0),
level: 0,
range: TextRange::default(),
};
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
Expand Down
Expand Up @@ -68,7 +68,7 @@ pub(crate) fn deprecated_c_element_tree(checker: &mut Checker, stmt: &Stmt) {
level,
range: _,
}) => {
if level.is_some_and(|level| level > 0) {
if *level > 0 {
// Ex) `import .xml.etree.cElementTree as ET`
} else if let Some(module) = module {
if module == "xml.etree.cElementTree" {
Expand Down
Expand Up @@ -643,10 +643,10 @@ pub(crate) fn deprecated_import(
stmt: &Stmt,
names: &[Alias],
module: Option<&str>,
level: Option<u32>,
level: u32,
) {
// Avoid relative and star imports.
if level.is_some_and(|level| level > 0) {
if level > 0 {
return;
}
if names.first().is_some_and(|name| &name.name == "*") {
Expand Down
Expand Up @@ -319,7 +319,7 @@ pub(crate) fn deprecated_mock_import(checker: &mut Checker, stmt: &Stmt) {
level,
..
}) => {
if level.is_some_and(|level| level > 0) {
if *level > 0 {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_ast/src/comparable.rs
Expand Up @@ -1306,7 +1306,7 @@ pub struct StmtImport<'a> {
pub struct StmtImportFrom<'a> {
module: Option<&'a str>,
names: Vec<ComparableAlias<'a>>,
level: Option<u32>,
level: u32,
}

#[derive(Debug, PartialEq, Eq, Hash)]
Expand Down
62 changes: 25 additions & 37 deletions crates/ruff_python_ast/src/helpers.rs
Expand Up @@ -732,13 +732,13 @@ where
/// ```rust
/// # use ruff_python_ast::helpers::format_import_from;
///
/// assert_eq!(format_import_from(None, None), "".to_string());
/// assert_eq!(format_import_from(Some(1), None), ".".to_string());
/// assert_eq!(format_import_from(Some(1), Some("foo")), ".foo".to_string());
/// assert_eq!(format_import_from(0, None), "".to_string());
/// assert_eq!(format_import_from(1, None), ".".to_string());
/// assert_eq!(format_import_from(1, Some("foo")), ".foo".to_string());
/// ```
pub fn format_import_from(level: Option<u32>, module: Option<&str>) -> String {
pub fn format_import_from(level: u32, module: Option<&str>) -> String {
let mut module_name = String::with_capacity(16);
if let Some(level) = level {
if level > 0 {
for _ in 0..level {
module_name.push('.');
}
Expand All @@ -756,18 +756,15 @@ pub fn format_import_from(level: Option<u32>, module: Option<&str>) -> String {
/// ```rust
/// # use ruff_python_ast::helpers::format_import_from_member;
///
/// assert_eq!(format_import_from_member(None, None, "bar"), "bar".to_string());
/// assert_eq!(format_import_from_member(Some(1), None, "bar"), ".bar".to_string());
/// assert_eq!(format_import_from_member(Some(1), Some("foo"), "bar"), ".foo.bar".to_string());
/// assert_eq!(format_import_from_member(0, None, "bar"), "bar".to_string());
/// assert_eq!(format_import_from_member(1, None, "bar"), ".bar".to_string());
/// assert_eq!(format_import_from_member(1, Some("foo"), "bar"), ".foo.bar".to_string());
/// ```
pub fn format_import_from_member(level: Option<u32>, module: Option<&str>, member: &str) -> String {
pub fn format_import_from_member(level: u32, module: Option<&str>, member: &str) -> String {
let mut qualified_name = String::with_capacity(
(level.unwrap_or(0) as usize)
+ module.as_ref().map_or(0, |module| module.len())
+ 1
+ member.len(),
(level as usize) + module.as_ref().map_or(0, |module| module.len()) + 1 + member.len(),
);
if let Some(level) = level {
if level > 0 {
for _ in 0..level {
qualified_name.push('.');
}
Expand Down Expand Up @@ -801,29 +798,27 @@ pub fn to_module_path(package: &Path, path: &Path) -> Option<Vec<String>> {
/// ```rust
/// # use ruff_python_ast::helpers::collect_import_from_member;
///
/// assert_eq!(collect_import_from_member(None, None, "bar").segments(), ["bar"]);
/// assert_eq!(collect_import_from_member(Some(1), None, "bar").segments(), [".", "bar"]);
/// assert_eq!(collect_import_from_member(Some(1), Some("foo"), "bar").segments(), [".", "foo", "bar"]);
/// assert_eq!(collect_import_from_member(0, None, "bar").segments(), ["bar"]);
/// assert_eq!(collect_import_from_member(1, None, "bar").segments(), [".", "bar"]);
/// assert_eq!(collect_import_from_member(1, Some("foo"), "bar").segments(), [".", "foo", "bar"]);
/// ```
pub fn collect_import_from_member<'a>(
level: Option<u32>,
level: u32,
module: Option<&'a str>,
member: &'a str,
) -> QualifiedName<'a> {
let mut qualified_name_builder = QualifiedNameBuilder::with_capacity(
level.unwrap_or_default() as usize
level as usize
+ module
.map(|module| module.split('.').count())
.unwrap_or_default()
+ 1,
);

// Include the dots as standalone segments.
if let Some(level) = level {
if level > 0 {
for _ in 0..level {
qualified_name_builder.push(".");
}
if level > 0 {
for _ in 0..level {
qualified_name_builder.push(".");
}
}

Expand Down Expand Up @@ -875,14 +870,10 @@ pub fn from_relative_import<'a>(
/// Given an imported module (based on its relative import level and module name), return the
/// fully-qualified module path.
pub fn resolve_imported_module_path<'a>(
level: Option<u32>,
level: u32,
module: Option<&'a str>,
module_path: Option<&[String]>,
) -> Option<Cow<'a, str>> {
let Some(level) = level else {
return Some(Cow::Borrowed(module.unwrap_or("")));
};

if level == 0 {
return Some(Cow::Borrowed(module.unwrap_or("")));
}
Expand Down Expand Up @@ -1572,14 +1563,14 @@ mod tests {
fn resolve_import() {
// Return the module directly.
assert_eq!(
resolve_imported_module_path(None, Some("foo"), None),
resolve_imported_module_path(0, Some("foo"), None),
Some(Cow::Borrowed("foo"))
);

// Construct the module path from the calling module's path.
assert_eq!(
resolve_imported_module_path(
Some(1),
1,
Some("foo"),
Some(&["bar".to_string(), "baz".to_string()])
),
Expand All @@ -1588,19 +1579,16 @@ mod tests {

// We can't return the module if it's a relative import, and we don't know the calling
// module's path.
assert_eq!(
resolve_imported_module_path(Some(1), Some("foo"), None),
None
);
assert_eq!(resolve_imported_module_path(1, Some("foo"), None), None);

// We can't return the module if it's a relative import, and the path goes beyond the
// calling module's path.
assert_eq!(
resolve_imported_module_path(Some(1), Some("foo"), Some(&["bar".to_string()])),
resolve_imported_module_path(1, Some("foo"), Some(&["bar".to_string()])),
None,
);
assert_eq!(
resolve_imported_module_path(Some(2), Some("foo"), Some(&["bar".to_string()])),
resolve_imported_module_path(2, Some("foo"), Some(&["bar".to_string()])),
None
);
}
Expand Down

0 comments on commit 845ba7c

Please sign in to comment.