mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-01 14:21:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Refactor: use Settings struct (#7665)

Browse source 
## Summary

Pass around a `Settings` struct instead of individual members to
simplify function signatures and to make it easier to add new settings.

This PR was suggested in [this
comment](https://github.com/astral-sh/ruff/issues/1567#issuecomment-1734182803).

## Note on the choices

I chose which functions to modify based on which seem most likely to use
new settings, but suggestions on my choices are welcome!
This commit is contained in:
bluthej 2023-09-26 19:17:18 +02:00 • committed by GitHub
parent 8165925e01
commit ee533332ed
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 80 additions and 230 deletions
Download patch file Download diff file Expand all files Collapse all files

109
crates/ruff_linter/src/rules/isort/mod.rs
View file

@ -1,6 +1,5 @@
//! Rules from [isort](https://pypi.org/project/isort/). //! Rules from [isort](https://pypi.org/project/isort/).
use std::collections::BTreeSet;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use annotate::annotate_imports; use annotate::annotate_imports;
@ -14,14 +13,12 @@ use order::order_imports;
use ruff_python_ast::PySourceType; use ruff_python_ast::PySourceType;
use ruff_python_codegen::Stylist; use ruff_python_codegen::Stylist;
use ruff_source_file::Locator; use ruff_source_file::Locator;
use settings::RelativeImportsOrder; use settings::Settings;
use sorting::cmp_either_import; use sorting::cmp_either_import;
use types::EitherImport::{Import, ImportFrom}; use types::EitherImport::{Import, ImportFrom};
use types::{AliasData, EitherImport, TrailingComma}; use types::{AliasData, EitherImport, ImportBlock, TrailingComma};
use crate::line_width::{LineLength, LineWidthBuilder}; use crate::line_width::{LineLength, LineWidthBuilder};
use crate::rules::isort::categorize::KnownModules;
use crate::rules::isort::types::ImportBlock;
use crate::settings::types::PythonVersion; use crate::settings::types::PythonVersion;
mod annotate; mod annotate;
@ -74,49 +71,25 @@ pub(crate) fn format_imports(
src: &[PathBuf], src: &[PathBuf],
package: Option<&Path>, package: Option<&Path>,
source_type: PySourceType, source_type: PySourceType,
combine_as_imports: bool,
force_single_line: bool,
force_sort_within_sections: bool,
case_sensitive: bool,
force_wrap_aliases: bool,
force_to_top: &BTreeSet<String>,
known_modules: &KnownModules,
order_by_type: bool,
detect_same_package: bool,
relative_imports_order: RelativeImportsOrder,
single_line_exclusions: &BTreeSet<String>,
split_on_trailing_comma: bool,
classes: &BTreeSet<String>,
constants: &BTreeSet<String>,
variables: &BTreeSet<String>,
no_lines_before: &BTreeSet<ImportSection>,
lines_after_imports: isize,
lines_between_types: usize,
forced_separate: &[String],
target_version: PythonVersion, target_version: PythonVersion,
section_order: &[ImportSection], settings: &Settings,
) -> String { ) -> String {
let trailer = &block.trailer; let trailer = &block.trailer;
let block = annotate_imports( let block = annotate_imports(
&block.imports, &block.imports,
comments, comments,
locator, locator,
split_on_trailing_comma, settings.split_on_trailing_comma,
source_type, source_type,
); );
// Normalize imports (i.e., deduplicate, aggregate `from` imports). // Normalize imports (i.e., deduplicate, aggregate `from` imports).
let block = normalize_imports( let block = normalize_imports(block, settings);
block,
combine_as_imports,
force_single_line,
single_line_exclusions,
);
// Categorize imports. // Categorize imports.
let mut output = String::new(); let mut output = String::new();
for block in split::split_by_forced_separate(block, forced_separate) { for block in split::split_by_forced_separate(block, &settings.forced_separate) {
let block_output = format_import_block( let block_output = format_import_block(
block, block,
line_length, line_length,
@ -124,22 +97,8 @@ pub(crate) fn format_imports(
stylist, stylist,
src, src,
package, package,
force_sort_within_sections,
case_sensitive,
force_wrap_aliases,
force_to_top,
known_modules,
order_by_type,
detect_same_package,
relative_imports_order,
split_on_trailing_comma,
classes,
constants,
variables,
no_lines_before,
lines_between_types,
target_version, target_version,
section_order, settings,
); );
if !block_output.is_empty() && !output.is_empty() { if !block_output.is_empty() && !output.is_empty() {
@ -150,6 +109,7 @@ pub(crate) fn format_imports(
output.push_str(block_output.as_str()); output.push_str(block_output.as_str());
} }
let lines_after_imports = settings.lines_after_imports;
match trailer { match trailer {
None => {} None => {}
Some(Trailer::Sibling) => { Some(Trailer::Sibling) => {
@ -183,30 +143,16 @@ fn format_import_block(
stylist: &Stylist, stylist: &Stylist,
src: &[PathBuf], src: &[PathBuf],
package: Option<&Path>, package: Option<&Path>,
force_sort_within_sections: bool,
case_sensitive: bool,
force_wrap_aliases: bool,
force_to_top: &BTreeSet<String>,
known_modules: &KnownModules,
order_by_type: bool,
detect_same_package: bool,
relative_imports_order: RelativeImportsOrder,
split_on_trailing_comma: bool,
classes: &BTreeSet<String>,
constants: &BTreeSet<String>,
variables: &BTreeSet<String>,
no_lines_before: &BTreeSet<ImportSection>,
lines_between_types: usize,
target_version: PythonVersion, target_version: PythonVersion,
section_order: &[ImportSection], settings: &Settings,
) -> String { ) -> String {
// Categorize by type (e.g., first-party vs. third-party). // Categorize by type (e.g., first-party vs. third-party).
let mut block_by_type = categorize_imports( let mut block_by_type = categorize_imports(
block, block,
src, src,
package, package,
detect_same_package, settings.detect_same_package,
known_modules, &settings.known_modules,
target_version, target_version,
); );
@ -215,26 +161,17 @@ fn format_import_block(
// Generate replacement source code. // Generate replacement source code.
let mut is_first_block = true; let mut is_first_block = true;
let mut pending_lines_before = false; let mut pending_lines_before = false;
for import_section in section_order { for import_section in &settings.section_order {
let import_block = block_by_type.remove(import_section); let import_block = block_by_type.remove(import_section);
if !no_lines_before.contains(import_section) { if !settings.no_lines_before.contains(import_section) {
pending_lines_before = true; pending_lines_before = true;
} }
let Some(import_block) = import_block else { let Some(import_block) = import_block else {
continue; continue;
}; };
let imports = order_imports( let imports = order_imports(import_block, settings);
import_block,
order_by_type,
case_sensitive,
relative_imports_order,
classes,
constants,
variables,
force_to_top,
);
let imports = { let imports = {
let mut imports = imports let mut imports = imports
@ -243,16 +180,8 @@ fn format_import_block(
.map(Import) .map(Import)
.chain(imports.import_from.into_iter().map(ImportFrom)) .chain(imports.import_from.into_iter().map(ImportFrom))
.collect::<Vec<EitherImport>>(); .collect::<Vec<EitherImport>>();
if force_sort_within_sections { if settings.force_sort_within_sections {
imports.sort_by(|import1, import2| { imports.sort_by(|import1, import2| cmp_either_import(import1, import2, settings));
cmp_either_import(
import1,
import2,
relative_imports_order,
force_to_top,
case_sensitive,
)
});
}; };
imports imports
}; };
@ -269,6 +198,7 @@ fn format_import_block(
let mut lines_inserted = false; let mut lines_inserted = false;
let mut has_direct_import = false; let mut has_direct_import = false;
let mut is_first_statement = true; let mut is_first_statement = true;
let lines_between_types = settings.lines_between_types;
for import in imports { for import in imports {
match import { match import {
Import((alias, comments)) => { Import((alias, comments)) => {
@ -299,9 +229,10 @@ fn format_import_block(
line_length, line_length,
indentation_width, indentation_width,
stylist, stylist,
force_wrap_aliases, settings.force_wrap_aliases,
is_first_statement, is_first_statement,
split_on_trailing_comma && matches!(trailing_comma, TrailingComma::Present), settings.split_on_trailing_comma
&& matches!(trailing_comma, TrailingComma::Present),
)); ));
} }
} }

23
crates/ruff_linter/src/rules/isort/normalize.rs
View file

@ -1,15 +1,10 @@
use std::collections::BTreeSet; use super::settings::Settings;
use super::types::{AliasData, ImportBlock, ImportFromData, TrailingComma};
use crate::rules::isort::types::TrailingComma;
use super::types::{AliasData, ImportBlock, ImportFromData};
use super::AnnotatedImport; use super::AnnotatedImport;
pub(crate) fn normalize_imports<'a>( pub(crate) fn normalize_imports<'a>(
imports: Vec<AnnotatedImport<'a>>, imports: Vec<AnnotatedImport<'a>>,
combine_as_imports: bool, settings: &Settings,
force_single_line: bool,
single_line_exclusions: &'a BTreeSet<String>,
) -> ImportBlock<'a> { ) -> ImportBlock<'a> {
let mut block = ImportBlock::default(); let mut block = ImportBlock::default();
for import in imports { for import in imports {
@ -56,8 +51,10 @@ pub(crate) fn normalize_imports<'a>(
trailing_comma, trailing_comma,
} => { } => {
// Whether to track each member of the import as a separate entry. // Whether to track each member of the import as a separate entry.
let isolate_aliases = force_single_line let isolate_aliases = settings.force_single_line
&& module.map_or(true, |module| !single_line_exclusions.contains(module)) && module.map_or(true, |module| {
!settings.single_line_exclusions.contains(module)
})
&& !names.first().is_some_and(|alias| alias.name == "*"); && !names.first().is_some_and(|alias| alias.name == "*");
// Insert comments on the statement itself. // Insert comments on the statement itself.
@ -95,7 +92,7 @@ pub(crate) fn normalize_imports<'a>(
.import_from_star .import_from_star
.entry(ImportFromData { module, level }) .entry(ImportFromData { module, level })
.or_default() .or_default()
} else if alias.asname.is_none() || combine_as_imports { } else if alias.asname.is_none() || settings.combine_as_imports {
block block
.import_from .import_from
.entry(ImportFromData { module, level }) .entry(ImportFromData { module, level })
@ -130,7 +127,9 @@ pub(crate) fn normalize_imports<'a>(
.import_from_star .import_from_star
.entry(ImportFromData { module, level }) .entry(ImportFromData { module, level })
.or_default() .or_default()
} else if !isolate_aliases && (alias.asname.is_none() || combine_as_imports) { } else if !isolate_aliases
&& (alias.asname.is_none() || settings.combine_as_imports)
{
block block
.import_from .import_from
.entry(ImportFromData { module, level }) .entry(ImportFromData { module, level })

61
crates/ruff_linter/src/rules/isort/order.rs
View file

@ -1,24 +1,14 @@
use std::cmp::Ordering; use std::cmp::Ordering;
use std::collections::BTreeSet;
use itertools::Itertools; use itertools::Itertools;
use crate::rules::isort::types::ImportFromStatement; use super::settings::Settings;
use super::settings::RelativeImportsOrder;
use super::sorting::{cmp_import_from, cmp_members, cmp_modules}; use super::sorting::{cmp_import_from, cmp_members, cmp_modules};
use super::types::{AliasData, CommentSet, ImportBlock, OrderedImportBlock}; use super::types::{AliasData, CommentSet, ImportBlock, ImportFromStatement, OrderedImportBlock};
#[allow(clippy::too_many_arguments)]
pub(crate) fn order_imports<'a>( pub(crate) fn order_imports<'a>(
block: ImportBlock<'a>, block: ImportBlock<'a>,
order_by_type: bool, settings: &Settings,
case_sensitive: bool,
relative_imports_order: RelativeImportsOrder,
classes: &'a BTreeSet<String>,
constants: &'a BTreeSet<String>,
variables: &'a BTreeSet<String>,
force_to_top: &'a BTreeSet<String>,
) -> OrderedImportBlock<'a> { ) -> OrderedImportBlock<'a> {
let mut ordered = OrderedImportBlock::default(); let mut ordered = OrderedImportBlock::default();
@ -27,9 +17,7 @@ pub(crate) fn order_imports<'a>(
block block
.import .import
.into_iter() .into_iter()
.sorted_by(|(alias1, _), (alias2, _)| { .sorted_by(|(alias1, _), (alias2, _)| cmp_modules(alias1, alias2, settings)),
cmp_modules(alias1, alias2, force_to_top, case_sensitive)
}),
); );
// Sort `Stmt::ImportFrom`. // Sort `Stmt::ImportFrom`.
@ -66,16 +54,7 @@ pub(crate) fn order_imports<'a>(
aliases aliases
.into_iter() .into_iter()
.sorted_by(|(alias1, _), (alias2, _)| { .sorted_by(|(alias1, _), (alias2, _)| {
cmp_members( cmp_members(alias1, alias2, settings)
alias1,
alias2,
order_by_type,
classes,
constants,
variables,
force_to_top,
case_sensitive,
)
}) })
.collect::<Vec<(AliasData, CommentSet)>>(), .collect::<Vec<(AliasData, CommentSet)>>(),
) )
@ -83,27 +62,15 @@ pub(crate) fn order_imports<'a>(
) )
.sorted_by( .sorted_by(
|(import_from1, _, _, aliases1), (import_from2, _, _, aliases2)| { |(import_from1, _, _, aliases1), (import_from2, _, _, aliases2)| {
cmp_import_from( cmp_import_from(import_from1, import_from2, settings).then_with(|| {
import_from1, match (aliases1.first(), aliases2.first()) {
import_from2, (None, None) => Ordering::Equal,
relative_imports_order, (None, Some(_)) => Ordering::Less,
force_to_top, (Some(_), None) => Ordering::Greater,
case_sensitive, (Some((alias1, _)), Some((alias2, _))) => {
) cmp_members(alias1, alias2, settings)
.then_with(|| match (aliases1.first(), aliases2.first()) { }
(None, None) => Ordering::Equal, }
(None, Some(_)) => Ordering::Less,
(Some(_), None) => Ordering::Greater,
(Some((alias1, _)), Some((alias2, _))) => cmp_members(
alias1,
alias2,
order_by_type,
classes,
constants,
variables,
force_to_top,
case_sensitive,
),
}) })
}, },
), ),

21
crates/ruff_linter/src/rules/isort/rules/organize_imports.rs
View file

@ -126,27 +126,8 @@ pub(crate) fn organize_imports(
&settings.src, &settings.src,
package, package,
source_type, source_type,
settings.isort.combine_as_imports,
settings.isort.force_single_line,
settings.isort.force_sort_within_sections,
settings.isort.case_sensitive,
settings.isort.force_wrap_aliases,
&settings.isort.force_to_top,
&settings.isort.known_modules,
settings.isort.order_by_type,
settings.isort.detect_same_package,
settings.isort.relative_imports_order,
&settings.isort.single_line_exclusions,
settings.isort.split_on_trailing_comma,
&settings.isort.classes,
&settings.isort.constants,
&settings.isort.variables,
&settings.isort.no_lines_before,
settings.isort.lines_after_imports,
settings.isort.lines_between_types,
&settings.isort.forced_separate,
settings.target_version, settings.target_version,
&settings.isort.section_order, &settings.isort,
); );
// Expand the span the entire range, including leading and trailing space. // Expand the span the entire range, including leading and trailing space.

96
crates/ruff_linter/src/rules/isort/sorting.rs
View file

@ -4,11 +4,9 @@ use std::collections::BTreeSet;
use ruff_python_stdlib::str; use ruff_python_stdlib::str;
use crate::rules::isort::types::Importable; use super::settings::{RelativeImportsOrder, Settings};
use super::settings::RelativeImportsOrder;
use super::types::EitherImport::{Import, ImportFrom}; use super::types::EitherImport::{Import, ImportFrom};
use super::types::{AliasData, EitherImport, ImportFromData}; use super::types::{AliasData, EitherImport, ImportFromData, Importable};
#[derive(PartialOrd, Ord, PartialEq, Eq, Copy, Clone)] #[derive(PartialOrd, Ord, PartialEq, Eq, Copy, Clone)]
pub(crate) enum Prefix { pub(crate) enum Prefix {
@ -17,19 +15,14 @@ pub(crate) enum Prefix {
Variables, Variables,
} }
fn prefix( fn prefix(name: &str, settings: &Settings) -> Prefix {
name: &str, if settings.constants.contains(name) {
classes: &BTreeSet<String>,
constants: &BTreeSet<String>,
variables: &BTreeSet<String>,
) -> Prefix {
if constants.contains(name) {
// Ex) `CONSTANT` // Ex) `CONSTANT`
Prefix::Constants Prefix::Constants
} else if classes.contains(name) { } else if settings.classes.contains(name) {
// Ex) `CLASS` // Ex) `CLASS`
Prefix::Classes Prefix::Classes
} else if variables.contains(name) { } else if settings.variables.contains(name) {
// Ex) `variable` // Ex) `variable`
Prefix::Variables Prefix::Variables
} else if name.len() > 1 && str::is_cased_uppercase(name) { } else if name.len() > 1 && str::is_cased_uppercase(name) {
@ -52,15 +45,10 @@ fn cmp_force_to_top(name1: &str, name2: &str, force_to_top: &BTreeSet<String>) -
} }
/// Compare two top-level modules. /// Compare two top-level modules.
pub(crate) fn cmp_modules( pub(crate) fn cmp_modules(alias1: &AliasData, alias2: &AliasData, settings: &Settings) -> Ordering {
alias1: &AliasData, cmp_force_to_top(alias1.name, alias2.name, &settings.force_to_top)
alias2: &AliasData,
force_to_top: &BTreeSet<String>,
case_sensitive: bool,
) -> Ordering {
cmp_force_to_top(alias1.name, alias2.name, force_to_top)
.then_with(|| { .then_with(|| {
if case_sensitive { if settings.case_sensitive {
natord::compare(alias1.name, alias2.name) natord::compare(alias1.name, alias2.name)
} else { } else {
natord::compare_ignore_case(alias1.name, alias2.name) natord::compare_ignore_case(alias1.name, alias2.name)
@ -76,27 +64,17 @@ pub(crate) fn cmp_modules(
} }
/// Compare two member imports within `Stmt::ImportFrom` blocks. /// Compare two member imports within `Stmt::ImportFrom` blocks.
#[allow(clippy::too_many_arguments)] pub(crate) fn cmp_members(alias1: &AliasData, alias2: &AliasData, settings: &Settings) -> Ordering {
pub(crate) fn cmp_members(
alias1: &AliasData,
alias2: &AliasData,
order_by_type: bool,
classes: &BTreeSet<String>,
constants: &BTreeSet<String>,
variables: &BTreeSet<String>,
force_to_top: &BTreeSet<String>,
case_sensitive: bool,
) -> Ordering {
match (alias1.name == "*", alias2.name == "*") { match (alias1.name == "*", alias2.name == "*") {
(true, false) => Ordering::Less, (true, false) => Ordering::Less,
(false, true) => Ordering::Greater, (false, true) => Ordering::Greater,
_ => { _ => {
if order_by_type { if settings.order_by_type {
prefix(alias1.name, classes, constants, variables) prefix(alias1.name, settings)
.cmp(&prefix(alias2.name, classes, constants, variables)) .cmp(&prefix(alias2.name, settings))
.then_with(|| cmp_modules(alias1, alias2, force_to_top, case_sensitive)) .then_with(|| cmp_modules(alias1, alias2, settings))
} else { } else {
cmp_modules(alias1, alias2, force_to_top, case_sensitive) cmp_modules(alias1, alias2, settings)
} }
} }
} }
@ -123,20 +101,18 @@ pub(crate) fn cmp_levels(
pub(crate) fn cmp_import_from( pub(crate) fn cmp_import_from(
import_from1: &ImportFromData, import_from1: &ImportFromData,
import_from2: &ImportFromData, import_from2: &ImportFromData,
relative_imports_order: RelativeImportsOrder, settings: &Settings,
force_to_top: &BTreeSet<String>,
case_sensitive: bool,
) -> Ordering { ) -> Ordering {
cmp_levels( cmp_levels(
import_from1.level, import_from1.level,
import_from2.level, import_from2.level,
relative_imports_order, settings.relative_imports_order,
) )
.then_with(|| { .then_with(|| {
cmp_force_to_top( cmp_force_to_top(
&import_from1.module_name(), &import_from1.module_name(),
&import_from2.module_name(), &import_from2.module_name(),
force_to_top, &settings.force_to_top,
) )
}) })
.then_with(|| match (&import_from1.module, import_from2.module) { .then_with(|| match (&import_from1.module, import_from2.module) {
@ -144,7 +120,7 @@ pub(crate) fn cmp_import_from(
(None, Some(_)) => Ordering::Less, (None, Some(_)) => Ordering::Less,
(Some(_), None) => Ordering::Greater, (Some(_), None) => Ordering::Greater,
(Some(module1), Some(module2)) => { (Some(module1), Some(module2)) => {
if case_sensitive { if settings.case_sensitive {
natord::compare(module1, module2) natord::compare(module1, module2)
} else { } else {
natord::compare_ignore_case(module1, module2) natord::compare_ignore_case(module1, module2)
@ -157,11 +133,15 @@ pub(crate) fn cmp_import_from(
fn cmp_import_import_from( fn cmp_import_import_from(
import: &AliasData, import: &AliasData,
import_from: &ImportFromData, import_from: &ImportFromData,
force_to_top: &BTreeSet<String>, settings: &Settings,
case_sensitive: bool,
) -> Ordering { ) -> Ordering {
cmp_force_to_top(import.name, &import_from.module_name(), force_to_top).then_with(|| { cmp_force_to_top(
if case_sensitive { import.name,
&import_from.module_name(),
&settings.force_to_top,
)
.then_with(|| {
if settings.case_sensitive {
natord::compare(import.name, import_from.module.unwrap_or_default()) natord::compare(import.name, import_from.module.unwrap_or_default())
} else { } else {
natord::compare_ignore_case(import.name, import_from.module.unwrap_or_default()) natord::compare_ignore_case(import.name, import_from.module.unwrap_or_default())
@ -174,26 +154,18 @@ fn cmp_import_import_from(
pub(crate) fn cmp_either_import( pub(crate) fn cmp_either_import(
a: &EitherImport, a: &EitherImport,
b: &EitherImport, b: &EitherImport,
relative_imports_order: RelativeImportsOrder, settings: &Settings,
force_to_top: &BTreeSet<String>,
case_sensitive: bool,
) -> Ordering { ) -> Ordering {
match (a, b) { match (a, b) {
(Import((alias1, _)), Import((alias2, _))) => { (Import((alias1, _)), Import((alias2, _))) => cmp_modules(alias1, alias2, settings),
cmp_modules(alias1, alias2, force_to_top, case_sensitive)
}
(ImportFrom((import_from, ..)), Import((alias, _))) => { (ImportFrom((import_from, ..)), Import((alias, _))) => {
cmp_import_import_from(alias, import_from, force_to_top, case_sensitive).reverse() cmp_import_import_from(alias, import_from, settings).reverse()
} }
(Import((alias, _)), ImportFrom((import_from, ..))) => { (Import((alias, _)), ImportFrom((import_from, ..))) => {
cmp_import_import_from(alias, import_from, force_to_top, case_sensitive) cmp_import_import_from(alias, import_from, settings)
}
(ImportFrom((import_from1, ..)), ImportFrom((import_from2, ..))) => {
cmp_import_from(import_from1, import_from2, settings)
} }
(ImportFrom((import_from1, ..)), ImportFrom((import_from2, ..))) => cmp_import_from(
import_from1,
import_from2,
relative_imports_order,
force_to_top,
case_sensitive,
),
} }
} }