From 6f2bfd3344ae7321409ca1829b2ba907007fd05e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 5 Nov 2022 23:29:20 -0400 Subject: [PATCH 01/10] Implement import sorting --- src/check_ast.rs | 13 ++++++ src/import_tracking.rs | 102 +++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + test/foo.py | 41 +++++++++++++++++ test/local.py | 13 ++++++ test/local_from.py | 0 test/pyproject.toml | 3 ++ 7 files changed, 173 insertions(+) create mode 100644 src/import_tracking.rs create mode 100644 test/foo.py create mode 100644 test/local.py create mode 100644 test/local_from.py create mode 100644 test/pyproject.toml diff --git a/src/check_ast.rs b/src/check_ast.rs index 5860664922188..266edf8d6d693 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -23,6 +23,7 @@ use crate::ast::{helpers, operations, visitor}; use crate::autofix::fixer; use crate::checks::{Check, CheckCode, CheckKind}; use crate::docstrings::definition::{Definition, DefinitionKind, Documentable}; +use crate::import_tracking::{normalize, ImportTracker}; use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS}; use crate::python::future::ALL_FEATURE_NAMES; use crate::python::typing; @@ -77,6 +78,7 @@ pub struct Checker<'a> { deferred_functions: Vec<(&'a Stmt, Vec, Vec, VisibleScope)>, deferred_lambdas: Vec<(&'a Expr, Vec, Vec)>, deferred_assignments: Vec, + import_tracker: ImportTracker<'a>, // Internal, derivative state. visible_scope: VisibleScope, in_f_string: Option, @@ -115,6 +117,8 @@ impl<'a> Checker<'a> { deferred_functions: Default::default(), deferred_lambdas: Default::default(), deferred_assignments: Default::default(), + import_tracker: ImportTracker::new(), + // Internal, derivative state. visible_scope: VisibleScope { modifier: Modifier::Module, visibility: module_visibility(path), @@ -209,6 +213,9 @@ where } } + // Track all import blocks (to power import sorting). + self.import_tracker.visit_stmt(stmt); + // Pre-visit. match &stmt.node { StmtKind::Global { names } | StmtKind::Nonlocal { names } => { @@ -2591,5 +2598,11 @@ pub fn check_ast( // Check docstrings. checker.check_definitions(); + for block in checker.import_tracker.blocks { + if !block.is_empty() { + println!("{:?}", normalize(block)); + } + } + checker.checks } diff --git a/src/import_tracking.rs b/src/import_tracking.rs new file mode 100644 index 0000000000000..99c2d856955d9 --- /dev/null +++ b/src/import_tracking.rs @@ -0,0 +1,102 @@ +use std::collections::{BTreeMap, BTreeSet}; + +use rustpython_ast::{Stmt, StmtKind}; + +use crate::autofix::{Fix, Patch}; + +#[derive(Debug)] +pub struct ImportTracker<'a> { + pub blocks: Vec>, +} + +impl<'a> ImportTracker<'a> { + pub fn new() -> Self { + Self { + blocks: vec![vec![]], + } + } + + pub fn visit_stmt(&mut self, stmt: &'a Stmt) { + let index = self.blocks.len() - 1; + if matches!( + stmt.node, + StmtKind::Import { .. } | StmtKind::ImportFrom { .. } + ) { + self.blocks[index].push(stmt); + } else { + if !self.blocks[index].is_empty() { + self.blocks.push(vec![]); + } + } + } +} + +type FromData<'a> = (&'a Option, &'a Option); +type AliasData<'a> = (&'a str, &'a Option); + +#[derive(Debug, Default)] +pub struct ImportBlock<'a> { + // Map from (module, level) to `AliasData`. + import_from: BTreeMap, BTreeSet>>, + // Set of (name, asname). + import: BTreeSet>, +} + +enum ImportType { + // __future__ + Future, + // Known standard library + StandardLibrary, + // Doesn't fit into any other category + ThirdParty, + // Local modules (but non-dotted) + FirstParty, + // Dot imports + LocalFolder, +} + +impl ImportType { + fn categorize(module: &str) -> Self { + ImportType::FirstParty + } +} + +pub fn normalize(imports: Vec<&Stmt>) -> ImportBlock { + let mut block: ImportBlock = Default::default(); + for import in imports { + match &import.node { + StmtKind::Import { names } => { + for name in names { + block.import.insert((&name.node.name, &name.node.asname)); + } + } + StmtKind::ImportFrom { + module, + names, + level, + } => { + let targets = block.import_from.entry((module, level)).or_default(); + for name in names { + targets.insert((&name.node.name, &name.node.asname)); + } + } + _ => unreachable!("Expected StmtKind::Import | StmtKind::ImportFrom"), + } + } + block +} + +fn sort_block(block: Vec<&Stmt>) -> Fix { + // Categorize each module as: __future__, standard library, third-party, first-party. + // Deduplicate. + // Consolidate `from` imports under the same module. + + Fix { + patch: Patch { + content: "".to_string(), + location: Default::default(), + end_location: Default::default(), + }, + applied: false, + } +} diff --git a/src/lib.rs b/src/lib.rs index 67466370bbe63..ab23f56c753fd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,6 +32,7 @@ mod flake8_comprehensions; mod flake8_print; pub mod flake8_quotes; pub mod fs; +mod import_tracking; mod lex; pub mod linter; pub mod logging; diff --git a/test/foo.py b/test/foo.py new file mode 100644 index 0000000000000..f6f6669ec1dc8 --- /dev/null +++ b/test/foo.py @@ -0,0 +1,41 @@ +import local +from local_from import ( + import1, + import2, + import3, + import4, + import5, + import6, + import7, + import8, + import9, +) +import __future__.bar +from __future__ import annotations +# Comment 1 +import os +import baz.bar +import baz.foo + +# Comment 2 + +# Comment 3 (line 1) +# Comment 3 (line 2) +from bop import bar # Comment 4 +from bop import foo # Comment 5 +from boo import ( + import1, + import2, + import3, + import4, + import5, + import6, + import7, + import8, + import9, +) + +x = 1 + +import collections +import typing diff --git a/test/local.py b/test/local.py new file mode 100644 index 0000000000000..0f0b77ef8e835 --- /dev/null +++ b/test/local.py @@ -0,0 +1,13 @@ +import foo as bar +import foo as baz +import foo.f1 as g1 +import foo.f1 as g2 +import __future__.bar as foo +import __future__.baz +import __future__.bar as foo +import __future__.baz +import wop +from . import bar +from .boop import bar +from wop import wop +import foo, bar diff --git a/test/local_from.py b/test/local_from.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/test/pyproject.toml b/test/pyproject.toml new file mode 100644 index 0000000000000..c15a9bcfd201b --- /dev/null +++ b/test/pyproject.toml @@ -0,0 +1,3 @@ +[tool.isort] +line_length = 88 +profile = 'black' From d291ae15164e39410dcd895eae943ffc6e2f25d1 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 8 Nov 2022 22:08:26 -0500 Subject: [PATCH 02/10] Implement basic formatting --- bar.py | 4 + foo.py | 3 + src/check_ast.rs | 31 ++-- src/checks.rs | 17 ++- src/checks_gen.rs | 12 ++ src/docstrings/helpers.rs | 8 +- src/import_tracking.rs | 102 ------------- src/imports/categorize.rs | 35 +++++ src/imports/mod.rs | 135 ++++++++++++++++ src/imports/plugins.rs | 156 +++++++++++++++++++ src/imports/track.rs | 28 ++++ src/imports/types.rs | 13 ++ src/lib.rs | 2 +- src/python/mod.rs | 1 + src/python/sys.rs | 313 ++++++++++++++++++++++++++++++++++++++ test/local.py | 17 +++ 16 files changed, 759 insertions(+), 118 deletions(-) create mode 100644 bar.py create mode 100644 foo.py delete mode 100644 src/import_tracking.rs create mode 100644 src/imports/categorize.rs create mode 100644 src/imports/mod.rs create mode 100644 src/imports/plugins.rs create mode 100644 src/imports/track.rs create mode 100644 src/imports/types.rs create mode 100644 src/python/sys.rs diff --git a/bar.py b/bar.py new file mode 100644 index 0000000000000..955b01ce987b5 --- /dev/null +++ b/bar.py @@ -0,0 +1,4 @@ +try: + import sys +except: + import os diff --git a/foo.py b/foo.py new file mode 100644 index 0000000000000..cd000d8e084b4 --- /dev/null +++ b/foo.py @@ -0,0 +1,3 @@ +if True: + x = 1; import os; import sys + import bar; import baz; y = 1; diff --git a/src/check_ast.rs b/src/check_ast.rs index 266edf8d6d693..a4ae2dacf3f27 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -23,7 +23,7 @@ use crate::ast::{helpers, operations, visitor}; use crate::autofix::fixer; use crate::checks::{Check, CheckCode, CheckKind}; use crate::docstrings::definition::{Definition, DefinitionKind, Documentable}; -use crate::import_tracking::{normalize, ImportTracker}; +use crate::imports::track::ImportTracker; use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS}; use crate::python::future::ALL_FEATURE_NAMES; use crate::python::typing; @@ -34,7 +34,7 @@ use crate::source_code_locator::SourceCodeLocator; use crate::visibility::{module_visibility, transition_scope, Modifier, Visibility, VisibleScope}; use crate::{ docstrings, flake8_annotations, flake8_bugbear, flake8_builtins, flake8_comprehensions, - flake8_print, pep8_naming, pycodestyle, pydocstyle, pyflakes, pyupgrade, + flake8_print, imports, pep8_naming, pycodestyle, pydocstyle, pyflakes, pyupgrade, }; const GLOBAL_SCOPE_INDEX: usize = 0; @@ -187,6 +187,11 @@ where fn visit_stmt(&mut self, stmt: &'b Stmt) { self.push_parent(stmt); + // Track all import blocks (to power import sorting). + // TODO(charlie): This doesn't work with exception handlers. Maybe we should + // have some more general clearing mechanism? + self.import_tracker.visit_stmt(stmt); + // Track whether we've seen docstrings, non-imports, etc. match &stmt.node { StmtKind::ImportFrom { module, .. } => { @@ -213,9 +218,6 @@ where } } - // Track all import blocks (to power import sorting). - self.import_tracker.visit_stmt(stmt); - // Pre-visit. match &stmt.node { StmtKind::Global { names } | StmtKind::Nonlocal { names } => { @@ -2421,6 +2423,18 @@ impl<'a> Checker<'a> { self.add_checks(checks.into_iter()); } + fn check_import_blocks(&mut self) { + if !self.settings.enabled.contains(&CheckCode::I001) { + return; + } + + while let Some(block) = self.import_tracker.blocks.pop() { + if !block.is_empty() { + imports::plugins::check_imports(self, block); + } + } + } + fn check_definitions(&mut self) { while let Some((definition, visibility)) = self.definitions.pop() { // flake8-annotations @@ -2598,11 +2612,8 @@ pub fn check_ast( // Check docstrings. checker.check_definitions(); - for block in checker.import_tracker.blocks { - if !block.is_empty() { - println!("{:?}", normalize(block)); - } - } + // Check import blocks. + checker.check_import_blocks(); checker.checks } diff --git a/src/checks.rs b/src/checks.rs index 4a88b68da843a..640a0dbe92e92 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -204,6 +204,8 @@ pub enum CheckCode { N816, N817, N818, + // isort + I001, // Ruff RUF001, RUF002, @@ -225,6 +227,7 @@ pub enum CheckCategory { Flake8Print, Flake8Quotes, Flake8Annotations, + ISort, Ruff, Meta, } @@ -243,6 +246,7 @@ impl CheckCategory { CheckCategory::Pyupgrade => "pyupgrade", CheckCategory::Pydocstyle => "pydocstyle", CheckCategory::PEP8Naming => "pep8-naming", + CheckCategory::ISort => "isort", CheckCategory::Ruff => "Ruff-specific rules", CheckCategory::Meta => "Meta rules", } @@ -269,6 +273,7 @@ impl CheckCategory { CheckCategory::Pyupgrade => Some("https://pypi.org/project/pyupgrade/3.2.0/"), CheckCategory::Pydocstyle => Some("https://pypi.org/project/pydocstyle/6.1.1/"), CheckCategory::PEP8Naming => Some("https://pypi.org/project/pep8-naming/0.13.2/"), + CheckCategory::ISort => Some("https://pypi.org/project/isort/5.10.1/"), CheckCategory::Ruff => None, CheckCategory::Meta => None, } @@ -470,6 +475,8 @@ pub enum CheckKind { MixedCaseVariableInGlobalScope(String), CamelcaseImportedAsAcronym(String, String), ErrorSuffixOnExceptionName(String), + // isort + UnsortedImports, // Ruff AmbiguousUnicodeCharacterString(char, char), AmbiguousUnicodeCharacterDocstring(char, char), @@ -717,6 +724,8 @@ impl CheckCode { CheckKind::CamelcaseImportedAsAcronym("...".to_string(), "...".to_string()) } CheckCode::N818 => CheckKind::ErrorSuffixOnExceptionName("...".to_string()), + // isort + CheckCode::I001 => CheckKind::UnsortedImports, // Ruff CheckCode::RUF001 => CheckKind::AmbiguousUnicodeCharacterString('𝐁', 'B'), CheckCode::RUF002 => CheckKind::AmbiguousUnicodeCharacterDocstring('𝐁', 'B'), @@ -895,6 +904,7 @@ impl CheckCode { CheckCode::N816 => CheckCategory::PEP8Naming, CheckCode::N817 => CheckCategory::PEP8Naming, CheckCode::N818 => CheckCategory::PEP8Naming, + CheckCode::I001 => CheckCategory::ISort, CheckCode::RUF001 => CheckCategory::Ruff, CheckCode::RUF002 => CheckCategory::Ruff, CheckCode::RUF003 => CheckCategory::Ruff, @@ -1085,6 +1095,8 @@ impl CheckKind { CheckKind::MixedCaseVariableInGlobalScope(..) => &CheckCode::N816, CheckKind::CamelcaseImportedAsAcronym(..) => &CheckCode::N817, CheckKind::ErrorSuffixOnExceptionName(..) => &CheckCode::N818, + // isort + CheckKind::UnsortedImports => &CheckCode::I001, // Ruff CheckKind::AmbiguousUnicodeCharacterString(..) => &CheckCode::RUF001, CheckKind::AmbiguousUnicodeCharacterDocstring(..) => &CheckCode::RUF002, @@ -1644,6 +1656,8 @@ impl CheckKind { CheckKind::PEP3120UnnecessaryCodingComment => { "utf-8 encoding declaration is unnecessary".to_string() } + // isort + CheckKind::UnsortedImports => "Import block is un-sorted or un-formatted".to_string(), // Ruff CheckKind::AmbiguousUnicodeCharacterString(confusable, representant) => { format!( @@ -1749,12 +1763,13 @@ impl CheckKind { | CheckKind::UnnecessaryGeneratorSet | CheckKind::UnnecessaryLRUCacheParams | CheckKind::UnnecessaryListCall - | CheckKind::UnnecessaryListComprehensionSet | CheckKind::UnnecessaryListComprehensionDict + | CheckKind::UnnecessaryListComprehensionSet | CheckKind::UnnecessaryLiteralDict(_) | CheckKind::UnnecessaryLiteralSet(_) | CheckKind::UnnecessaryLiteralWithinListCall(_) | CheckKind::UnnecessaryLiteralWithinTupleCall(_) + | CheckKind::UnsortedImports | CheckKind::UnusedImport(_, false) | CheckKind::UnusedLoopControlVariable(_) | CheckKind::UnusedNOQA(_) diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 81e92849e54be..a37154b9dddfa 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -203,6 +203,10 @@ pub enum CheckCodePrefix { F9, F90, F901, + I, + I0, + I00, + I001, M, M0, M00, @@ -852,6 +856,10 @@ impl CheckCodePrefix { CheckCodePrefix::F9 => vec![CheckCode::F901], CheckCodePrefix::F90 => vec![CheckCode::F901], CheckCodePrefix::F901 => vec![CheckCode::F901], + CheckCodePrefix::I => vec![CheckCode::I001], + CheckCodePrefix::I0 => vec![CheckCode::I001], + CheckCodePrefix::I00 => vec![CheckCode::I001], + CheckCodePrefix::I001 => vec![CheckCode::I001], CheckCodePrefix::M => vec![CheckCode::M001], CheckCodePrefix::M0 => vec![CheckCode::M001], CheckCodePrefix::M00 => vec![CheckCode::M001], @@ -1216,6 +1224,10 @@ impl CheckCodePrefix { CheckCodePrefix::F9 => PrefixSpecificity::Hundreds, CheckCodePrefix::F90 => PrefixSpecificity::Tens, CheckCodePrefix::F901 => PrefixSpecificity::Explicit, + CheckCodePrefix::I => PrefixSpecificity::Category, + CheckCodePrefix::I0 => PrefixSpecificity::Hundreds, + CheckCodePrefix::I00 => PrefixSpecificity::Tens, + CheckCodePrefix::I001 => PrefixSpecificity::Explicit, CheckCodePrefix::M => PrefixSpecificity::Category, CheckCodePrefix::M0 => PrefixSpecificity::Hundreds, CheckCodePrefix::M00 => PrefixSpecificity::Tens, diff --git a/src/docstrings/helpers.rs b/src/docstrings/helpers.rs index 34a8c02a9745c..8249ede6856b2 100644 --- a/src/docstrings/helpers.rs +++ b/src/docstrings/helpers.rs @@ -1,4 +1,4 @@ -use rustpython_ast::{Expr, Location}; +use rustpython_ast::{Located, Location}; use crate::ast::types::Range; use crate::check_ast::Checker; @@ -24,9 +24,9 @@ pub fn leading_space(line: &str) -> String { .collect() } -/// Extract the leading indentation from a docstring. -pub fn indentation<'a>(checker: &'a Checker, docstring: &Expr) -> String { - let range = Range::from_located(docstring); +/// Extract the leading indentation from a line. +pub fn indentation<'a, T>(checker: &'a Checker, located: &Located) -> String { + let range = Range::from_located(located); checker .locator .slice_source_code_range(&Range { diff --git a/src/import_tracking.rs b/src/import_tracking.rs deleted file mode 100644 index 99c2d856955d9..0000000000000 --- a/src/import_tracking.rs +++ /dev/null @@ -1,102 +0,0 @@ -use std::collections::{BTreeMap, BTreeSet}; - -use rustpython_ast::{Stmt, StmtKind}; - -use crate::autofix::{Fix, Patch}; - -#[derive(Debug)] -pub struct ImportTracker<'a> { - pub blocks: Vec>, -} - -impl<'a> ImportTracker<'a> { - pub fn new() -> Self { - Self { - blocks: vec![vec![]], - } - } - - pub fn visit_stmt(&mut self, stmt: &'a Stmt) { - let index = self.blocks.len() - 1; - if matches!( - stmt.node, - StmtKind::Import { .. } | StmtKind::ImportFrom { .. } - ) { - self.blocks[index].push(stmt); - } else { - if !self.blocks[index].is_empty() { - self.blocks.push(vec![]); - } - } - } -} - -type FromData<'a> = (&'a Option, &'a Option); -type AliasData<'a> = (&'a str, &'a Option); - -#[derive(Debug, Default)] -pub struct ImportBlock<'a> { - // Map from (module, level) to `AliasData`. - import_from: BTreeMap, BTreeSet>>, - // Set of (name, asname). - import: BTreeSet>, -} - -enum ImportType { - // __future__ - Future, - // Known standard library - StandardLibrary, - // Doesn't fit into any other category - ThirdParty, - // Local modules (but non-dotted) - FirstParty, - // Dot imports - LocalFolder, -} - -impl ImportType { - fn categorize(module: &str) -> Self { - ImportType::FirstParty - } -} - -pub fn normalize(imports: Vec<&Stmt>) -> ImportBlock { - let mut block: ImportBlock = Default::default(); - for import in imports { - match &import.node { - StmtKind::Import { names } => { - for name in names { - block.import.insert((&name.node.name, &name.node.asname)); - } - } - StmtKind::ImportFrom { - module, - names, - level, - } => { - let targets = block.import_from.entry((module, level)).or_default(); - for name in names { - targets.insert((&name.node.name, &name.node.asname)); - } - } - _ => unreachable!("Expected StmtKind::Import | StmtKind::ImportFrom"), - } - } - block -} - -fn sort_block(block: Vec<&Stmt>) -> Fix { - // Categorize each module as: __future__, standard library, third-party, first-party. - // Deduplicate. - // Consolidate `from` imports under the same module. - - Fix { - patch: Patch { - content: "".to_string(), - location: Default::default(), - end_location: Default::default(), - }, - applied: false, - } -} diff --git a/src/imports/categorize.rs b/src/imports/categorize.rs new file mode 100644 index 0000000000000..6a9cc9a7dcb60 --- /dev/null +++ b/src/imports/categorize.rs @@ -0,0 +1,35 @@ +use std::collections::BTreeMap; + +use once_cell::sync::Lazy; + +use crate::python::sys::STDLIB_MODULE_NAMES; + +static STATIC_CLASSIFICATIONS: Lazy> = Lazy::new(|| { + BTreeMap::from([ + ("__future__", ImportType::Future), + ("__main__", ImportType::FirstParty), + // Force `disutils` to be considered third-party. + ("disutils", ImportType::ThirdParty), + // Relative imports (e.g., `from . import module`). + ("", ImportType::FirstParty), + ]) +}); + +#[derive(Debug, PartialOrd, Ord, PartialEq, Eq, Clone)] +pub enum ImportType { + Future, + StandardLibrary, + ThirdParty, + FirstParty, +} + +pub fn categorize(module_base: &str) -> ImportType { + if let Some(import_type) = STATIC_CLASSIFICATIONS.get(module_base) { + import_type.clone() + } else if STDLIB_MODULE_NAMES.contains(module_base) { + ImportType::StandardLibrary + } else { + // STOPSHIP(charlie): Implement first-party classification. + ImportType::ThirdParty + } +} diff --git a/src/imports/mod.rs b/src/imports/mod.rs new file mode 100644 index 0000000000000..4f3b521dd1630 --- /dev/null +++ b/src/imports/mod.rs @@ -0,0 +1,135 @@ +use std::collections::BTreeMap; + +use ropey::RopeBuilder; +use rustpython_ast::{Stmt, StmtKind}; + +use crate::imports::categorize::{categorize, ImportType}; +use crate::imports::types::ImportBlock; + +mod categorize; +pub mod plugins; +pub mod track; +mod types; + +// Hard-code four-space indentation for the imports themselves, to match Black. +const INDENT: &str = " "; + +fn normalize_imports<'a>(imports: &'a [&'a Stmt]) -> ImportBlock<'a> { + let mut block: ImportBlock = Default::default(); + for import in imports { + match &import.node { + StmtKind::Import { names } => { + for name in names { + block.import.insert((&name.node.name, &name.node.asname)); + } + } + StmtKind::ImportFrom { + module, + names, + level, + } => { + let targets = block.import_from.entry((module, level)).or_default(); + for name in names { + targets.insert((&name.node.name, &name.node.asname)); + } + } + _ => unreachable!("Expected StmtKind::Import | StmtKind::ImportFrom"), + } + } + block +} + +fn categorize_imports(block: ImportBlock) -> BTreeMap { + let mut block_by_type: BTreeMap = Default::default(); + // Categorize `StmtKind::Import`. + for (name, asname) in block.import { + let module_base = name.split('.').next().unwrap(); + let import_type = categorize(module_base); + block_by_type + .entry(import_type) + .or_default() + .import + .insert((name, asname)); + } + // Categorize `StmtKind::ImportFrom`. + for ((module, level), aliases) in block.import_from { + let mut module_base = String::new(); + if let Some(level) = level { + if level > &0 { + module_base.push_str(&".".repeat(*level)); + } + } + if let Some(module) = module { + module_base.push_str(module); + } + let module_base = module_base.split('.').next().unwrap(); + let classification = categorize(module_base); + block_by_type + .entry(classification) + .or_default() + .import_from + .insert((module, level), aliases); + } + block_by_type +} + +pub fn sort_imports(block: Vec<&Stmt>, line_length: usize) -> String { + // Normalize imports (i.e., deduplicate, aggregate `from` imports). + let block = normalize_imports(&block); + + // Categorize by type (e.g., first-party vs. third-party). + let block_by_type = categorize_imports(block); + + // Generate replacement source code. + let mut output = RopeBuilder::new(); + let mut first_block = true; + for import_type in [ + ImportType::Future, + ImportType::StandardLibrary, + ImportType::ThirdParty, + ImportType::FirstParty, + ] { + if let Some(import_block) = block_by_type.get(&import_type) { + // Add a blank line between every section. + if !first_block { + output.append("\n"); + } else { + first_block = false; + } + + // Format `StmtKind::Import` statements. + for (name, asname) in import_block.import.iter() { + if let Some(asname) = asname { + output.append(&format!("import {} as {}\n", name, asname)); + } else { + output.append(&format!("import {}\n", name)); + } + } + + // Format `StmtKind::ImportFrom` statements. + for ((module, level), aliases) in import_block.import_from.iter() { + // STOPSHIP(charlie): Extract this into a method. + let mut module_base = String::new(); + if let Some(level) = level { + if level > &0 { + module_base.push_str(&".".repeat(*level)); + } + } + if let Some(module) = module { + module_base.push_str(module); + } + // STOPSHIP(charlie): Try to squeeze into available line-length. + output.append(&format!("from {} import (\n", module_base)); + for (name, asname) in aliases { + if let Some(asname) = asname { + output.append(&format!("{}{} as {},\n", INDENT, name, asname)); + } else { + output.append(&format!("{}{},\n", INDENT, name)); + } + } + output.append(")\n"); + } + } + } + output.finish().to_string() +} diff --git a/src/imports/plugins.rs b/src/imports/plugins.rs new file mode 100644 index 0000000000000..0f244ba2823cb --- /dev/null +++ b/src/imports/plugins.rs @@ -0,0 +1,156 @@ +use rustpython_ast::{Location, Stmt}; +use textwrap::{dedent, indent}; + +use crate::ast::types::Range; +use crate::autofix::Fix; +use crate::check_ast::Checker; +use crate::checks::CheckKind; +use crate::docstrings::helpers::leading_space; +use crate::imports::sort_imports; +use crate::{Check, SourceCodeLocator}; + +// STOPSHIP(charlie): If an import isn't the first or last statement on a line, +// this will remove other valid code. +fn extract_range(body: &[&Stmt]) -> Range { + // Extract the range of the existing import block. We extend to include the + // entire first and last line. + let location = body.iter().map(|stmt| stmt.location).min().unwrap(); + let end_location = body + .iter() + .map(|stmt| stmt.end_location) + .max() + .unwrap() + .unwrap(); + Range { + location: Location::new(location.row(), 0), + end_location: Location::new(end_location.row() + 1, 0), + } +} + +/// I001 +pub fn check_imports(checker: &mut Checker, body: Vec<&Stmt>) { + // Extract the existing import block. + let range = extract_range(&body); + let existing = checker.locator.slice_source_code_range(&range); + + // Infer existing indentation. + let indentation = leading_space(&existing); + + // Dedent the existing import block. + let actual = dedent(&existing); + + // Generate the sorted import block. + let expected = sort_imports(body, 100); + + // Compare the two? + if actual != expected { + let mut check = Check::new(CheckKind::UnsortedImports, range); + if checker.patch() { + check.amend(Fix::replacement( + indent(&expected, &indentation), + range.location, + range.end_location, + )); + } + checker.add_check(check); + } +} + +// STOPSHIP(charlie): Exists for testing. +fn actual(body: &[&Stmt], locator: &SourceCodeLocator) -> String { + let range = extract_range(body); + let existing = locator.slice_source_code_range(&range); + dedent(&existing) +} + +// STOPSHIP(charlie): Exists for testing. +fn expected(body: Vec<&Stmt>, locator: &SourceCodeLocator) -> String { + let range = extract_range(&body); + let existing = locator.slice_source_code_range(&range); + let indentation = leading_space(&existing); + let expected = sort_imports(body, 100); + indent(&expected, &indentation) +} + +#[cfg(test)] +mod tests { + use anyhow::Result; + use rustpython_ast::Stmt; + use rustpython_parser::parser; + + use crate::imports::plugins::{actual, expected}; + use crate::SourceCodeLocator; + + #[test] + fn single() -> Result<()> { + let contents = r#"import os +"#; + let suite = parser::parse_program(contents, "")?; + let locator = SourceCodeLocator::new(&contents); + + let body: Vec<&Stmt> = suite.iter().collect(); + + let actual = actual(&body, &locator); + let expected = expected(body, &locator); + + assert_eq!(actual, expected); + + Ok(()) + } + + #[test] + fn double() -> Result<()> { + let contents = r#"import sys +import os +"#; + let suite = parser::parse_program(contents, "")?; + let locator = SourceCodeLocator::new(&contents); + let body: Vec<&Stmt> = suite.iter().collect(); + + let actual = actual(&body, &locator); + assert_eq!( + actual, + r#"import sys +import os +"# + ); + + let expected = expected(body, &locator); + assert_eq!( + expected, + r#"import os +import sys +"# + ); + + Ok(()) + } + + #[test] + fn indented() -> Result<()> { + let contents = r#" import sys + import os +"#; + let suite = parser::parse_program(contents, "")?; + let locator = SourceCodeLocator::new(&contents); + let body: Vec<&Stmt> = suite.iter().collect(); + + let actual = actual(&body, &locator); + assert_eq!( + actual, + r#"import sys +import os +"# + ); + + let expected = expected(body, &locator); + assert_eq!( + expected, + r#"import os +import sys +"# + ); + + Ok(()) + } +} diff --git a/src/imports/track.rs b/src/imports/track.rs new file mode 100644 index 0000000000000..018cede7f1299 --- /dev/null +++ b/src/imports/track.rs @@ -0,0 +1,28 @@ +use rustpython_ast::{Stmt, StmtKind}; + +#[derive(Debug)] +pub struct ImportTracker<'a> { + pub blocks: Vec>, +} + +impl<'a> ImportTracker<'a> { + pub fn new() -> Self { + Self { + blocks: vec![vec![]], + } + } + + pub fn visit_stmt(&mut self, stmt: &'a Stmt) { + let index = self.blocks.len() - 1; + if matches!( + stmt.node, + StmtKind::Import { .. } | StmtKind::ImportFrom { .. } + ) { + self.blocks[index].push(stmt); + } else { + if !self.blocks[index].is_empty() { + self.blocks.push(vec![]); + } + } + } +} diff --git a/src/imports/types.rs b/src/imports/types.rs new file mode 100644 index 0000000000000..03624907d95e0 --- /dev/null +++ b/src/imports/types.rs @@ -0,0 +1,13 @@ +use std::collections::{BTreeMap, BTreeSet}; + +// STOPSHIP(charlie): Turn these into structs. +pub type FromData<'a> = (&'a Option, &'a Option); +pub type AliasData<'a> = (&'a str, &'a Option); + +#[derive(Debug, Default)] +pub struct ImportBlock<'a> { + // Map from (module, level) to `AliasData`. + pub import_from: BTreeMap, BTreeSet>>, + // Set of (name, asname). + pub import: BTreeSet>, +} diff --git a/src/lib.rs b/src/lib.rs index ab23f56c753fd..6527361a81f5d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,7 +32,7 @@ mod flake8_comprehensions; mod flake8_print; pub mod flake8_quotes; pub mod fs; -mod import_tracking; +mod imports; mod lex; pub mod linter; pub mod logging; diff --git a/src/python/mod.rs b/src/python/mod.rs index 611cd0691ae13..7381cc6810e71 100644 --- a/src/python/mod.rs +++ b/src/python/mod.rs @@ -1,4 +1,5 @@ pub mod builtins; pub mod future; pub mod keyword; +pub mod sys; pub mod typing; diff --git a/src/python/sys.rs b/src/python/sys.rs new file mode 100644 index 0000000000000..69ff20df81fec --- /dev/null +++ b/src/python/sys.rs @@ -0,0 +1,313 @@ +use std::collections::BTreeSet; + +use once_cell::sync::Lazy; + +/// A copy of `sys.stdlib_module_names` on Python 3.10.2. +/// TODO(charlie): Bundle a list for each version? Or just the latest? +pub static STDLIB_MODULE_NAMES: Lazy> = Lazy::new(|| { + BTreeSet::from([ + "ipaddress", + "_frozen_importlib", + "uu", + "sre_parse", + "_datetime", + "_lzma", + "_multibytecodec", + "nntplib", + "operator", + "shelve", + "_json", + "_locale", + "_tracemalloc", + "doctest", + "turtledemo", + "logging", + "_sqlite3", + "_gdbm", + "cmath", + "mmap", + "this", + "sndhdr", + "gc", + "profile", + "_abc", + "readline", + "urllib", + "_csv", + "_ssl", + "select", + "aifc", + "nt", + "socket", + "_markupbase", + "_frozen_importlib_external", + "secrets", + "smtplib", + "_posixsubprocess", + "_crypt", + "mimetypes", + "_sha3", + "array", + "_tkinter", + "wsgiref", + "colorsys", + "_contextvars", + "queue", + "filecmp", + "_multiprocessing", + "compileall", + "_uuid", + "fcntl", + "marshal", + "json", + "grp", + "syslog", + "contextvars", + "uuid", + "zipfile", + "time", + "curses", + "nturl2path", + "_hashlib", + "io", + "_stat", + "_scproxy", + "pickletools", + "_osx_support", + "enum", + "pydoc_data", + "quopri", + "rlcompleter", + "_py_abc", + "binhex", + "turtle", + "configparser", + "posix", + "pty", + "getpass", + "threading", + "wave", + "fractions", + "math", + "atexit", + "sys", + "_zoneinfo", + "difflib", + "stringprep", + "_queue", + "_blake2", + "socketserver", + "_bisect", + "_codecs_kr", + "tokenize", + "_asyncio", + "codecs", + "plistlib", + "chunk", + "distutils", + "antigravity", + "csv", + "sre_compile", + "cgi", + "msilib", + "platform", + "_sha512", + "_codecs_iso2022", + "_compression", + "builtins", + "decimal", + "runpy", + "tty", + "errno", + "_strptime", + "_functools", + "argparse", + "mailcap", + "functools", + "netrc", + "_aix_support", + "ntpath", + "asyncio", + "_sha256", + "selectors", + "tarfile", + "optparse", + "abc", + "winsound", + "_codecs_cn", + "_operator", + "calendar", + "nis", + "ssl", + "_codecs_jp", + "contextlib", + "cProfile", + "tkinter", + "shutil", + "unicodedata", + "py_compile", + "_posixshmem", + "sre_constants", + "zipapp", + "posixpath", + "binascii", + "imaplib", + "pipes", + "token", + "_elementtree", + "_heapq", + "copy", + "zipimport", + "_pyio", + "bz2", + "html", + "unittest", + "resource", + "_codecs_hk", + "crypt", + "gzip", + "_decimal", + "ftplib", + "email", + "keyword", + "pdb", + "typing", + "modulefinder", + "statistics", + "tabnanny", + "xdrlib", + "zoneinfo", + "ast", + "_overlapped", + "_codecs_tw", + "_warnings", + "__future__", + "asyncore", + "concurrent", + "_msi", + "_opcode", + "http", + "_weakrefset", + "fileinput", + "pyexpat", + "reprlib", + "sched", + "shlex", + "_collections_abc", + "pstats", + "warnings", + "_struct", + "_bootsubprocess", + "asynchat", + "_thread", + "dis", + "xmlrpc", + "cmd", + "_random", + "tracemalloc", + "dataclasses", + "_string", + "collections", + "_sha1", + "ensurepip", + "_imp", + "_md5", + "hmac", + "sqlite3", + "pprint", + "_ctypes", + "_winapi", + "_collections", + "timeit", + "_bz2", + "winreg", + "opcode", + "tempfile", + "_codecs", + "_dbm", + "base64", + "gettext", + "codeop", + "importlib", + "mailbox", + "multiprocessing", + "random", + "lib2to3", + "telnetlib", + "pathlib", + "ossaudiodev", + "weakref", + "graphlib", + "_sre", + "copyreg", + "bisect", + "_pickle", + "itertools", + "symtable", + "linecache", + "pickle", + "datetime", + "numbers", + "smtpd", + "_pydecimal", + "locale", + "struct", + "_sitebuiltins", + "heapq", + "imghdr", + "_statistics", + "types", + "signal", + "_curses", + "_signal", + "dbm", + "poplib", + "pwd", + "imp", + "pyclbr", + "site", + "sysconfig", + "textwrap", + "_threading_local", + "idlelib", + "re", + "faulthandler", + "lzma", + "pydoc", + "trace", + "_compat_pickle", + "_weakref", + "genericpath", + "inspect", + "_curses_panel", + "stat", + "getopt", + "xml", + "os", + "subprocess", + "ctypes", + "webbrowser", + "bdb", + "venv", + "_lsprof", + "cgitb", + "string", + "code", + "hashlib", + "_socket", + "glob", + "fnmatch", + "encodings", + "_symtable", + "audioop", + "msvcrt", + "_ast", + "termios", + "sunau", + "_io", + "spwd", + "zlib", + "traceback", + "pkgutil", + ]) +}); diff --git a/test/local.py b/test/local.py index 0f0b77ef8e835..5984554846fb0 100644 --- a/test/local.py +++ b/test/local.py @@ -11,3 +11,20 @@ from .boop import bar from wop import wop import foo, bar + +# [isort] +# import __future__.bar as foo +# import __future__.baz +# +# import bar +# import wop +# from wop import wop +# +# import foo +# import foo as bar +# import foo as baz +# import foo.f1 as g1 +# import foo.f1 as g2 +# +# from . import bar +# from .boop import bar From c921e5c841d811a0ddcbb5a7451a87730465bff6 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 9 Nov 2022 23:05:47 -0500 Subject: [PATCH 03/10] Implement local file tracking --- src/check_ast.rs | 19 +- src/imports/categorize.rs | 35 --- src/imports/track.rs | 28 -- src/imports/types.rs | 13 - src/isort/categorize.rs | 93 ++++++ src/{imports => isort}/mod.rs | 107 ++++--- src/{imports => isort}/plugins.rs | 25 +- src/isort/settings.rs | 41 +++ src/isort/track.rs | 45 +++ src/isort/types.rs | 55 ++++ src/lib.rs | 2 +- src/python/sys.rs | 475 ++++++++++++------------------ src/settings/configuration.rs | 31 +- src/settings/mod.rs | 13 +- src/settings/options.rs | 4 +- src/settings/pyproject.rs | 12 + src/settings/user.rs | 6 +- test/local.py | 1 + test/pyproject.toml | 3 + 19 files changed, 591 insertions(+), 417 deletions(-) delete mode 100644 src/imports/categorize.rs delete mode 100644 src/imports/track.rs delete mode 100644 src/imports/types.rs create mode 100644 src/isort/categorize.rs rename src/{imports => isort}/mod.rs (53%) rename src/{imports => isort}/plugins.rs (86%) create mode 100644 src/isort/settings.rs create mode 100644 src/isort/track.rs create mode 100644 src/isort/types.rs diff --git a/src/check_ast.rs b/src/check_ast.rs index a4ae2dacf3f27..de37fd539ef00 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -23,7 +23,7 @@ use crate::ast::{helpers, operations, visitor}; use crate::autofix::fixer; use crate::checks::{Check, CheckCode, CheckKind}; use crate::docstrings::definition::{Definition, DefinitionKind, Documentable}; -use crate::imports::track::ImportTracker; +use crate::isort::track::ImportTracker; use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS}; use crate::python::future::ALL_FEATURE_NAMES; use crate::python::typing; @@ -34,7 +34,7 @@ use crate::source_code_locator::SourceCodeLocator; use crate::visibility::{module_visibility, transition_scope, Modifier, Visibility, VisibleScope}; use crate::{ docstrings, flake8_annotations, flake8_bugbear, flake8_builtins, flake8_comprehensions, - flake8_print, imports, pep8_naming, pycodestyle, pydocstyle, pyflakes, pyupgrade, + flake8_print, isort, pep8_naming, pycodestyle, pydocstyle, pyflakes, pyupgrade, }; const GLOBAL_SCOPE_INDEX: usize = 0; @@ -187,11 +187,6 @@ where fn visit_stmt(&mut self, stmt: &'b Stmt) { self.push_parent(stmt); - // Track all import blocks (to power import sorting). - // TODO(charlie): This doesn't work with exception handlers. Maybe we should - // have some more general clearing mechanism? - self.import_tracker.visit_stmt(stmt); - // Track whether we've seen docstrings, non-imports, etc. match &stmt.node { StmtKind::ImportFrom { module, .. } => { @@ -947,6 +942,9 @@ where }; self.pop_parent(); + + // Call-through to any composed visitors. + self.import_tracker.visit_stmt(stmt); } fn visit_annotation(&mut self, expr: &'b Expr) { @@ -1745,6 +1743,9 @@ where } } } + + // Call-through to any composed visitors. + self.import_tracker.visit_excepthandler(excepthandler); } fn visit_arguments(&mut self, arguments: &'b Arguments) { @@ -2428,9 +2429,9 @@ impl<'a> Checker<'a> { return; } - while let Some(block) = self.import_tracker.blocks.pop() { + while let Some(block) = self.import_tracker.next() { if !block.is_empty() { - imports::plugins::check_imports(self, block); + isort::plugins::check_imports(self, block); } } } diff --git a/src/imports/categorize.rs b/src/imports/categorize.rs deleted file mode 100644 index 6a9cc9a7dcb60..0000000000000 --- a/src/imports/categorize.rs +++ /dev/null @@ -1,35 +0,0 @@ -use std::collections::BTreeMap; - -use once_cell::sync::Lazy; - -use crate::python::sys::STDLIB_MODULE_NAMES; - -static STATIC_CLASSIFICATIONS: Lazy> = Lazy::new(|| { - BTreeMap::from([ - ("__future__", ImportType::Future), - ("__main__", ImportType::FirstParty), - // Force `disutils` to be considered third-party. - ("disutils", ImportType::ThirdParty), - // Relative imports (e.g., `from . import module`). - ("", ImportType::FirstParty), - ]) -}); - -#[derive(Debug, PartialOrd, Ord, PartialEq, Eq, Clone)] -pub enum ImportType { - Future, - StandardLibrary, - ThirdParty, - FirstParty, -} - -pub fn categorize(module_base: &str) -> ImportType { - if let Some(import_type) = STATIC_CLASSIFICATIONS.get(module_base) { - import_type.clone() - } else if STDLIB_MODULE_NAMES.contains(module_base) { - ImportType::StandardLibrary - } else { - // STOPSHIP(charlie): Implement first-party classification. - ImportType::ThirdParty - } -} diff --git a/src/imports/track.rs b/src/imports/track.rs deleted file mode 100644 index 018cede7f1299..0000000000000 --- a/src/imports/track.rs +++ /dev/null @@ -1,28 +0,0 @@ -use rustpython_ast::{Stmt, StmtKind}; - -#[derive(Debug)] -pub struct ImportTracker<'a> { - pub blocks: Vec>, -} - -impl<'a> ImportTracker<'a> { - pub fn new() -> Self { - Self { - blocks: vec![vec![]], - } - } - - pub fn visit_stmt(&mut self, stmt: &'a Stmt) { - let index = self.blocks.len() - 1; - if matches!( - stmt.node, - StmtKind::Import { .. } | StmtKind::ImportFrom { .. } - ) { - self.blocks[index].push(stmt); - } else { - if !self.blocks[index].is_empty() { - self.blocks.push(vec![]); - } - } - } -} diff --git a/src/imports/types.rs b/src/imports/types.rs deleted file mode 100644 index 03624907d95e0..0000000000000 --- a/src/imports/types.rs +++ /dev/null @@ -1,13 +0,0 @@ -use std::collections::{BTreeMap, BTreeSet}; - -// STOPSHIP(charlie): Turn these into structs. -pub type FromData<'a> = (&'a Option, &'a Option); -pub type AliasData<'a> = (&'a str, &'a Option); - -#[derive(Debug, Default)] -pub struct ImportBlock<'a> { - // Map from (module, level) to `AliasData`. - pub import_from: BTreeMap, BTreeSet>>, - // Set of (name, asname). - pub import: BTreeSet>, -} diff --git a/src/isort/categorize.rs b/src/isort/categorize.rs new file mode 100644 index 0000000000000..70933bdd8496c --- /dev/null +++ b/src/isort/categorize.rs @@ -0,0 +1,93 @@ +use std::collections::{BTreeMap, BTreeSet}; +use std::fs; +use std::os::macos::fs::MetadataExt; +use std::path::{Path, PathBuf}; + +use anyhow::Result; +use once_cell::sync::Lazy; + +use crate::python::sys::KNOWN_STANDARD_LIBRARY; + +#[derive(Debug, PartialOrd, Ord, PartialEq, Eq, Clone)] +pub enum ImportType { + Future, + StandardLibrary, + ThirdParty, + FirstParty, +} + +pub fn categorize( + module_base: &str, + src_paths: &[PathBuf], + known_first_party: &BTreeSet, + known_third_party: &BTreeSet, + extra_standard_library: &BTreeSet, +) -> Result { + if known_first_party.contains(module_base) { + Ok(ImportType::FirstParty) + } else if known_third_party.contains(module_base) { + Ok(ImportType::ThirdParty) + } else if extra_standard_library.contains(module_base) { + Ok(ImportType::StandardLibrary) + } else if let Some(import_type) = STATIC_CLASSIFICATIONS.get(module_base) { + Ok(import_type.clone()) + } else if KNOWN_STANDARD_LIBRARY.contains(module_base) { + Ok(ImportType::StandardLibrary) + } else { + // STOPSHIP(charlie): Do this once. + let app_dirs = get_app( + src_paths + .iter() + .map(|src_path| Path::new(src_path).to_path_buf()), + )?; + println!("app_dirs = {:?}", app_dirs); + if find_local(&app_dirs, module_base) { + Ok(ImportType::FirstParty) + } else { + Ok(ImportType::ThirdParty) + } + } +} + +static STATIC_CLASSIFICATIONS: Lazy> = Lazy::new(|| { + BTreeMap::from([ + ("__future__", ImportType::Future), + ("__main__", ImportType::FirstParty), + // Force `disutils` to be considered third-party. + ("disutils", ImportType::ThirdParty), + // Relative imports (e.g., `from . import module`). + ("", ImportType::FirstParty), + ]) +}); + +fn path_key(path: &PathBuf) -> Result<(u64, u64)> { + let metadata = fs::metadata(path)?; + Ok((metadata.st_ino(), metadata.st_dev())) +} + +fn get_app(app_dirs: impl Iterator) -> Result> { + let mut paths = vec![]; + let mut seen: BTreeSet<(u64, u64)> = Default::default(); + for app_dir in app_dirs { + if seen.insert(path_key(&app_dir)?) { + paths.push(app_dir); + } + } + Ok(paths) +} + +fn find_local(paths: &[PathBuf], base: &str) -> bool { + for path in paths { + if let Ok(metadata) = fs::metadata(path.join(base)) { + if metadata.is_dir() { + return true; + } + } + if let Ok(metadata) = fs::metadata(path.join(format!("{base}.py"))) { + if metadata.is_file() { + return true; + } + } + } + false +} diff --git a/src/imports/mod.rs b/src/isort/mod.rs similarity index 53% rename from src/imports/mod.rs rename to src/isort/mod.rs index 4f3b521dd1630..68f0032b5dc5c 100644 --- a/src/imports/mod.rs +++ b/src/isort/mod.rs @@ -1,13 +1,17 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; +use std::path::PathBuf; +use anyhow::Result; use ropey::RopeBuilder; use rustpython_ast::{Stmt, StmtKind}; -use crate::imports::categorize::{categorize, ImportType}; -use crate::imports::types::ImportBlock; +use crate::isort::categorize::{categorize, ImportType}; +use crate::isort::settings::Settings; +use crate::isort::types::{AliasData, ImportBlock, ImportFromData, Importable}; mod categorize; pub mod plugins; +pub mod settings; pub mod track; mod types; @@ -20,7 +24,10 @@ fn normalize_imports<'a>(imports: &'a [&'a Stmt]) -> ImportBlock<'a> { match &import.node { StmtKind::Import { names } => { for name in names { - block.import.insert((&name.node.name, &name.node.asname)); + block.import.insert(AliasData { + name: &name.node.name, + asname: &name.node.asname, + }); } } StmtKind::ImportFrom { @@ -28,9 +35,15 @@ fn normalize_imports<'a>(imports: &'a [&'a Stmt]) -> ImportBlock<'a> { names, level, } => { - let targets = block.import_from.entry((module, level)).or_default(); + let targets = block + .import_from + .entry(ImportFromData { module, level }) + .or_default(); for name in names { - targets.insert((&name.node.name, &name.node.asname)); + targets.insert(AliasData { + name: &name.node.name, + asname: &name.node.asname, + }); } } _ => unreachable!("Expected StmtKind::Import | StmtKind::ImportFrom"), @@ -39,46 +52,66 @@ fn normalize_imports<'a>(imports: &'a [&'a Stmt]) -> ImportBlock<'a> { block } -fn categorize_imports(block: ImportBlock) -> BTreeMap { +fn categorize_imports<'a>( + block: ImportBlock<'a>, + src_paths: &[PathBuf], + known_first_party: &BTreeSet, + known_third_party: &BTreeSet, + extra_standard_library: &BTreeSet, +) -> Result>> { let mut block_by_type: BTreeMap = Default::default(); // Categorize `StmtKind::Import`. - for (name, asname) in block.import { - let module_base = name.split('.').next().unwrap(); - let import_type = categorize(module_base); + for alias in block.import { + let import_type = categorize( + &alias.module_base(), + src_paths, + known_first_party, + known_third_party, + extra_standard_library, + )?; block_by_type .entry(import_type) .or_default() .import - .insert((name, asname)); + .insert(alias); } // Categorize `StmtKind::ImportFrom`. - for ((module, level), aliases) in block.import_from { - let mut module_base = String::new(); - if let Some(level) = level { - if level > &0 { - module_base.push_str(&".".repeat(*level)); - } - } - if let Some(module) = module { - module_base.push_str(module); - } - let module_base = module_base.split('.').next().unwrap(); - let classification = categorize(module_base); + for (import_from, aliases) in block.import_from { + let classification = categorize( + &import_from.module_base(), + src_paths, + known_first_party, + known_third_party, + extra_standard_library, + )?; block_by_type .entry(classification) .or_default() .import_from - .insert((module, level), aliases); + .insert(import_from, aliases); } - block_by_type + Ok(block_by_type) } -pub fn sort_imports(block: Vec<&Stmt>, line_length: usize) -> String { +pub fn sort_imports( + block: Vec<&Stmt>, + line_length: &usize, + src_paths: &[PathBuf], + known_first_party: &BTreeSet, + known_third_party: &BTreeSet, + extra_standard_library: &BTreeSet, +) -> Result { // Normalize imports (i.e., deduplicate, aggregate `from` imports). let block = normalize_imports(&block); // Categorize by type (e.g., first-party vs. third-party). - let block_by_type = categorize_imports(block); + let block_by_type = categorize_imports( + block, + src_paths, + known_first_party, + known_third_party, + extra_standard_library, + )?; // Generate replacement source code. let mut output = RopeBuilder::new(); @@ -98,7 +131,7 @@ pub fn sort_imports(block: Vec<&Stmt>, line_length: usize) -> String { } // Format `StmtKind::Import` statements. - for (name, asname) in import_block.import.iter() { + for AliasData { name, asname } in import_block.import.iter() { if let Some(asname) = asname { output.append(&format!("import {} as {}\n", name, asname)); } else { @@ -107,20 +140,10 @@ pub fn sort_imports(block: Vec<&Stmt>, line_length: usize) -> String { } // Format `StmtKind::ImportFrom` statements. - for ((module, level), aliases) in import_block.import_from.iter() { - // STOPSHIP(charlie): Extract this into a method. - let mut module_base = String::new(); - if let Some(level) = level { - if level > &0 { - module_base.push_str(&".".repeat(*level)); - } - } - if let Some(module) = module { - module_base.push_str(module); - } + for (import_from, aliases) in import_block.import_from.iter() { // STOPSHIP(charlie): Try to squeeze into available line-length. - output.append(&format!("from {} import (\n", module_base)); - for (name, asname) in aliases { + output.append(&format!("from {} import (\n", import_from.module_name())); + for AliasData { name, asname } in aliases { if let Some(asname) = asname { output.append(&format!("{}{} as {},\n", INDENT, name, asname)); } else { @@ -131,5 +154,5 @@ pub fn sort_imports(block: Vec<&Stmt>, line_length: usize) -> String { } } } - output.finish().to_string() + Ok(output.finish().to_string()) } diff --git a/src/imports/plugins.rs b/src/isort/plugins.rs similarity index 86% rename from src/imports/plugins.rs rename to src/isort/plugins.rs index 0f244ba2823cb..9f2e99708bc83 100644 --- a/src/imports/plugins.rs +++ b/src/isort/plugins.rs @@ -1,3 +1,4 @@ +use path_absolutize::path_dedot; use rustpython_ast::{Location, Stmt}; use textwrap::{dedent, indent}; @@ -6,7 +7,7 @@ use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::CheckKind; use crate::docstrings::helpers::leading_space; -use crate::imports::sort_imports; +use crate::isort::sort_imports; use crate::{Check, SourceCodeLocator}; // STOPSHIP(charlie): If an import isn't the first or last statement on a line, @@ -40,7 +41,15 @@ pub fn check_imports(checker: &mut Checker, body: Vec<&Stmt>) { let actual = dedent(&existing); // Generate the sorted import block. - let expected = sort_imports(body, 100); + let expected = sort_imports( + body, + &checker.settings.line_length, + &checker.settings.src_paths, + &checker.settings.isort.known_first_party, + &checker.settings.isort.known_third_party, + &checker.settings.isort.extra_standard_library, + ) + .unwrap(); // Compare the two? if actual != expected { @@ -68,7 +77,15 @@ fn expected(body: Vec<&Stmt>, locator: &SourceCodeLocator) -> String { let range = extract_range(&body); let existing = locator.slice_source_code_range(&range); let indentation = leading_space(&existing); - let expected = sort_imports(body, 100); + let expected = sort_imports( + body, + &100, + &vec![path_dedot::CWD.clone()], + &Default::default(), + &Default::default(), + &Default::default(), + ) + .unwrap(); indent(&expected, &indentation) } @@ -78,7 +95,7 @@ mod tests { use rustpython_ast::Stmt; use rustpython_parser::parser; - use crate::imports::plugins::{actual, expected}; + use crate::isort::plugins::{actual, expected}; use crate::SourceCodeLocator; #[test] diff --git a/src/isort/settings.rs b/src/isort/settings.rs new file mode 100644 index 0000000000000..d651d46e7ae31 --- /dev/null +++ b/src/isort/settings.rs @@ -0,0 +1,41 @@ +//! Settings for the `isort` plugin. + +use serde::{Deserialize, Serialize}; +use std::collections::BTreeSet; + +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +pub struct Options { + pub known_first_party: Option>, + pub known_third_party: Option>, + pub extra_standard_library: Option>, +} + +#[derive(Debug, Hash)] +pub struct Settings { + pub known_first_party: BTreeSet, + pub known_third_party: BTreeSet, + pub extra_standard_library: BTreeSet, +} + +impl Settings { + pub fn from_options(options: Options) -> Self { + Self { + known_first_party: BTreeSet::from_iter(options.known_first_party.unwrap_or_default()), + known_third_party: BTreeSet::from_iter(options.known_third_party.unwrap_or_default()), + extra_standard_library: BTreeSet::from_iter( + options.extra_standard_library.unwrap_or_default(), + ), + } + } +} + +impl Default for Settings { + fn default() -> Self { + Self { + known_first_party: BTreeSet::new(), + known_third_party: BTreeSet::new(), + extra_standard_library: BTreeSet::new(), + } + } +} diff --git a/src/isort/track.rs b/src/isort/track.rs new file mode 100644 index 0000000000000..9efa5fc399c70 --- /dev/null +++ b/src/isort/track.rs @@ -0,0 +1,45 @@ +use rustpython_ast::{Excepthandler, Stmt, StmtKind}; + +use crate::ast::visitor::Visitor; + +#[derive(Debug)] +pub struct ImportTracker<'a> { + blocks: Vec>, +} +impl<'a> ImportTracker<'a> { + pub fn new() -> Self { + Self { + blocks: vec![vec![]], + } + } + + pub fn next(&mut self) -> Option> { + self.blocks.pop() + } +} + +impl<'a, 'b> Visitor<'b> for ImportTracker<'a> +where + 'b: 'a, +{ + fn visit_stmt(&mut self, stmt: &'b Stmt) { + let index = self.blocks.len() - 1; + if matches!( + stmt.node, + StmtKind::Import { .. } | StmtKind::ImportFrom { .. } + ) { + self.blocks[index].push(stmt); + } else { + if !self.blocks[index].is_empty() { + self.blocks.push(vec![]); + } + } + } + + fn visit_excepthandler(&mut self, _: &'a Excepthandler) { + let index = self.blocks.len() - 1; + if !self.blocks[index].is_empty() { + self.blocks.push(vec![]); + } + } +} diff --git a/src/isort/types.rs b/src/isort/types.rs new file mode 100644 index 0000000000000..042126755b6ec --- /dev/null +++ b/src/isort/types.rs @@ -0,0 +1,55 @@ +use std::collections::{BTreeMap, BTreeSet}; + +#[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq)] +pub struct ImportFromData<'a> { + pub module: &'a Option, + pub level: &'a Option, +} + +#[derive(Debug, Hash, Ord, PartialOrd, Eq, PartialEq)] +pub struct AliasData<'a> { + pub name: &'a str, + pub asname: &'a Option, +} + +pub trait Importable { + fn module_name(&self) -> String; + fn module_base(&self) -> String; +} + +impl Importable for AliasData<'_> { + fn module_name(&self) -> String { + self.name.to_string() + } + + fn module_base(&self) -> String { + self.module_name().split('.').next().unwrap().to_string() + } +} + +impl Importable for ImportFromData<'_> { + fn module_name(&self) -> String { + let mut module_name = String::new(); + if let Some(level) = self.level { + if level > &0 { + module_name.push_str(&".".repeat(*level)); + } + } + if let Some(module) = self.module { + module_name.push_str(module); + } + module_name + } + + fn module_base(&self) -> String { + self.module_name().split('.').next().unwrap().to_string() + } +} + +#[derive(Debug, Default)] +pub struct ImportBlock<'a> { + // Map from (module, level) to `AliasData`. + pub import_from: BTreeMap, BTreeSet>>, + // Set of (name, asname). + pub import: BTreeSet>, +} diff --git a/src/lib.rs b/src/lib.rs index 6527361a81f5d..982c6193401a8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,7 +32,7 @@ mod flake8_comprehensions; mod flake8_print; pub mod flake8_quotes; pub mod fs; -mod imports; +mod isort; mod lex; pub mod linter; pub mod logging; diff --git a/src/python/sys.rs b/src/python/sys.rs index 69ff20df81fec..36d4ee2fd5be2 100644 --- a/src/python/sys.rs +++ b/src/python/sys.rs @@ -2,312 +2,227 @@ use std::collections::BTreeSet; use once_cell::sync::Lazy; -/// A copy of `sys.stdlib_module_names` on Python 3.10.2. -/// TODO(charlie): Bundle a list for each version? Or just the latest? -pub static STDLIB_MODULE_NAMES: Lazy> = Lazy::new(|| { +// See: https://pycqa.github.io/isort/docs/configuration/options.html#known-standard-library +pub static KNOWN_STANDARD_LIBRARY: Lazy> = Lazy::new(|| { BTreeSet::from([ - "ipaddress", - "_frozen_importlib", - "uu", - "sre_parse", - "_datetime", - "_lzma", - "_multibytecodec", - "nntplib", - "operator", - "shelve", - "_json", - "_locale", - "_tracemalloc", - "doctest", - "turtledemo", - "logging", - "_sqlite3", - "_gdbm", - "cmath", - "mmap", - "this", - "sndhdr", - "gc", - "profile", - "_abc", - "readline", - "urllib", - "_csv", - "_ssl", - "select", + "_ast", + "_dummy_thread", + "_thread", + "abc", "aifc", - "nt", - "socket", - "_markupbase", - "_frozen_importlib_external", - "secrets", - "smtplib", - "_posixsubprocess", - "_crypt", - "mimetypes", - "_sha3", + "argparse", "array", - "_tkinter", - "wsgiref", + "ast", + "asynchat", + "asyncio", + "asyncore", + "atexit", + "audioop", + "base64", + "bdb", + "binascii", + "binhex", + "bisect", + "builtins", + "bz2", + "cProfile", + "calendar", + "cgi", + "cgitb", + "chunk", + "cmath", + "cmd", + "code", + "codecs", + "codeop", + "collections", "colorsys", - "_contextvars", - "queue", - "filecmp", - "_multiprocessing", "compileall", - "_uuid", - "fcntl", - "marshal", - "json", - "grp", - "syslog", + "concurrent", + "configparser", + "contextlib", "contextvars", - "uuid", - "zipfile", - "time", + "copy", + "copyreg", + "crypt", + "csv", + "ctypes", "curses", - "nturl2path", - "_hashlib", - "io", - "_stat", - "_scproxy", - "pickletools", - "_osx_support", - "enum", - "pydoc_data", - "quopri", - "rlcompleter", - "_py_abc", - "binhex", - "turtle", - "configparser", - "posix", - "pty", - "getpass", - "threading", - "wave", - "fractions", - "math", - "atexit", - "sys", - "_zoneinfo", + "dataclasses", + "datetime", + "dbm", + "decimal", "difflib", - "stringprep", - "_queue", - "_blake2", - "socketserver", - "_bisect", - "_codecs_kr", - "tokenize", - "_asyncio", - "codecs", - "plistlib", - "chunk", + "dis", "distutils", - "antigravity", - "csv", - "sre_compile", - "cgi", - "msilib", - "platform", - "_sha512", - "_codecs_iso2022", - "_compression", - "builtins", - "decimal", - "runpy", - "tty", + "doctest", + "dummy_threading", + "email", + "encodings", + "ensurepip", + "enum", "errno", - "_strptime", - "_functools", - "argparse", - "mailcap", + "faulthandler", + "fcntl", + "filecmp", + "fileinput", + "fnmatch", + "formatter", + "fpectl", + "fractions", + "ftplib", "functools", - "netrc", - "_aix_support", - "ntpath", - "asyncio", - "_sha256", - "selectors", - "tarfile", - "optparse", - "abc", - "winsound", - "_codecs_cn", - "_operator", - "calendar", - "nis", - "ssl", - "_codecs_jp", - "contextlib", - "cProfile", - "tkinter", - "shutil", - "unicodedata", - "py_compile", - "_posixshmem", - "sre_constants", - "zipapp", - "posixpath", - "binascii", - "imaplib", - "pipes", - "token", - "_elementtree", - "_heapq", - "copy", - "zipimport", - "_pyio", - "bz2", - "html", - "unittest", - "resource", - "_codecs_hk", - "crypt", + "gc", + "getopt", + "getpass", + "gettext", + "glob", + "graphlib", + "grp", "gzip", - "_decimal", - "ftplib", - "email", - "keyword", - "pdb", - "typing", - "modulefinder", - "statistics", - "tabnanny", - "xdrlib", - "zoneinfo", - "ast", - "_overlapped", - "_codecs_tw", - "_warnings", - "__future__", - "asyncore", - "concurrent", - "_msi", - "_opcode", - "http", - "_weakrefset", - "fileinput", - "pyexpat", - "reprlib", - "sched", - "shlex", - "_collections_abc", - "pstats", - "warnings", - "_struct", - "_bootsubprocess", - "asynchat", - "_thread", - "dis", - "xmlrpc", - "cmd", - "_random", - "tracemalloc", - "dataclasses", - "_string", - "collections", - "_sha1", - "ensurepip", - "_imp", - "_md5", + "hashlib", + "heapq", "hmac", - "sqlite3", - "pprint", - "_ctypes", - "_winapi", - "_collections", - "timeit", - "_bz2", - "winreg", - "opcode", - "tempfile", - "_codecs", - "_dbm", - "base64", - "gettext", - "codeop", + "html", + "http", + "imaplib", + "imghdr", + "imp", "importlib", + "inspect", + "io", + "ipaddress", + "itertools", + "json", + "keyword", + "lib2to3", + "linecache", + "locale", + "logging", + "lzma", + "macpath", "mailbox", + "mailcap", + "marshal", + "math", + "mimetypes", + "mmap", + "modulefinder", + "msilib", + "msvcrt", "multiprocessing", - "random", - "lib2to3", - "telnetlib", - "pathlib", + "netrc", + "nis", + "nntplib", + "ntpath", + "numbers", + "operator", + "optparse", + "os", "ossaudiodev", - "weakref", - "graphlib", - "_sre", - "copyreg", - "bisect", - "_pickle", - "itertools", - "symtable", - "linecache", + "parser", + "pathlib", + "pdb", "pickle", - "datetime", - "numbers", - "smtpd", - "_pydecimal", - "locale", - "struct", - "_sitebuiltins", - "heapq", - "imghdr", - "_statistics", - "types", - "signal", - "_curses", - "_signal", - "dbm", + "pickletools", + "pipes", + "pkgutil", + "platform", + "plistlib", "poplib", + "posix", + "posixpath", + "pprint", + "profile", + "pstats", + "pty", "pwd", - "imp", + "py_compile", "pyclbr", + "pydoc", + "queue", + "quopri", + "random", + "re", + "readline", + "reprlib", + "resource", + "rlcompleter", + "runpy", + "sched", + "secrets", + "select", + "selectors", + "shelve", + "shlex", + "shutil", + "signal", "site", + "smtpd", + "smtplib", + "sndhdr", + "socket", + "socketserver", + "spwd", + "sqlite3", + "sre", + "sre_compile", + "sre_constants", + "sre_parse", + "ssl", + "stat", + "statistics", + "string", + "stringprep", + "struct", + "subprocess", + "sunau", + "symbol", + "symtable", + "sys", "sysconfig", + "syslog", + "tabnanny", + "tarfile", + "telnetlib", + "tempfile", + "termios", + "test", "textwrap", - "_threading_local", - "idlelib", - "re", - "faulthandler", - "lzma", - "pydoc", + "threading", + "time", + "timeit", + "tkinter", + "token", + "tokenize", "trace", - "_compat_pickle", - "_weakref", - "genericpath", - "inspect", - "_curses_panel", - "stat", - "getopt", - "xml", - "os", - "subprocess", - "ctypes", - "webbrowser", - "bdb", + "traceback", + "tracemalloc", + "tty", + "turtle", + "turtledemo", + "types", + "typing", + "unicodedata", + "unittest", + "urllib", + "uu", + "uuid", "venv", - "_lsprof", - "cgitb", - "string", - "code", - "hashlib", - "_socket", - "glob", - "fnmatch", - "encodings", - "_symtable", - "audioop", - "msvcrt", - "_ast", - "termios", - "sunau", - "_io", - "spwd", + "warnings", + "wave", + "weakref", + "webbrowser", + "winreg", + "winsound", + "wsgiref", + "xdrlib", + "xml", + "xmlrpc", + "zipapp", + "zipfile", + "zipimport", "zlib", - "traceback", - "pkgutil", + "zoneinfo", ]) }); diff --git a/src/settings/configuration.rs b/src/settings/configuration.rs index 61e763a6d5df2..7085e006f0e22 100644 --- a/src/settings/configuration.rs +++ b/src/settings/configuration.rs @@ -2,16 +2,17 @@ //! command-line options. Structure mirrors the user-facing representation of //! the various parameters. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use anyhow::{anyhow, Result}; use once_cell::sync::Lazy; +use path_absolutize::path_dedot; use regex::Regex; use crate::checks_gen::CheckCodePrefix; use crate::settings::pyproject::load_options; use crate::settings::types::{FilePattern, PerFileIgnore, PythonVersion}; -use crate::{flake8_annotations, flake8_quotes, pep8_naming}; +use crate::{flake8_annotations, flake8_quotes, fs, isort, pep8_naming}; #[derive(Debug)] pub struct Configuration { @@ -25,10 +26,12 @@ pub struct Configuration { pub line_length: usize, pub per_file_ignores: Vec, pub select: Vec, + pub src_paths: Vec, pub target_version: PythonVersion, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, pub flake8_quotes: flake8_quotes::settings::Settings, + pub isort: isort::settings::Settings, pub pep8_naming: pep8_naming::settings::Settings, } @@ -71,6 +74,26 @@ impl Configuration { .map_err(|e| anyhow!("Invalid dummy-variable-rgx value: {e}"))?, None => DEFAULT_DUMMY_VARIABLE_RGX.clone(), }, + src_paths: options + .src_paths + .map(|src_paths| { + src_paths + .iter() + .map(|path| { + let path = Path::new(path); + match project_root { + Some(project_root) => fs::normalize_path_to(path, project_root), + None => fs::normalize_path(path), + } + }) + .collect() + }) + .unwrap_or_else(|| { + vec![match project_root { + Some(project_root) => project_root.clone(), + None => path_dedot::CWD.clone(), + }] + }), target_version: options.target_version.unwrap_or(PythonVersion::Py310), exclude: options .exclude @@ -115,6 +138,10 @@ impl Configuration { .flake8_quotes .map(flake8_quotes::settings::Settings::from_options) .unwrap_or_default(), + isort: options + .isort + .map(isort::settings::Settings::from_options) + .unwrap_or_default(), pep8_naming: options .pep8_naming .map(pep8_naming::settings::Settings::from_options) diff --git a/src/settings/mod.rs b/src/settings/mod.rs index c9d8fdbb209f9..91cf079023906 100644 --- a/src/settings/mod.rs +++ b/src/settings/mod.rs @@ -4,14 +4,16 @@ use std::collections::BTreeSet; use std::hash::{Hash, Hasher}; +use std::path::PathBuf; +use path_absolutize::path_dedot; use regex::Regex; use crate::checks::CheckCode; use crate::checks_gen::{CheckCodePrefix, PrefixSpecificity}; use crate::settings::configuration::Configuration; use crate::settings::types::{FilePattern, PerFileIgnore, PythonVersion}; -use crate::{flake8_annotations, flake8_quotes, pep8_naming}; +use crate::{flake8_annotations, flake8_quotes, isort, pep8_naming}; pub mod configuration; pub mod options; @@ -27,10 +29,12 @@ pub struct Settings { pub extend_exclude: Vec, pub line_length: usize, pub per_file_ignores: Vec, + pub src_paths: Vec, pub target_version: PythonVersion, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, pub flake8_quotes: flake8_quotes::settings::Settings, + pub isort: isort::settings::Settings, pub pep8_naming: pep8_naming::settings::Settings, } @@ -48,9 +52,11 @@ impl Settings { extend_exclude: config.extend_exclude, flake8_annotations: config.flake8_annotations, flake8_quotes: config.flake8_quotes, + isort: config.isort, line_length: config.line_length, pep8_naming: config.pep8_naming, per_file_ignores: config.per_file_ignores, + src_paths: config.src_paths, target_version: config.target_version, } } @@ -63,9 +69,11 @@ impl Settings { extend_exclude: Default::default(), line_length: 88, per_file_ignores: Default::default(), + src_paths: vec![path_dedot::CWD.clone()], target_version: PythonVersion::Py310, flake8_annotations: Default::default(), flake8_quotes: Default::default(), + isort: Default::default(), pep8_naming: Default::default(), } } @@ -78,9 +86,11 @@ impl Settings { extend_exclude: Default::default(), line_length: 88, per_file_ignores: Default::default(), + src_paths: vec![path_dedot::CWD.clone()], target_version: PythonVersion::Py310, flake8_annotations: Default::default(), flake8_quotes: Default::default(), + isort: Default::default(), pep8_naming: Default::default(), } } @@ -101,6 +111,7 @@ impl Hash for Settings { // Add plugin properties in alphabetical order. self.flake8_annotations.hash(state); self.flake8_quotes.hash(state); + self.isort.hash(state); self.pep8_naming.hash(state); } } diff --git a/src/settings/options.rs b/src/settings/options.rs index d87ae776a98ad..ccd6d40e363c9 100644 --- a/src/settings/options.rs +++ b/src/settings/options.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use crate::checks_gen::CheckCodePrefix; use crate::settings::types::PythonVersion; -use crate::{flake8_annotations, flake8_quotes, pep8_naming}; +use crate::{flake8_annotations, flake8_quotes, isort, pep8_naming}; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] @@ -21,9 +21,11 @@ pub struct Options { pub line_length: Option, pub per_file_ignores: Option>>, pub select: Option>, + pub src_paths: Option>, pub target_version: Option, // Plugins pub flake8_annotations: Option, pub flake8_quotes: Option, + pub isort: Option, pub pep8_naming: Option, } diff --git a/src/settings/pyproject.rs b/src/settings/pyproject.rs index ae407a061a487..9ad2a0243fb4f 100644 --- a/src/settings/pyproject.rs +++ b/src/settings/pyproject.rs @@ -143,9 +143,11 @@ mod tests { extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, + src_paths: None, target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }) }) @@ -172,9 +174,11 @@ line-length = 79 extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, + src_paths: None, target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }) }) @@ -201,9 +205,11 @@ exclude = ["foo.py"] extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, + src_paths: None, target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }) }) @@ -230,9 +236,11 @@ select = ["E501"] extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, + src_paths: None, target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }) }) @@ -260,9 +268,11 @@ ignore = ["E501"] extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, + src_paths: None, target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }) }) @@ -336,6 +346,7 @@ other-attribute = 1 vec![CheckCodePrefix::F401] ),])), dummy_variable_rgx: None, + src_paths: None, target_version: None, flake8_annotations: None, flake8_quotes: Some(flake8_quotes::settings::Options { @@ -344,6 +355,7 @@ other-attribute = 1 docstring_quotes: Some(Quote::Double), avoid_escape: Some(true), }), + isort: None, pep8_naming: Some(pep8_naming::settings::Options { ignore_names: Some(vec![ "setUp".to_string(), diff --git a/src/settings/user.rs b/src/settings/user.rs index c86ec4d0cf383..62ca411baa9af 100644 --- a/src/settings/user.rs +++ b/src/settings/user.rs @@ -7,7 +7,7 @@ use regex::Regex; use crate::checks::CheckCode; use crate::checks_gen::CheckCodePrefix; use crate::settings::types::{FilePattern, PythonVersion}; -use crate::{flake8_annotations, flake8_quotes, pep8_naming, Configuration}; +use crate::{flake8_annotations, flake8_quotes, isort, pep8_naming, Configuration}; /// Struct to render user-facing exclusion patterns. #[derive(Debug)] @@ -45,10 +45,12 @@ pub struct UserConfiguration { pub line_length: usize, pub per_file_ignores: Vec<(Exclusion, Vec)>, pub select: Vec, + pub src_paths: Vec, pub target_version: PythonVersion, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, pub flake8_quotes: flake8_quotes::settings::Settings, + pub isort: isort::settings::Settings, pub pep8_naming: pep8_naming::settings::Settings, // Non-settings exposed to the user pub project_root: Option, @@ -89,9 +91,11 @@ impl UserConfiguration { }) .collect(), select: configuration.select, + src_paths: configuration.src_paths, target_version: configuration.target_version, flake8_annotations: configuration.flake8_annotations, flake8_quotes: configuration.flake8_quotes, + isort: configuration.isort, pep8_naming: configuration.pep8_naming, project_root, pyproject, diff --git a/test/local.py b/test/local.py index 5984554846fb0..70655e226186c 100644 --- a/test/local.py +++ b/test/local.py @@ -11,6 +11,7 @@ from .boop import bar from wop import wop import foo, bar +import local_from # [isort] # import __future__.bar as foo diff --git a/test/pyproject.toml b/test/pyproject.toml index c15a9bcfd201b..d5f705014e080 100644 --- a/test/pyproject.toml +++ b/test/pyproject.toml @@ -1,3 +1,6 @@ +[tool.ruff.isort] +#src-paths = ['test'] + [tool.isort] line_length = 88 profile = 'black' From 478ddef9db5748612432398cd537efb59770c5bd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 10 Nov 2022 10:03:10 -0500 Subject: [PATCH 04/10] Remove Result<> from isort --- src/isort/categorize.rs | 45 +++++++++-------------------------------- src/isort/mod.rs | 16 +++++++-------- src/isort/plugins.rs | 6 ++---- 3 files changed, 19 insertions(+), 48 deletions(-) diff --git a/src/isort/categorize.rs b/src/isort/categorize.rs index 70933bdd8496c..24e65aef8d185 100644 --- a/src/isort/categorize.rs +++ b/src/isort/categorize.rs @@ -1,9 +1,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::fs; -use std::os::macos::fs::MetadataExt; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; -use anyhow::Result; use once_cell::sync::Lazy; use crate::python::sys::KNOWN_STANDARD_LIBRARY; @@ -22,29 +20,22 @@ pub fn categorize( known_first_party: &BTreeSet, known_third_party: &BTreeSet, extra_standard_library: &BTreeSet, -) -> Result { +) -> ImportType { if known_first_party.contains(module_base) { - Ok(ImportType::FirstParty) + ImportType::FirstParty } else if known_third_party.contains(module_base) { - Ok(ImportType::ThirdParty) + ImportType::ThirdParty } else if extra_standard_library.contains(module_base) { - Ok(ImportType::StandardLibrary) + ImportType::StandardLibrary } else if let Some(import_type) = STATIC_CLASSIFICATIONS.get(module_base) { - Ok(import_type.clone()) + import_type.clone() } else if KNOWN_STANDARD_LIBRARY.contains(module_base) { - Ok(ImportType::StandardLibrary) + ImportType::StandardLibrary } else { - // STOPSHIP(charlie): Do this once. - let app_dirs = get_app( - src_paths - .iter() - .map(|src_path| Path::new(src_path).to_path_buf()), - )?; - println!("app_dirs = {:?}", app_dirs); - if find_local(&app_dirs, module_base) { - Ok(ImportType::FirstParty) + if find_local(&src_paths, module_base) { + ImportType::FirstParty } else { - Ok(ImportType::ThirdParty) + ImportType::ThirdParty } } } @@ -60,22 +51,6 @@ static STATIC_CLASSIFICATIONS: Lazy> = Lazy:: ]) }); -fn path_key(path: &PathBuf) -> Result<(u64, u64)> { - let metadata = fs::metadata(path)?; - Ok((metadata.st_ino(), metadata.st_dev())) -} - -fn get_app(app_dirs: impl Iterator) -> Result> { - let mut paths = vec![]; - let mut seen: BTreeSet<(u64, u64)> = Default::default(); - for app_dir in app_dirs { - if seen.insert(path_key(&app_dir)?) { - paths.push(app_dir); - } - } - Ok(paths) -} - fn find_local(paths: &[PathBuf], base: &str) -> bool { for path in paths { if let Ok(metadata) = fs::metadata(path.join(base)) { diff --git a/src/isort/mod.rs b/src/isort/mod.rs index 68f0032b5dc5c..16aa22e4c60a3 100644 --- a/src/isort/mod.rs +++ b/src/isort/mod.rs @@ -1,12 +1,10 @@ use std::collections::{BTreeMap, BTreeSet}; use std::path::PathBuf; -use anyhow::Result; use ropey::RopeBuilder; use rustpython_ast::{Stmt, StmtKind}; use crate::isort::categorize::{categorize, ImportType}; -use crate::isort::settings::Settings; use crate::isort::types::{AliasData, ImportBlock, ImportFromData, Importable}; mod categorize; @@ -58,7 +56,7 @@ fn categorize_imports<'a>( known_first_party: &BTreeSet, known_third_party: &BTreeSet, extra_standard_library: &BTreeSet, -) -> Result>> { +) -> BTreeMap> { let mut block_by_type: BTreeMap = Default::default(); // Categorize `StmtKind::Import`. for alias in block.import { @@ -68,7 +66,7 @@ fn categorize_imports<'a>( known_first_party, known_third_party, extra_standard_library, - )?; + ); block_by_type .entry(import_type) .or_default() @@ -83,14 +81,14 @@ fn categorize_imports<'a>( known_first_party, known_third_party, extra_standard_library, - )?; + ); block_by_type .entry(classification) .or_default() .import_from .insert(import_from, aliases); } - Ok(block_by_type) + block_by_type } pub fn sort_imports( @@ -100,7 +98,7 @@ pub fn sort_imports( known_first_party: &BTreeSet, known_third_party: &BTreeSet, extra_standard_library: &BTreeSet, -) -> Result { +) -> String { // Normalize imports (i.e., deduplicate, aggregate `from` imports). let block = normalize_imports(&block); @@ -111,7 +109,7 @@ pub fn sort_imports( known_first_party, known_third_party, extra_standard_library, - )?; + ); // Generate replacement source code. let mut output = RopeBuilder::new(); @@ -154,5 +152,5 @@ pub fn sort_imports( } } } - Ok(output.finish().to_string()) + output.finish().to_string() } diff --git a/src/isort/plugins.rs b/src/isort/plugins.rs index 9f2e99708bc83..8a3066a87670d 100644 --- a/src/isort/plugins.rs +++ b/src/isort/plugins.rs @@ -48,8 +48,7 @@ pub fn check_imports(checker: &mut Checker, body: Vec<&Stmt>) { &checker.settings.isort.known_first_party, &checker.settings.isort.known_third_party, &checker.settings.isort.extra_standard_library, - ) - .unwrap(); + ); // Compare the two? if actual != expected { @@ -84,8 +83,7 @@ fn expected(body: Vec<&Stmt>, locator: &SourceCodeLocator) -> String { &Default::default(), &Default::default(), &Default::default(), - ) - .unwrap(); + ); indent(&expected, &indentation) } From 4ec64bdae20fcafaf11bb4c8f2ce065709dd3ef7 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 10 Nov 2022 10:21:52 -0500 Subject: [PATCH 05/10] Add line-length handling --- bar.py | 4 +-- fast.py | 7 ++++++ pyproject.toml | 4 +++ src/check_ast.rs | 12 ++++----- src/isort/mod.rs | 57 +++++++++++++++++++++++++++++++++++++------ src/isort/settings.rs | 3 ++- test/local.py | 3 ++- 7 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 fast.py diff --git a/bar.py b/bar.py index 955b01ce987b5..1566170f466f0 100644 --- a/bar.py +++ b/bar.py @@ -1,4 +1,4 @@ try: - import sys -except: import os +except: + import sys diff --git a/fast.py b/fast.py new file mode 100644 index 0000000000000..194174c826ef1 --- /dev/null +++ b/fast.py @@ -0,0 +1,7 @@ +# comment 1a +# comment 1b +# comment 2a +# comment 2b +from .param_functions import Body # comment 1c +from .param_functions import Header # comment 1d +from .param_functions import Cookie, Request diff --git a/pyproject.toml b/pyproject.toml index acd70ec63833c..ed43e4deaf14c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,3 +32,7 @@ build-backend = "maturin" bindings = "bin" sdist-include = ["Cargo.lock"] strip = true + +[tool.isort] +profile = "black" +known_third_party = ["fastapi", "pydantic", "starlette"] diff --git a/src/check_ast.rs b/src/check_ast.rs index de37fd539ef00..3a534c81824fe 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -185,6 +185,9 @@ where 'b: 'a, { fn visit_stmt(&mut self, stmt: &'b Stmt) { + // Call-through to any composed visitors. + self.import_tracker.visit_stmt(stmt); + self.push_parent(stmt); // Track whether we've seen docstrings, non-imports, etc. @@ -942,9 +945,6 @@ where }; self.pop_parent(); - - // Call-through to any composed visitors. - self.import_tracker.visit_stmt(stmt); } fn visit_annotation(&mut self, expr: &'b Expr) { @@ -1664,6 +1664,9 @@ where } fn visit_excepthandler(&mut self, excepthandler: &'b Excepthandler) { + // Call-through to any composed visitors. + self.import_tracker.visit_excepthandler(excepthandler); + match &excepthandler.node { ExcepthandlerKind::ExceptHandler { type_, name, .. } => { if self.settings.enabled.contains(&CheckCode::E722) && type_.is_none() { @@ -1743,9 +1746,6 @@ where } } } - - // Call-through to any composed visitors. - self.import_tracker.visit_excepthandler(excepthandler); } fn visit_arguments(&mut self, arguments: &'b Arguments) { diff --git a/src/isort/mod.rs b/src/isort/mod.rs index 16aa22e4c60a3..4af086cac4d8b 100644 --- a/src/isort/mod.rs +++ b/src/isort/mod.rs @@ -139,16 +139,57 @@ pub fn sort_imports( // Format `StmtKind::ImportFrom` statements. for (import_from, aliases) in import_block.import_from.iter() { - // STOPSHIP(charlie): Try to squeeze into available line-length. - output.append(&format!("from {} import (\n", import_from.module_name())); - for AliasData { name, asname } in aliases { - if let Some(asname) = asname { - output.append(&format!("{}{} as {},\n", INDENT, name, asname)); - } else { - output.append(&format!("{}{},\n", INDENT, name)); + let prelude: String = format!("from {} import ", import_from.module_name()); + let members: Vec = aliases + .iter() + .map(|AliasData { name, asname }| { + if let Some(asname) = asname { + format!("{} as {}", name, asname) + } else { + format!("{}", name) + } + }) + .collect(); + + // Can we fit the import on a single line? + let expected_len: usize = + // `from base import ` + prelude.len() + // `member( as alias)?` + + members.iter().map(|part| part.len()).sum::() + // `, ` + + 2 * (members.len() - 1); + + if expected_len <= *line_length { + // `from base import ` + output.append(&prelude); + // `member( as alias)?(, )?` + for (index, part) in members.into_iter().enumerate() { + if index > 0 { + output.append(", "); + } + output.append(&part); + } + // `\n` + output.append("\n"); + } else { + // `from base import (\n` + output.append(&prelude); + output.append("("); + output.append("\n"); + + // ` member( as alias)?,\n` + for part in members { + output.append(INDENT); + output.append(&part); + output.append(","); + output.append("\n"); } + + // `)\n` + output.append(")"); + output.append("\n"); } - output.append(")\n"); } } } diff --git a/src/isort/settings.rs b/src/isort/settings.rs index d651d46e7ae31..923d5fb37db0a 100644 --- a/src/isort/settings.rs +++ b/src/isort/settings.rs @@ -1,8 +1,9 @@ //! Settings for the `isort` plugin. -use serde::{Deserialize, Serialize}; use std::collections::BTreeSet; +use serde::{Deserialize, Serialize}; + #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct Options { diff --git a/test/local.py b/test/local.py index 70655e226186c..c39bcc85017bd 100644 --- a/test/local.py +++ b/test/local.py @@ -9,7 +9,8 @@ import wop from . import bar from .boop import bar -from wop import wop +from wop import wop as wop +from wop import bop as bop import foo, bar import local_from From ad2d9c9fd0e647c4a523ee547ef15c8e018ed408 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 10 Nov 2022 14:41:27 -0500 Subject: [PATCH 06/10] Try to fix tracking --- src/ast/visitor.rs | 13 ++++++++++--- src/isort/track.rs | 23 ++++++++++++++++++----- test/pyproject.toml | 2 +- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/ast/visitor.rs b/src/ast/visitor.rs index ae73e30e82480..68105a27327bd 100644 --- a/src/ast/visitor.rs +++ b/src/ast/visitor.rs @@ -7,6 +7,9 @@ use rustpython_parser::ast::{ use crate::ast::helpers::match_name_or_attr; pub trait Visitor<'a> { + fn visit_body(&mut self, body: &[&'a Stmt]) { + walk_body(self, body); + } fn visit_stmt(&mut self, stmt: &'a Stmt) { walk_stmt(self, stmt); } @@ -63,6 +66,12 @@ pub trait Visitor<'a> { } } +pub fn walk_body<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, body: &[&'a Stmt]) { + for stmt in body { + visitor.visit_stmt(stmt); + } +} + pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { match &stmt.node { StmtKind::FunctionDef { @@ -79,9 +88,7 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { for expr in returns { visitor.visit_annotation(expr); } - for stmt in body { - visitor.visit_stmt(stmt); - } + vistor.visit_body(body); } StmtKind::AsyncFunctionDef { args, diff --git a/src/isort/track.rs b/src/isort/track.rs index 9efa5fc399c70..757797f3308d4 100644 --- a/src/isort/track.rs +++ b/src/isort/track.rs @@ -18,11 +18,24 @@ impl<'a> ImportTracker<'a> { } } -impl<'a, 'b> Visitor<'b> for ImportTracker<'a> -where - 'b: 'a, -{ - fn visit_stmt(&mut self, stmt: &'b Stmt) { +impl<'a> ImportTracker<'a> { + fn visit_stmt(&mut self, stmt: &'a Stmt) { + let index = self.blocks.len() - 1; + if matches!( + stmt.node, + StmtKind::Import { .. } | StmtKind::ImportFrom { .. } + ) { + self.blocks[index].push(stmt); + } else { + if !self.blocks[index].is_empty() { + self.blocks.push(vec![]); + } + } + } + + fn leave_stmt(&mut self, stmt: &'a Stmt) { + // STOPSHIP(charlie): This isn't quite right. We also have to reset whenever we reach a new + // level of indentation. let index = self.blocks.len() - 1; if matches!( stmt.node, diff --git a/test/pyproject.toml b/test/pyproject.toml index d5f705014e080..fb3315de0281b 100644 --- a/test/pyproject.toml +++ b/test/pyproject.toml @@ -1,5 +1,5 @@ [tool.ruff.isort] -#src-paths = ['test'] +src-paths = ['test'] [tool.isort] line_length = 88 From 495e7655c4cc40e3e21d206c028b55004030c5d5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 10 Nov 2022 15:49:38 -0500 Subject: [PATCH 07/10] Rewrite as its own visitor --- bar.py | 7 +- src/ast/visitor.rs | 13 +-- src/check_ast.rs | 14 +--- src/check_imports.rs | 42 ++++++++++ src/checks.rs | 2 + src/isort/plugins.rs | 29 ++++--- src/isort/track.rs | 185 +++++++++++++++++++++++++++++++++++++------ src/lib.rs | 1 + src/linter.rs | 24 ++++-- 9 files changed, 248 insertions(+), 69 deletions(-) create mode 100644 src/check_imports.rs diff --git a/bar.py b/bar.py index 1566170f466f0..c5ab34ca16b91 100644 --- a/bar.py +++ b/bar.py @@ -1,4 +1,7 @@ try: - import os + import os + import sys + + import x except: - import sys + import os diff --git a/src/ast/visitor.rs b/src/ast/visitor.rs index 68105a27327bd..ae73e30e82480 100644 --- a/src/ast/visitor.rs +++ b/src/ast/visitor.rs @@ -7,9 +7,6 @@ use rustpython_parser::ast::{ use crate::ast::helpers::match_name_or_attr; pub trait Visitor<'a> { - fn visit_body(&mut self, body: &[&'a Stmt]) { - walk_body(self, body); - } fn visit_stmt(&mut self, stmt: &'a Stmt) { walk_stmt(self, stmt); } @@ -66,12 +63,6 @@ pub trait Visitor<'a> { } } -pub fn walk_body<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, body: &[&'a Stmt]) { - for stmt in body { - visitor.visit_stmt(stmt); - } -} - pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { match &stmt.node { StmtKind::FunctionDef { @@ -88,7 +79,9 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { for expr in returns { visitor.visit_annotation(expr); } - vistor.visit_body(body); + for stmt in body { + visitor.visit_stmt(stmt); + } } StmtKind::AsyncFunctionDef { args, diff --git a/src/check_ast.rs b/src/check_ast.rs index 3a534c81824fe..4c47899079a67 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -2424,18 +2424,6 @@ impl<'a> Checker<'a> { self.add_checks(checks.into_iter()); } - fn check_import_blocks(&mut self) { - if !self.settings.enabled.contains(&CheckCode::I001) { - return; - } - - while let Some(block) = self.import_tracker.next() { - if !block.is_empty() { - isort::plugins::check_imports(self, block); - } - } - } - fn check_definitions(&mut self) { while let Some((definition, visibility)) = self.definitions.pop() { // flake8-annotations @@ -2614,7 +2602,7 @@ pub fn check_ast( checker.check_definitions(); // Check import blocks. - checker.check_import_blocks(); + // checker.check_import_blocks(); checker.checks } diff --git a/src/check_imports.rs b/src/check_imports.rs new file mode 100644 index 0000000000000..0aadaa57f12b7 --- /dev/null +++ b/src/check_imports.rs @@ -0,0 +1,42 @@ +//! Lint rules based on import analysis. + +use rustpython_parser::ast::Suite; + +use crate::ast::visitor::Visitor; +use crate::autofix::fixer; +use crate::checks::Check; +use crate::isort; +use crate::isort::track::ImportTracker; +use crate::settings::Settings; +use crate::source_code_locator::SourceCodeLocator; + +fn check_import_blocks( + tracker: &mut ImportTracker, + locator: &SourceCodeLocator, + settings: &Settings, + autofix: &fixer::Mode, +) -> Vec { + let mut checks = vec![]; + while let Some(block) = tracker.next() { + if !block.is_empty() { + if let Some(check) = isort::plugins::check_imports(block, locator, settings, autofix) { + checks.push(check); + } + } + } + checks +} + +pub fn check_imports( + python_ast: &Suite, + locator: &SourceCodeLocator, + settings: &Settings, + autofix: &fixer::Mode, +) -> Vec { + let mut tracker = ImportTracker::new(); + for stmt in python_ast { + tracker.visit_stmt(stmt); + } + println!("{:?}", tracker.blocks); + check_import_blocks(&mut tracker, locator, settings, autofix) +} diff --git a/src/checks.rs b/src/checks.rs index 640a0dbe92e92..380ffce0e34a1 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -286,6 +286,7 @@ pub enum LintSource { FileSystem, Lines, Tokens, + Imports, } #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -502,6 +503,7 @@ impl CheckCode { | CheckCode::RUF002 | CheckCode::RUF003 => &LintSource::Tokens, CheckCode::E902 => &LintSource::FileSystem, + CheckCode::I001 => &LintSource::Imports, _ => &LintSource::AST, } } diff --git a/src/isort/plugins.rs b/src/isort/plugins.rs index 8a3066a87670d..d40a8b84def35 100644 --- a/src/isort/plugins.rs +++ b/src/isort/plugins.rs @@ -3,12 +3,12 @@ use rustpython_ast::{Location, Stmt}; use textwrap::{dedent, indent}; use crate::ast::types::Range; -use crate::autofix::Fix; +use crate::autofix::{fixer, Fix}; use crate::check_ast::Checker; use crate::checks::CheckKind; use crate::docstrings::helpers::leading_space; use crate::isort::sort_imports; -use crate::{Check, SourceCodeLocator}; +use crate::{Check, Settings, SourceCodeLocator}; // STOPSHIP(charlie): If an import isn't the first or last statement on a line, // this will remove other valid code. @@ -29,10 +29,15 @@ fn extract_range(body: &[&Stmt]) -> Range { } /// I001 -pub fn check_imports(checker: &mut Checker, body: Vec<&Stmt>) { +pub fn check_imports( + body: Vec<&Stmt>, + locator: &SourceCodeLocator, + settings: &Settings, + autofix: &fixer::Mode, +) -> Option { // Extract the existing import block. let range = extract_range(&body); - let existing = checker.locator.slice_source_code_range(&range); + let existing = locator.slice_source_code_range(&range); // Infer existing indentation. let indentation = leading_space(&existing); @@ -43,24 +48,26 @@ pub fn check_imports(checker: &mut Checker, body: Vec<&Stmt>) { // Generate the sorted import block. let expected = sort_imports( body, - &checker.settings.line_length, - &checker.settings.src_paths, - &checker.settings.isort.known_first_party, - &checker.settings.isort.known_third_party, - &checker.settings.isort.extra_standard_library, + &settings.line_length, + &settings.src_paths, + &settings.isort.known_first_party, + &settings.isort.known_third_party, + &settings.isort.extra_standard_library, ); // Compare the two? if actual != expected { let mut check = Check::new(CheckKind::UnsortedImports, range); - if checker.patch() { + if autofix.patch() { check.amend(Fix::replacement( indent(&expected, &indentation), range.location, range.end_location, )); } - checker.add_check(check); + Some(check) + } else { + None } } diff --git a/src/isort/track.rs b/src/isort/track.rs index 757797f3308d4..62a2717a2a1cb 100644 --- a/src/isort/track.rs +++ b/src/isort/track.rs @@ -1,10 +1,15 @@ -use rustpython_ast::{Excepthandler, Stmt, StmtKind}; +use rustpython_ast::{ + Alias, Arg, Arguments, Boolop, Cmpop, Comprehension, Constant, Excepthandler, + ExcepthandlerKind, Expr, ExprContext, Keyword, MatchCase, Operator, Pattern, Stmt, StmtKind, + Unaryop, Withitem, +}; +use crate::ast::visitor; use crate::ast::visitor::Visitor; #[derive(Debug)] pub struct ImportTracker<'a> { - blocks: Vec>, + pub blocks: Vec>, } impl<'a> ImportTracker<'a> { pub fn new() -> Self { @@ -13,46 +18,174 @@ impl<'a> ImportTracker<'a> { } } + fn add_import(&mut self, stmt: &'a Stmt) { + let index = self.blocks.len() - 1; + self.blocks[index].push(stmt); + } + + fn finalize(&mut self) { + let index = self.blocks.len() - 1; + if !self.blocks[index].is_empty() { + self.blocks.push(vec![]); + } + } + pub fn next(&mut self) -> Option> { self.blocks.pop() } } -impl<'a> ImportTracker<'a> { - fn visit_stmt(&mut self, stmt: &'a Stmt) { - let index = self.blocks.len() - 1; +impl<'a, 'b> Visitor<'b> for ImportTracker<'a> +where + 'b: 'a, +{ + fn visit_stmt(&mut self, stmt: &'b Stmt) { + // Track imports. if matches!( stmt.node, StmtKind::Import { .. } | StmtKind::ImportFrom { .. } ) { - self.blocks[index].push(stmt); + self.add_import(stmt); } else { - if !self.blocks[index].is_empty() { - self.blocks.push(vec![]); + self.finalize(); + } + + // Track scope. + match &stmt.node { + StmtKind::FunctionDef { body, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + } + StmtKind::AsyncFunctionDef { body, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + } + StmtKind::ClassDef { body, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + } + StmtKind::For { body, orelse, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + + for stmt in orelse { + self.visit_stmt(stmt); + } + self.finalize(); + } + StmtKind::AsyncFor { body, orelse, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + + for stmt in orelse { + self.visit_stmt(stmt); + } + self.finalize(); + } + StmtKind::While { body, orelse, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + + for stmt in orelse { + self.visit_stmt(stmt); + } + self.finalize(); } + StmtKind::If { body, orelse, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + + for stmt in orelse { + self.visit_stmt(stmt); + } + self.finalize(); + } + StmtKind::With { body, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + } + StmtKind::AsyncWith { body, .. } => { + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + } + StmtKind::Match { cases, .. } => { + for match_case in cases { + self.visit_match_case(match_case); + } + } + StmtKind::Try { + body, + handlers, + orelse, + finalbody, + } => { + for excepthandler in handlers { + self.visit_excepthandler(excepthandler) + } + + for stmt in body { + self.visit_stmt(stmt); + } + self.finalize(); + + for stmt in orelse { + self.visit_stmt(stmt); + } + self.finalize(); + + for stmt in finalbody { + self.visit_stmt(stmt); + } + self.finalize(); + } + _ => {} } } - - fn leave_stmt(&mut self, stmt: &'a Stmt) { - // STOPSHIP(charlie): This isn't quite right. We also have to reset whenever we reach a new - // level of indentation. - let index = self.blocks.len() - 1; - if matches!( - stmt.node, - StmtKind::Import { .. } | StmtKind::ImportFrom { .. } - ) { - self.blocks[index].push(stmt); - } else { - if !self.blocks[index].is_empty() { - self.blocks.push(vec![]); + fn visit_annotation(&mut self, _: &'b Expr) {} + fn visit_expr(&mut self, _: &'b Expr) {} + fn visit_constant(&mut self, _: &'b Constant) {} + fn visit_expr_context(&mut self, _: &'b ExprContext) {} + fn visit_boolop(&mut self, _: &'b Boolop) {} + fn visit_operator(&mut self, _: &'b Operator) {} + fn visit_unaryop(&mut self, _: &'b Unaryop) {} + fn visit_cmpop(&mut self, _: &'b Cmpop) {} + fn visit_comprehension(&mut self, _: &'b Comprehension) {} + fn visit_excepthandler(&mut self, excepthandler: &'b Excepthandler) { + if let ExcepthandlerKind::ExceptHandler { body, .. } = &excepthandler.node { + for stmt in body { + self.visit_stmt(stmt); } + self.finalize(); } } - - fn visit_excepthandler(&mut self, _: &'a Excepthandler) { - let index = self.blocks.len() - 1; - if !self.blocks[index].is_empty() { - self.blocks.push(vec![]); + fn visit_arguments(&mut self, _: &'b Arguments) {} + fn visit_arg(&mut self, _: &'b Arg) {} + fn visit_keyword(&mut self, _: &'b Keyword) {} + fn visit_alias(&mut self, _: &'b Alias) {} + fn visit_withitem(&mut self, _: &'b Withitem) {} + fn visit_match_case(&mut self, match_case: &'b MatchCase) { + for stmt in &match_case.body { + self.visit_stmt(stmt); } + self.finalize(); } + fn visit_pattern(&mut self, _: &'b Pattern) {} } diff --git a/src/lib.rs b/src/lib.rs index 982c6193401a8..5986929c21e3c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,6 +17,7 @@ mod ast; pub mod autofix; pub mod cache; pub mod check_ast; +mod check_imports; mod check_lines; mod check_tokens; pub mod checks; diff --git a/src/linter.rs b/src/linter.rs index 756a7d0aa93db..83769bc727a15 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -16,6 +16,7 @@ use crate::ast::types::Range; use crate::autofix::fixer; use crate::autofix::fixer::fix_file; use crate::check_ast::check_ast; +use crate::check_imports::check_imports; use crate::check_lines::check_lines; use crate::check_tokens::check_tokens; use crate::checks::{Check, CheckCode, CheckKind, LintSource}; @@ -63,23 +64,32 @@ pub(crate) fn check_path( let mut checks: Vec = vec![]; // Run the token-based checks. - if settings + let use_tokens = settings .enabled .iter() - .any(|check_code| matches!(check_code.lint_source(), LintSource::Tokens)) - { + .any(|check_code| matches!(check_code.lint_source(), LintSource::Tokens)); + if use_tokens { check_tokens(&mut checks, locator, &tokens, settings, autofix); } // Run the AST-based checks. - if settings + let use_ast = settings .enabled .iter() - .any(|check_code| matches!(check_code.lint_source(), LintSource::AST)) - { + .any(|check_code| matches!(check_code.lint_source(), LintSource::AST)); + let use_imports = settings + .enabled + .iter() + .any(|check_code| matches!(check_code.lint_source(), LintSource::Imports)); + if use_ast || use_imports { match parse_program_tokens(tokens, "") { Ok(python_ast) => { - checks.extend(check_ast(&python_ast, locator, settings, autofix, path)) + if use_ast { + checks.extend(check_ast(&python_ast, locator, settings, autofix, path)); + } + if use_imports { + checks.extend(check_imports(&python_ast, locator, settings, autofix)); + } } Err(parse_error) => { if settings.enabled.contains(&CheckCode::E999) { From 4b649f77bcd6edeaa06f0fa8604df8233ad07a49 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 10 Nov 2022 16:11:29 -0500 Subject: [PATCH 08/10] Add test cases --- foo.py | 4 +- src/check_ast.rs | 2 +- src/check_imports.rs | 7 ++- src/isort/plugins.rs | 89 ++++++++++++++++++++++--------- src/isort/track.rs | 13 +++-- test/local.py | 16 ------ bar.py => test_cases/after/bar.py | 2 - test_cases/after/comment.py | 3 ++ test_cases/after/foo.py | 6 +++ test_cases/after/prefix.py | 3 ++ test_cases/after/suffix.py | 3 ++ test_cases/before/bar.py | 5 ++ test_cases/before/comment.py | 3 ++ test_cases/before/foo.py | 4 ++ test_cases/before/prefix.py | 2 + test_cases/before/suffix.py | 2 + 16 files changed, 108 insertions(+), 56 deletions(-) rename bar.py => test_cases/after/bar.py (80%) create mode 100644 test_cases/after/comment.py create mode 100644 test_cases/after/foo.py create mode 100644 test_cases/after/prefix.py create mode 100644 test_cases/after/suffix.py create mode 100644 test_cases/before/bar.py create mode 100644 test_cases/before/comment.py create mode 100644 test_cases/before/foo.py create mode 100644 test_cases/before/prefix.py create mode 100644 test_cases/before/suffix.py diff --git a/foo.py b/foo.py index cd000d8e084b4..a64c31c8b87e4 100644 --- a/foo.py +++ b/foo.py @@ -1,3 +1,3 @@ if True: - x = 1; import os; import sys - import bar; import baz; y = 1; + x = 1; + y = 1; diff --git a/src/check_ast.rs b/src/check_ast.rs index 4c47899079a67..6f4bdf30272a6 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -34,7 +34,7 @@ use crate::source_code_locator::SourceCodeLocator; use crate::visibility::{module_visibility, transition_scope, Modifier, Visibility, VisibleScope}; use crate::{ docstrings, flake8_annotations, flake8_bugbear, flake8_builtins, flake8_comprehensions, - flake8_print, isort, pep8_naming, pycodestyle, pydocstyle, pyflakes, pyupgrade, + flake8_print, pep8_naming, pycodestyle, pydocstyle, pyflakes, pyupgrade, }; const GLOBAL_SCOPE_INDEX: usize = 0; diff --git a/src/check_imports.rs b/src/check_imports.rs index 0aadaa57f12b7..f01b5b163ff7a 100644 --- a/src/check_imports.rs +++ b/src/check_imports.rs @@ -11,13 +11,13 @@ use crate::settings::Settings; use crate::source_code_locator::SourceCodeLocator; fn check_import_blocks( - tracker: &mut ImportTracker, + tracker: ImportTracker, locator: &SourceCodeLocator, settings: &Settings, autofix: &fixer::Mode, ) -> Vec { let mut checks = vec![]; - while let Some(block) = tracker.next() { + for block in tracker.into_iter() { if !block.is_empty() { if let Some(check) = isort::plugins::check_imports(block, locator, settings, autofix) { checks.push(check); @@ -37,6 +37,5 @@ pub fn check_imports( for stmt in python_ast { tracker.visit_stmt(stmt); } - println!("{:?}", tracker.blocks); - check_import_blocks(&mut tracker, locator, settings, autofix) + check_import_blocks(tracker, locator, settings, autofix) } diff --git a/src/isort/plugins.rs b/src/isort/plugins.rs index d40a8b84def35..ff2c6c28cb584 100644 --- a/src/isort/plugins.rs +++ b/src/isort/plugins.rs @@ -4,28 +4,50 @@ use textwrap::{dedent, indent}; use crate::ast::types::Range; use crate::autofix::{fixer, Fix}; -use crate::check_ast::Checker; use crate::checks::CheckKind; use crate::docstrings::helpers::leading_space; use crate::isort::sort_imports; use crate::{Check, Settings, SourceCodeLocator}; -// STOPSHIP(charlie): If an import isn't the first or last statement on a line, -// this will remove other valid code. fn extract_range(body: &[&Stmt]) -> Range { - // Extract the range of the existing import block. We extend to include the - // entire first and last line. - let location = body.iter().map(|stmt| stmt.location).min().unwrap(); - let end_location = body - .iter() - .map(|stmt| stmt.end_location) - .max() - .unwrap() - .unwrap(); + let location = body.first().unwrap().location; + let end_location = body.last().unwrap().end_location.unwrap(); Range { + location, + end_location, + } +} + +fn extract_indentation(body: &[&Stmt], locator: &SourceCodeLocator) -> String { + let location = body.first().unwrap().location; + let range = Range { + location: Location::new(location.row(), 0), + end_location: location, + }; + let existing = locator.slice_source_code_range(&range); + leading_space(&existing) +} + +fn match_leading_content(body: &[&Stmt], locator: &SourceCodeLocator) -> bool { + let location = body.first().unwrap().location; + let range = Range { location: Location::new(location.row(), 0), + end_location: location, + }; + let prefix = locator.slice_source_code_range(&range); + prefix.chars().any(|char| !char.is_whitespace()) +} + +fn match_trailing_content(body: &[&Stmt], locator: &SourceCodeLocator) -> bool { + let end_location = body.last().unwrap().end_location.unwrap(); + let range = Range { + location: end_location, end_location: Location::new(end_location.row() + 1, 0), - } + }; + let suffix = locator.slice_source_code_range(&range); + suffix + .chars() + .any(|char| !char.is_whitespace() && char != '#') } /// I001 @@ -35,15 +57,12 @@ pub fn check_imports( settings: &Settings, autofix: &fixer::Mode, ) -> Option { - // Extract the existing import block. let range = extract_range(&body); - let existing = locator.slice_source_code_range(&range); + let indentation = extract_indentation(&body, locator); - // Infer existing indentation. - let indentation = leading_space(&existing); - - // Dedent the existing import block. - let actual = dedent(&existing); + // Special-cases: there's leading or trailing content in the import block. + let has_leading_content = match_leading_content(&body, locator); + let has_trailing_content = match_trailing_content(&body, locator); // Generate the sorted import block. let expected = sort_imports( @@ -55,19 +74,41 @@ pub fn check_imports( &settings.isort.extra_standard_library, ); - // Compare the two? - if actual != expected { + if has_leading_content || has_trailing_content { let mut check = Check::new(CheckKind::UnsortedImports, range); if autofix.patch() { + let mut content = String::new(); + if has_leading_content { + // TODO(charlie): Strip semicolon. + content.push('\n'); + } + content.push_str(&indent(&expected, &indentation)); + if has_trailing_content { + // TODO(charlie): Strip semicolon. + content.push('\n'); + } check.amend(Fix::replacement( - indent(&expected, &indentation), + content, range.location, range.end_location, )); } Some(check) } else { - None + let actual = dedent(&locator.slice_source_code_range(&range)); + if actual != expected { + let mut check = Check::new(CheckKind::UnsortedImports, range); + if autofix.patch() { + check.amend(Fix::replacement( + indent(&expected, &indentation), + range.location, + range.end_location, + )); + } + Some(check) + } else { + None + } } } diff --git a/src/isort/track.rs b/src/isort/track.rs index 62a2717a2a1cb..7f13502a4a7d4 100644 --- a/src/isort/track.rs +++ b/src/isort/track.rs @@ -30,8 +30,8 @@ impl<'a> ImportTracker<'a> { } } - pub fn next(&mut self) -> Option> { - self.blocks.pop() + pub fn into_iter(self) -> impl IntoIterator> { + self.blocks.into_iter() } } @@ -169,12 +169,11 @@ where fn visit_cmpop(&mut self, _: &'b Cmpop) {} fn visit_comprehension(&mut self, _: &'b Comprehension) {} fn visit_excepthandler(&mut self, excepthandler: &'b Excepthandler) { - if let ExcepthandlerKind::ExceptHandler { body, .. } = &excepthandler.node { - for stmt in body { - self.visit_stmt(stmt); - } - self.finalize(); + let ExcepthandlerKind::ExceptHandler { body, .. } = &excepthandler.node; + for stmt in body { + self.visit_stmt(stmt); } + self.finalize(); } fn visit_arguments(&mut self, _: &'b Arguments) {} fn visit_arg(&mut self, _: &'b Arg) {} diff --git a/test/local.py b/test/local.py index c39bcc85017bd..dfd0cd3227d48 100644 --- a/test/local.py +++ b/test/local.py @@ -1,19 +1,3 @@ -import foo as bar -import foo as baz -import foo.f1 as g1 -import foo.f1 as g2 -import __future__.bar as foo -import __future__.baz -import __future__.bar as foo -import __future__.baz -import wop -from . import bar -from .boop import bar -from wop import wop as wop -from wop import bop as bop -import foo, bar -import local_from - # [isort] # import __future__.bar as foo # import __future__.baz diff --git a/bar.py b/test_cases/after/bar.py similarity index 80% rename from bar.py rename to test_cases/after/bar.py index c5ab34ca16b91..508c064d6811f 100644 --- a/bar.py +++ b/test_cases/after/bar.py @@ -1,7 +1,5 @@ try: import os import sys - - import x except: import os diff --git a/test_cases/after/comment.py b/test_cases/after/comment.py new file mode 100644 index 0000000000000..d103bde8da272 --- /dev/null +++ b/test_cases/after/comment.py @@ -0,0 +1,3 @@ +import abc +import os # Line 2 +import sys diff --git a/test_cases/after/foo.py b/test_cases/after/foo.py new file mode 100644 index 0000000000000..6a444c8f18e4d --- /dev/null +++ b/test_cases/after/foo.py @@ -0,0 +1,6 @@ +from __future__ import annotations + +import os +import sys + +import bar diff --git a/test_cases/after/prefix.py b/test_cases/after/prefix.py new file mode 100644 index 0000000000000..01e54ab4f4951 --- /dev/null +++ b/test_cases/after/prefix.py @@ -0,0 +1,3 @@ +x = 1 +import sys +import os diff --git a/test_cases/after/suffix.py b/test_cases/after/suffix.py new file mode 100644 index 0000000000000..557bb08a4a5dc --- /dev/null +++ b/test_cases/after/suffix.py @@ -0,0 +1,3 @@ +import sys +import os +x = 1 diff --git a/test_cases/before/bar.py b/test_cases/before/bar.py new file mode 100644 index 0000000000000..e8adff0d1372f --- /dev/null +++ b/test_cases/before/bar.py @@ -0,0 +1,5 @@ +try: + import sys + import os +except: + import os diff --git a/test_cases/before/comment.py b/test_cases/before/comment.py new file mode 100644 index 0000000000000..59a33c913ce62 --- /dev/null +++ b/test_cases/before/comment.py @@ -0,0 +1,3 @@ +import sys +import os # Line 2 +import abc diff --git a/test_cases/before/foo.py b/test_cases/before/foo.py new file mode 100644 index 0000000000000..e2c323e20e8b6 --- /dev/null +++ b/test_cases/before/foo.py @@ -0,0 +1,4 @@ +import sys +import os +from __future__ import annotations +import bar diff --git a/test_cases/before/prefix.py b/test_cases/before/prefix.py new file mode 100644 index 0000000000000..364a6d8dc8ae7 --- /dev/null +++ b/test_cases/before/prefix.py @@ -0,0 +1,2 @@ +x=1;import sys +import os diff --git a/test_cases/before/suffix.py b/test_cases/before/suffix.py new file mode 100644 index 0000000000000..9866710ba90a4 --- /dev/null +++ b/test_cases/before/suffix.py @@ -0,0 +1,2 @@ +import sys +import os; x = 1 From 08640963f1a8d37986227a8ed0f74534eb08d031 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 10 Nov 2022 17:20:12 -0500 Subject: [PATCH 09/10] Disable unicode newlines --- examples/foo.rs | 31 +++++++++++++++++++++++++++++++ foo.py | 15 ++++++++++++--- src/isort/categorize.rs | 2 +- src/isort/mod.rs | 2 +- src/isort/plugins.rs | 6 ++---- src/isort/settings.rs | 12 +----------- src/isort/track.rs | 1 - src/linter.rs | 1 + 8 files changed, 49 insertions(+), 21 deletions(-) create mode 100644 examples/foo.rs diff --git a/examples/foo.rs b/examples/foo.rs new file mode 100644 index 0000000000000..92dae3bc49974 --- /dev/null +++ b/examples/foo.rs @@ -0,0 +1,31 @@ +//! Run round-trip source code generation on a given Python file. + +use std::fs; +use std::path::PathBuf; + +use anyhow::Result; +use clap::Parser; +use ruff::code_gen::SourceGenerator; +use ruff::source_code_locator::SourceCodeLocator; +use rustpython_ast::Location; +use rustpython_parser::parser; + +#[derive(Parser)] +pub struct Cli { + /// Python file to round-trip. + #[arg(required = true)] + file: PathBuf, +} + +pub fn main() -> Result<()> { + let cli = Cli::parse(); + let contents = fs::read_to_string(&cli.file)?; + let locator = SourceCodeLocator::new(&contents); + println!("{:?}", locator.slice_source_code_line(&Location::new(3, 0))); + println!("{:?}", locator.slice_source_code_line(&Location::new(4, 0))); + println!("{:?}", locator.slice_source_code_line(&Location::new(5, 0))); + println!("{:?}", locator.slice_source_code_line(&Location::new(6, 0))); + println!("{:?}", locator.slice_source_code_line(&Location::new(7, 0))); + println!("{:?}", locator.slice_source_code_line(&Location::new(8, 0))); + Ok(()) +} diff --git a/foo.py b/foo.py index a64c31c8b87e4..575d3511486b4 100644 --- a/foo.py +++ b/foo.py @@ -1,3 +1,12 @@ -if True: - x = 1; - y = 1; +# ABC +import os + +# ABC +import ColorDB + +# DEF2 +# DEF1 +import Main # GHI + +from foo import a as b # ghi +from foo import x as y # bef diff --git a/src/isort/categorize.rs b/src/isort/categorize.rs index 24e65aef8d185..67dff2196cc30 100644 --- a/src/isort/categorize.rs +++ b/src/isort/categorize.rs @@ -32,7 +32,7 @@ pub fn categorize( } else if KNOWN_STANDARD_LIBRARY.contains(module_base) { ImportType::StandardLibrary } else { - if find_local(&src_paths, module_base) { + if find_local(src_paths, module_base) { ImportType::FirstParty } else { ImportType::ThirdParty diff --git a/src/isort/mod.rs b/src/isort/mod.rs index 4af086cac4d8b..652334d47fe45 100644 --- a/src/isort/mod.rs +++ b/src/isort/mod.rs @@ -146,7 +146,7 @@ pub fn sort_imports( if let Some(asname) = asname { format!("{} as {}", name, asname) } else { - format!("{}", name) + name.to_string() } }) .collect(); diff --git a/src/isort/plugins.rs b/src/isort/plugins.rs index ff2c6c28cb584..70a1fd9223c4e 100644 --- a/src/isort/plugins.rs +++ b/src/isort/plugins.rs @@ -45,9 +45,7 @@ fn match_trailing_content(body: &[&Stmt], locator: &SourceCodeLocator) -> bool { end_location: Location::new(end_location.row() + 1, 0), }; let suffix = locator.slice_source_code_range(&range); - suffix - .chars() - .any(|char| !char.is_whitespace() && char != '#') + suffix.chars().any(|char| !char.is_whitespace()) } /// I001 @@ -127,7 +125,7 @@ fn expected(body: Vec<&Stmt>, locator: &SourceCodeLocator) -> String { let expected = sort_imports( body, &100, - &vec![path_dedot::CWD.clone()], + &[path_dedot::CWD.clone()], &Default::default(), &Default::default(), &Default::default(), diff --git a/src/isort/settings.rs b/src/isort/settings.rs index 923d5fb37db0a..ba5d2e1365ebe 100644 --- a/src/isort/settings.rs +++ b/src/isort/settings.rs @@ -12,7 +12,7 @@ pub struct Options { pub extra_standard_library: Option>, } -#[derive(Debug, Hash)] +#[derive(Debug, Hash, Default)] pub struct Settings { pub known_first_party: BTreeSet, pub known_third_party: BTreeSet, @@ -30,13 +30,3 @@ impl Settings { } } } - -impl Default for Settings { - fn default() -> Self { - Self { - known_first_party: BTreeSet::new(), - known_third_party: BTreeSet::new(), - extra_standard_library: BTreeSet::new(), - } - } -} diff --git a/src/isort/track.rs b/src/isort/track.rs index 7f13502a4a7d4..35705039562ab 100644 --- a/src/isort/track.rs +++ b/src/isort/track.rs @@ -4,7 +4,6 @@ use rustpython_ast::{ Unaryop, Withitem, }; -use crate::ast::visitor; use crate::ast::visitor::Visitor; #[derive(Debug)] diff --git a/src/linter.rs b/src/linter.rs index 83769bc727a15..b61f5c5809523 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -60,6 +60,7 @@ pub(crate) fn check_path( settings: &Settings, autofix: &fixer::Mode, ) -> Result> { + println!("{:?}", path); // Aggregate all checks. let mut checks: Vec = vec![]; From 4bd13d301aaa59a39ae0eb7439b4fdfd2c60cceb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 10 Nov 2022 17:41:56 -0500 Subject: [PATCH 10/10] Add tests --- README.md | 54 ++++++-- examples/foo.rs | 31 ----- fast.py | 7 - flake8_to_ruff/src/converter.rs | 14 ++ foo.py | 12 -- .../fixtures/isort/combine_import_froms.py | 5 + .../fixtures/isort/deduplicate_imports.py | 4 + .../test/fixtures/isort/fit_line_length.py | 1 + .../isort/import_from_after_import.py | 2 + .../test/fixtures/isort/leading_prefix.py | 6 + .../isort/no_reorder_within_section.py | 4 +- .../fixtures/isort/preserve_indentation.py | 5 +- .../fixtures/isort/reorder_within_section.py | 1 - .../isort/separate_first_party_imports.py | 5 + .../fixtures/isort/separate_future_imports.py | 1 - .../isort/separate_third_party_imports.py | 4 + .../test/fixtures/isort/trailing_suffix.py | 6 + src/checks.rs | 8 +- src/isort/categorize.rs | 4 +- src/isort/mod.rs | 69 +++++++++- src/isort/plugins.rs | 129 ++---------------- ...isort__tests__combine_import_froms.py.snap | 22 +++ ..._isort__tests__deduplicate_imports.py.snap | 22 +++ ...uff__isort__tests__fit_line_length.py.snap | 6 + ...t__tests__import_from_after_import.py.snap | 22 +++ ...ruff__isort__tests__leading_prefix.py.snap | 39 ++++++ ...__tests__no_reorder_within_section.py.snap | 6 + ...isort__tests__preserve_indentation.py.snap | 39 ++++++ ...ort__tests__reorder_within_section.py.snap | 22 +++ ...ests__separate_first_party_imports.py.snap | 22 +++ ...rt__tests__separate_future_imports.py.snap | 22 +++ ...ests__separate_third_party_imports.py.snap | 22 +++ ...uff__isort__tests__trailing_suffix.py.snap | 39 ++++++ src/isort/track.rs | 17 +++ src/linter.rs | 1 - src/settings/configuration.rs | 11 +- src/settings/mod.rs | 8 +- src/settings/options.rs | 2 +- src/settings/pyproject.rs | 12 +- src/settings/user.rs | 4 +- test/foo.py | 41 ------ test/local.py | 16 --- test/local_from.py | 0 test/pyproject.toml | 6 - test_cases/after/comment.py | 3 - test_cases/after/foo.py | 6 - test_cases/before/bar.py | 5 - test_cases/before/comment.py | 3 - test_cases/before/prefix.py | 2 - test_cases/before/suffix.py | 2 - 50 files changed, 498 insertions(+), 296 deletions(-) delete mode 100644 examples/foo.rs delete mode 100644 fast.py delete mode 100644 foo.py create mode 100644 resources/test/fixtures/isort/combine_import_froms.py create mode 100644 resources/test/fixtures/isort/deduplicate_imports.py create mode 100644 resources/test/fixtures/isort/fit_line_length.py create mode 100644 resources/test/fixtures/isort/import_from_after_import.py create mode 100644 resources/test/fixtures/isort/leading_prefix.py rename test_cases/after/prefix.py => resources/test/fixtures/isort/no_reorder_within_section.py (77%) rename test_cases/after/bar.py => resources/test/fixtures/isort/preserve_indentation.py (58%) rename test_cases/after/suffix.py => resources/test/fixtures/isort/reorder_within_section.py (77%) create mode 100644 resources/test/fixtures/isort/separate_first_party_imports.py rename test_cases/before/foo.py => resources/test/fixtures/isort/separate_future_imports.py (83%) create mode 100644 resources/test/fixtures/isort/separate_third_party_imports.py create mode 100644 resources/test/fixtures/isort/trailing_suffix.py create mode 100644 src/isort/snapshots/ruff__isort__tests__combine_import_froms.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__deduplicate_imports.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__fit_line_length.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__import_from_after_import.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__leading_prefix.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__no_reorder_within_section.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__preserve_indentation.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__reorder_within_section.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__separate_first_party_imports.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__separate_future_imports.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__separate_third_party_imports.py.snap create mode 100644 src/isort/snapshots/ruff__isort__tests__trailing_suffix.py.snap delete mode 100644 test/foo.py delete mode 100644 test/local.py delete mode 100644 test/local_from.py delete mode 100644 test/pyproject.toml delete mode 100644 test_cases/after/comment.py delete mode 100644 test_cases/after/foo.py delete mode 100644 test_cases/before/bar.py delete mode 100644 test_cases/before/comment.py delete mode 100644 test_cases/before/prefix.py delete mode 100644 test_cases/before/suffix.py diff --git a/README.md b/README.md index 0f98e6223f0ed..96d8bdc95e5e4 100644 --- a/README.md +++ b/README.md @@ -27,9 +27,10 @@ An extremely fast Python linter, written in Rust. Ruff aims to be orders of magnitude faster than alternative tools while integrating more functionality behind a single, common interface. Ruff can be used to replace Flake8 (plus a variety -of plugins), [`pydocstyle`](https://pypi.org/project/pydocstyle/), [`yesqa`](https://github.com/asottile/yesqa), -and even a subset of [`pyupgrade`](https://pypi.org/project/pyupgrade/) and [`autoflake`](https://pypi.org/project/autoflake/) -all while executing tens or hundreds of times faster than any individual tool. +of plugins), [`isort`](https://pypi.org/project/isort/), [`pydocstyle`](https://pypi.org/project/pydocstyle/), +[`yesqa`](https://github.com/asottile/yesqa), and even a subset of [`pyupgrade`](https://pypi.org/project/pyupgrade/) +and [`autoflake`](https://pypi.org/project/autoflake/) all while executing tens or hundreds of times +faster than any individual tool. (Coming from Flake8? Try [`flake8-to-ruff`](https://pypi.org/project/flake8-to-ruff/) to automatically convert your existing configuration.) @@ -285,16 +286,16 @@ Ruff supports several workflows to aid in `noqa` management. First, Ruff provides a special error code, `M001`, to enforce that your `noqa` directives are "valid", in that the errors they _say_ they ignore are actually being triggered on that line (and -thus suppressed). **You can run `ruff /path/to/file.py --extend-select M001` to flag unused `noqa` -directives.** +thus suppressed). You can run `ruff /path/to/file.py --extend-select M001` to flag unused `noqa` +directives. Second, Ruff can _automatically remove_ unused `noqa` directives via its autofix functionality. -**You can run `ruff /path/to/file.py --extend-select M001 --fix` to automatically remove unused -`noqa` directives.** +You can run `ruff /path/to/file.py --extend-select M001 --fix` to automatically remove unused +`noqa` directives. Third, Ruff can _automatically add_ `noqa` directives to all failing lines. This is useful when -migrating a new codebase to Ruff. **You can run `ruff /path/to/file.py --add-noqa` to automatically -add `noqa` directives to all failing lines, with the appropriate error codes.** +migrating a new codebase to Ruff. You can run `ruff /path/to/file.py --add-noqa` to automatically +add `noqa` directives to all failing lines, with the appropriate error codes. ## Supported Rules @@ -365,6 +366,14 @@ For more, see [pycodestyle](https://pypi.org/project/pycodestyle/2.9.1/) on PyPI | W292 | NoNewLineAtEndOfFile | No newline at end of file | | | W605 | InvalidEscapeSequence | Invalid escape sequence: '\c' | | +### isort + +For more, see [isort](https://pypi.org/project/isort/5.10.1/) on PyPI. + +| Code | Name | Message | Fix | +| ---- | ---- | ------- | --- | +| I001 | UnsortedImports | Import block is un-sorted or un-formatted | 🛠 | + ### pydocstyle For more, see [pydocstyle](https://pypi.org/project/pydocstyle/6.1.1/) on PyPI. @@ -681,7 +690,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) - [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (19/32) -Ruff also implements the functionality that you get from [`yesqa`](https://github.com/asottile/yesqa), +Ruff can also replace [`isort`](https://pypi.org/project/isort/), [`yesqa`](https://github.com/asottile/yesqa), and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34). If you're looking to use Ruff, but rely on an unsupported Flake8 plugin, free to file an Issue. @@ -702,6 +711,31 @@ on Rust at all. Ruff does not yet support third-party plugins, though a plugin system is within-scope for the project. See [#283](https://github.com/charliermarsh/ruff/issues/283) for more. +### How does Ruff's import sorting compare to [`isort`](https://pypi.org/project/isort/)? + +Ruff's import sorting is intended to be equivalent to `isort` when used `profile = "black"` and +`combine_as_imports = true`. Like `isort`, Ruff's import sorting is compatible with Black. + +Ruff is less configurable than `isort`, but supports the `known-first-party`, `known-third-party`, +`extra-standard-library`, and `src` settings, like so: + +```toml +[tool.ruff] +select = [ + # Pyflakes + "F", + # Pycodestyle + "E", + "W", + # isort + "I" +] +src = ["src", "tests"] + +[tool.ruff.isort] +known-first-party = ["my_module1", "my_module2"] +``` + ### Does Ruff support NumPy- or Google-style docstrings? Yes! To enable a specific docstring convention, start by enabling all `pydocstyle` error codes, and diff --git a/examples/foo.rs b/examples/foo.rs deleted file mode 100644 index 92dae3bc49974..0000000000000 --- a/examples/foo.rs +++ /dev/null @@ -1,31 +0,0 @@ -//! Run round-trip source code generation on a given Python file. - -use std::fs; -use std::path::PathBuf; - -use anyhow::Result; -use clap::Parser; -use ruff::code_gen::SourceGenerator; -use ruff::source_code_locator::SourceCodeLocator; -use rustpython_ast::Location; -use rustpython_parser::parser; - -#[derive(Parser)] -pub struct Cli { - /// Python file to round-trip. - #[arg(required = true)] - file: PathBuf, -} - -pub fn main() -> Result<()> { - let cli = Cli::parse(); - let contents = fs::read_to_string(&cli.file)?; - let locator = SourceCodeLocator::new(&contents); - println!("{:?}", locator.slice_source_code_line(&Location::new(3, 0))); - println!("{:?}", locator.slice_source_code_line(&Location::new(4, 0))); - println!("{:?}", locator.slice_source_code_line(&Location::new(5, 0))); - println!("{:?}", locator.slice_source_code_line(&Location::new(6, 0))); - println!("{:?}", locator.slice_source_code_line(&Location::new(7, 0))); - println!("{:?}", locator.slice_source_code_line(&Location::new(8, 0))); - Ok(()) -} diff --git a/fast.py b/fast.py deleted file mode 100644 index 194174c826ef1..0000000000000 --- a/fast.py +++ /dev/null @@ -1,7 +0,0 @@ -# comment 1a -# comment 1b -# comment 2a -# comment 2b -from .param_functions import Body # comment 1c -from .param_functions import Header # comment 1d -from .param_functions import Cookie, Request diff --git a/flake8_to_ruff/src/converter.rs b/flake8_to_ruff/src/converter.rs index 69ec13aba440c..30f8b5f0f3bc3 100644 --- a/flake8_to_ruff/src/converter.rs +++ b/flake8_to_ruff/src/converter.rs @@ -208,6 +208,7 @@ mod tests { let actual = convert(&HashMap::from([]), None)?; let expected = Pyproject::new(Options { line_length: None, + src: None, fix: None, exclude: None, extend_exclude: None, @@ -224,6 +225,7 @@ mod tests { target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }); assert_eq!(actual, expected); @@ -239,6 +241,7 @@ mod tests { )?; let expected = Pyproject::new(Options { line_length: Some(100), + src: None, fix: None, exclude: None, extend_exclude: None, @@ -255,6 +258,7 @@ mod tests { target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }); assert_eq!(actual, expected); @@ -270,6 +274,7 @@ mod tests { )?; let expected = Pyproject::new(Options { line_length: Some(100), + src: None, fix: None, exclude: None, extend_exclude: None, @@ -286,6 +291,7 @@ mod tests { target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }); assert_eq!(actual, expected); @@ -301,6 +307,7 @@ mod tests { )?; let expected = Pyproject::new(Options { line_length: None, + src: None, fix: None, exclude: None, extend_exclude: None, @@ -317,6 +324,7 @@ mod tests { target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }); assert_eq!(actual, expected); @@ -332,6 +340,7 @@ mod tests { )?; let expected = Pyproject::new(Options { line_length: None, + src: None, fix: None, exclude: None, extend_exclude: None, @@ -353,6 +362,7 @@ mod tests { docstring_quotes: None, avoid_escape: None, }), + isort: None, pep8_naming: None, }); assert_eq!(actual, expected); @@ -371,6 +381,7 @@ mod tests { )?; let expected = Pyproject::new(Options { line_length: None, + src: None, fix: None, exclude: None, extend_exclude: None, @@ -422,6 +433,7 @@ mod tests { target_version: None, flake8_annotations: None, flake8_quotes: None, + isort: None, pep8_naming: None, }); assert_eq!(actual, expected); @@ -437,6 +449,7 @@ mod tests { )?; let expected = Pyproject::new(Options { line_length: None, + src: None, fix: None, exclude: None, extend_exclude: None, @@ -459,6 +472,7 @@ mod tests { docstring_quotes: None, avoid_escape: None, }), + isort: None, pep8_naming: None, }); assert_eq!(actual, expected); diff --git a/foo.py b/foo.py deleted file mode 100644 index 575d3511486b4..0000000000000 --- a/foo.py +++ /dev/null @@ -1,12 +0,0 @@ -# ABC -import os - -# ABC -import ColorDB - -# DEF2 -# DEF1 -import Main # GHI - -from foo import a as b # ghi -from foo import x as y # bef diff --git a/resources/test/fixtures/isort/combine_import_froms.py b/resources/test/fixtures/isort/combine_import_froms.py new file mode 100644 index 0000000000000..e4deb2f46ac7b --- /dev/null +++ b/resources/test/fixtures/isort/combine_import_froms.py @@ -0,0 +1,5 @@ +from collections import Awaitable +from collections import AsyncIterable +from collections import Collection +from collections import ChainMap +from collections import MutableSequence, MutableMapping diff --git a/resources/test/fixtures/isort/deduplicate_imports.py b/resources/test/fixtures/isort/deduplicate_imports.py new file mode 100644 index 0000000000000..310bbfed316a5 --- /dev/null +++ b/resources/test/fixtures/isort/deduplicate_imports.py @@ -0,0 +1,4 @@ +import os +import os +import os as os1 +import os as os2 diff --git a/resources/test/fixtures/isort/fit_line_length.py b/resources/test/fixtures/isort/fit_line_length.py new file mode 100644 index 0000000000000..6d0a5714aa7da --- /dev/null +++ b/resources/test/fixtures/isort/fit_line_length.py @@ -0,0 +1 @@ +from collections import Collection diff --git a/resources/test/fixtures/isort/import_from_after_import.py b/resources/test/fixtures/isort/import_from_after_import.py new file mode 100644 index 0000000000000..75b6abc9f5d39 --- /dev/null +++ b/resources/test/fixtures/isort/import_from_after_import.py @@ -0,0 +1,2 @@ +from collections import Collection +import os diff --git a/resources/test/fixtures/isort/leading_prefix.py b/resources/test/fixtures/isort/leading_prefix.py new file mode 100644 index 0000000000000..e54d98ae50f9d --- /dev/null +++ b/resources/test/fixtures/isort/leading_prefix.py @@ -0,0 +1,6 @@ +x = 1; import sys +import os + +if True: + x = 1; import sys + import os diff --git a/test_cases/after/prefix.py b/resources/test/fixtures/isort/no_reorder_within_section.py similarity index 77% rename from test_cases/after/prefix.py rename to resources/test/fixtures/isort/no_reorder_within_section.py index 01e54ab4f4951..72017982278a9 100644 --- a/test_cases/after/prefix.py +++ b/resources/test/fixtures/isort/no_reorder_within_section.py @@ -1,3 +1,3 @@ -x = 1 -import sys +# OK import os +import sys diff --git a/test_cases/after/bar.py b/resources/test/fixtures/isort/preserve_indentation.py similarity index 58% rename from test_cases/after/bar.py rename to resources/test/fixtures/isort/preserve_indentation.py index 508c064d6811f..44d1e3e8675c2 100644 --- a/test_cases/after/bar.py +++ b/resources/test/fixtures/isort/preserve_indentation.py @@ -1,5 +1,6 @@ -try: +if True: + import sys import os +else: import sys -except: import os diff --git a/test_cases/after/suffix.py b/resources/test/fixtures/isort/reorder_within_section.py similarity index 77% rename from test_cases/after/suffix.py rename to resources/test/fixtures/isort/reorder_within_section.py index 557bb08a4a5dc..82ca618b41da0 100644 --- a/test_cases/after/suffix.py +++ b/resources/test/fixtures/isort/reorder_within_section.py @@ -1,3 +1,2 @@ import sys import os -x = 1 diff --git a/resources/test/fixtures/isort/separate_first_party_imports.py b/resources/test/fixtures/isort/separate_first_party_imports.py new file mode 100644 index 0000000000000..9d7ac6fa5c611 --- /dev/null +++ b/resources/test/fixtures/isort/separate_first_party_imports.py @@ -0,0 +1,5 @@ +import sys +import leading_prefix +import numpy as np +import os +from leading_prefix import Class diff --git a/test_cases/before/foo.py b/resources/test/fixtures/isort/separate_future_imports.py similarity index 83% rename from test_cases/before/foo.py rename to resources/test/fixtures/isort/separate_future_imports.py index e2c323e20e8b6..07f8b2f675771 100644 --- a/test_cases/before/foo.py +++ b/resources/test/fixtures/isort/separate_future_imports.py @@ -1,4 +1,3 @@ import sys import os from __future__ import annotations -import bar diff --git a/resources/test/fixtures/isort/separate_third_party_imports.py b/resources/test/fixtures/isort/separate_third_party_imports.py new file mode 100644 index 0000000000000..b3450fd69a462 --- /dev/null +++ b/resources/test/fixtures/isort/separate_third_party_imports.py @@ -0,0 +1,4 @@ +import pandas as pd +import sys +import numpy as np +import os diff --git a/resources/test/fixtures/isort/trailing_suffix.py b/resources/test/fixtures/isort/trailing_suffix.py new file mode 100644 index 0000000000000..2a47c056ec455 --- /dev/null +++ b/resources/test/fixtures/isort/trailing_suffix.py @@ -0,0 +1,6 @@ +import sys +import os; x = 1 + +if True: + import sys + import os; x = 1 diff --git a/src/checks.rs b/src/checks.rs index 380ffce0e34a1..0970cdcc7a97d 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -218,6 +218,7 @@ pub enum CheckCode { pub enum CheckCategory { Pyflakes, Pycodestyle, + Isort, Pydocstyle, Pyupgrade, PEP8Naming, @@ -227,7 +228,6 @@ pub enum CheckCategory { Flake8Print, Flake8Quotes, Flake8Annotations, - ISort, Ruff, Meta, } @@ -237,6 +237,7 @@ impl CheckCategory { match self { CheckCategory::Pycodestyle => "pycodestyle", CheckCategory::Pyflakes => "Pyflakes", + CheckCategory::Isort => "isort", CheckCategory::Flake8Builtins => "flake8-builtins", CheckCategory::Flake8Bugbear => "flake8-bugbear", CheckCategory::Flake8Comprehensions => "flake8-comprehensions", @@ -246,7 +247,6 @@ impl CheckCategory { CheckCategory::Pyupgrade => "pyupgrade", CheckCategory::Pydocstyle => "pydocstyle", CheckCategory::PEP8Naming => "pep8-naming", - CheckCategory::ISort => "isort", CheckCategory::Ruff => "Ruff-specific rules", CheckCategory::Meta => "Meta rules", } @@ -256,6 +256,7 @@ impl CheckCategory { match self { CheckCategory::Pycodestyle => Some("https://pypi.org/project/pycodestyle/2.9.1/"), CheckCategory::Pyflakes => Some("https://pypi.org/project/pyflakes/2.5.0/"), + CheckCategory::Isort => Some("https://pypi.org/project/isort/5.10.1/"), CheckCategory::Flake8Builtins => { Some("https://pypi.org/project/flake8-builtins/2.0.1/") } @@ -273,7 +274,6 @@ impl CheckCategory { CheckCategory::Pyupgrade => Some("https://pypi.org/project/pyupgrade/3.2.0/"), CheckCategory::Pydocstyle => Some("https://pypi.org/project/pydocstyle/6.1.1/"), CheckCategory::PEP8Naming => Some("https://pypi.org/project/pep8-naming/0.13.2/"), - CheckCategory::ISort => Some("https://pypi.org/project/isort/5.10.1/"), CheckCategory::Ruff => None, CheckCategory::Meta => None, } @@ -906,7 +906,7 @@ impl CheckCode { CheckCode::N816 => CheckCategory::PEP8Naming, CheckCode::N817 => CheckCategory::PEP8Naming, CheckCode::N818 => CheckCategory::PEP8Naming, - CheckCode::I001 => CheckCategory::ISort, + CheckCode::I001 => CheckCategory::Isort, CheckCode::RUF001 => CheckCategory::Ruff, CheckCode::RUF002 => CheckCategory::Ruff, CheckCode::RUF003 => CheckCategory::Ruff, diff --git a/src/isort/categorize.rs b/src/isort/categorize.rs index 67dff2196cc30..11ffd4f101c03 100644 --- a/src/isort/categorize.rs +++ b/src/isort/categorize.rs @@ -16,7 +16,7 @@ pub enum ImportType { pub fn categorize( module_base: &str, - src_paths: &[PathBuf], + src: &[PathBuf], known_first_party: &BTreeSet, known_third_party: &BTreeSet, extra_standard_library: &BTreeSet, @@ -32,7 +32,7 @@ pub fn categorize( } else if KNOWN_STANDARD_LIBRARY.contains(module_base) { ImportType::StandardLibrary } else { - if find_local(src_paths, module_base) { + if find_local(src, module_base) { ImportType::FirstParty } else { ImportType::ThirdParty diff --git a/src/isort/mod.rs b/src/isort/mod.rs index 652334d47fe45..d404f1c493664 100644 --- a/src/isort/mod.rs +++ b/src/isort/mod.rs @@ -52,7 +52,7 @@ fn normalize_imports<'a>(imports: &'a [&'a Stmt]) -> ImportBlock<'a> { fn categorize_imports<'a>( block: ImportBlock<'a>, - src_paths: &[PathBuf], + src: &[PathBuf], known_first_party: &BTreeSet, known_third_party: &BTreeSet, extra_standard_library: &BTreeSet, @@ -62,7 +62,7 @@ fn categorize_imports<'a>( for alias in block.import { let import_type = categorize( &alias.module_base(), - src_paths, + src, known_first_party, known_third_party, extra_standard_library, @@ -77,7 +77,7 @@ fn categorize_imports<'a>( for (import_from, aliases) in block.import_from { let classification = categorize( &import_from.module_base(), - src_paths, + src, known_first_party, known_third_party, extra_standard_library, @@ -94,7 +94,7 @@ fn categorize_imports<'a>( pub fn sort_imports( block: Vec<&Stmt>, line_length: &usize, - src_paths: &[PathBuf], + src: &[PathBuf], known_first_party: &BTreeSet, known_third_party: &BTreeSet, extra_standard_library: &BTreeSet, @@ -105,7 +105,7 @@ pub fn sort_imports( // Categorize by type (e.g., first-party vs. third-party). let block_by_type = categorize_imports( block, - src_paths, + src, known_first_party, known_third_party, extra_standard_library, @@ -195,3 +195,62 @@ pub fn sort_imports( } output.finish().to_string() } + +#[cfg(test)] +mod tests { + use std::path::Path; + + use anyhow::Result; + use rustpython_parser::lexer::LexResult; + use test_case::test_case; + + use crate::autofix::fixer; + use crate::checks::{Check, CheckCode}; + use crate::linter::tokenize; + use crate::{fs, linter, noqa, Settings, SourceCodeLocator}; + + fn check_path(path: &Path, settings: &Settings, autofix: &fixer::Mode) -> Result> { + let contents = fs::read_file(path)?; + let tokens: Vec = tokenize(&contents); + let locator = SourceCodeLocator::new(&contents); + let noqa_line_for = noqa::extract_noqa_line_for(&tokens); + linter::check_path( + path, + &contents, + tokens, + &locator, + &noqa_line_for, + settings, + autofix, + ) + } + + #[test_case(Path::new("reorder_within_section.py"))] + #[test_case(Path::new("no_reorder_within_section.py"))] + #[test_case(Path::new("separate_future_imports.py"))] + #[test_case(Path::new("separate_third_party_imports.py"))] + #[test_case(Path::new("separate_first_party_imports.py"))] + #[test_case(Path::new("deduplicate_imports.py"))] + #[test_case(Path::new("combine_import_froms.py"))] + #[test_case(Path::new("preserve_indentation.py"))] + #[test_case(Path::new("fit_line_length.py"))] + #[test_case(Path::new("import_from_after_import.py"))] + #[test_case(Path::new("leading_prefix.py"))] + #[test_case(Path::new("trailing_suffix.py"))] + fn isort(path: &Path) -> Result<()> { + let snapshot = format!("{}", path.to_string_lossy()); + let mut checks = check_path( + Path::new("./resources/test/fixtures/isort") + .join(path) + .as_path(), + &Settings { + src: vec![Path::new("resources/test/fixtures/isort").to_path_buf()], + ..Settings::for_rule(CheckCode::I001) + }, + &fixer::Mode::Generate, + )?; + checks.sort_by_key(|check| check.location); + insta::assert_yaml_snapshot!(snapshot, checks); + Ok(()) + } +} diff --git a/src/isort/plugins.rs b/src/isort/plugins.rs index 70a1fd9223c4e..968c183f9d323 100644 --- a/src/isort/plugins.rs +++ b/src/isort/plugins.rs @@ -1,4 +1,3 @@ -use path_absolutize::path_dedot; use rustpython_ast::{Location, Stmt}; use textwrap::{dedent, indent}; @@ -66,7 +65,7 @@ pub fn check_imports( let expected = sort_imports( body, &settings.line_length, - &settings.src_paths, + &settings.src, &settings.isort.known_first_party, &settings.isort.known_third_party, &settings.isort.extra_standard_library, @@ -77,22 +76,28 @@ pub fn check_imports( if autofix.patch() { let mut content = String::new(); if has_leading_content { - // TODO(charlie): Strip semicolon. content.push('\n'); } content.push_str(&indent(&expected, &indentation)); - if has_trailing_content { - // TODO(charlie): Strip semicolon. - content.push('\n'); - } check.amend(Fix::replacement( content, - range.location, - range.end_location, + // Preserve leading prefix (but put the imports on a new line). + if has_leading_content { + range.location + } else { + Location::new(range.location.row(), 0) + }, + // TODO(charlie): Preserve trailing suffixes. Right now, we strip them. + Location::new(range.end_location.row() + 1, 0), )); } Some(check) } else { + // Expand the span the entire range, including leading and trailing space. + let range = Range { + location: Location::new(range.location.row(), 0), + end_location: Location::new(range.end_location.row() + 1, 0), + }; let actual = dedent(&locator.slice_source_code_range(&range)); if actual != expected { let mut check = Check::new(CheckKind::UnsortedImports, range); @@ -109,109 +114,3 @@ pub fn check_imports( } } } - -// STOPSHIP(charlie): Exists for testing. -fn actual(body: &[&Stmt], locator: &SourceCodeLocator) -> String { - let range = extract_range(body); - let existing = locator.slice_source_code_range(&range); - dedent(&existing) -} - -// STOPSHIP(charlie): Exists for testing. -fn expected(body: Vec<&Stmt>, locator: &SourceCodeLocator) -> String { - let range = extract_range(&body); - let existing = locator.slice_source_code_range(&range); - let indentation = leading_space(&existing); - let expected = sort_imports( - body, - &100, - &[path_dedot::CWD.clone()], - &Default::default(), - &Default::default(), - &Default::default(), - ); - indent(&expected, &indentation) -} - -#[cfg(test)] -mod tests { - use anyhow::Result; - use rustpython_ast::Stmt; - use rustpython_parser::parser; - - use crate::isort::plugins::{actual, expected}; - use crate::SourceCodeLocator; - - #[test] - fn single() -> Result<()> { - let contents = r#"import os -"#; - let suite = parser::parse_program(contents, "")?; - let locator = SourceCodeLocator::new(&contents); - - let body: Vec<&Stmt> = suite.iter().collect(); - - let actual = actual(&body, &locator); - let expected = expected(body, &locator); - - assert_eq!(actual, expected); - - Ok(()) - } - - #[test] - fn double() -> Result<()> { - let contents = r#"import sys -import os -"#; - let suite = parser::parse_program(contents, "")?; - let locator = SourceCodeLocator::new(&contents); - let body: Vec<&Stmt> = suite.iter().collect(); - - let actual = actual(&body, &locator); - assert_eq!( - actual, - r#"import sys -import os -"# - ); - - let expected = expected(body, &locator); - assert_eq!( - expected, - r#"import os -import sys -"# - ); - - Ok(()) - } - - #[test] - fn indented() -> Result<()> { - let contents = r#" import sys - import os -"#; - let suite = parser::parse_program(contents, "")?; - let locator = SourceCodeLocator::new(&contents); - let body: Vec<&Stmt> = suite.iter().collect(); - - let actual = actual(&body, &locator); - assert_eq!( - actual, - r#"import sys -import os -"# - ); - - let expected = expected(body, &locator); - assert_eq!( - expected, - r#"import os -import sys -"# - ); - - Ok(()) - } -} diff --git a/src/isort/snapshots/ruff__isort__tests__combine_import_froms.py.snap b/src/isort/snapshots/ruff__isort__tests__combine_import_froms.py.snap new file mode 100644 index 0000000000000..d68593a497dc5 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__combine_import_froms.py.snap @@ -0,0 +1,22 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 6 + column: 0 + fix: + patch: + content: "from collections import (\n AsyncIterable,\n Awaitable,\n ChainMap,\n Collection,\n MutableMapping,\n MutableSequence,\n)\n" + location: + row: 1 + column: 0 + end_location: + row: 6 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__deduplicate_imports.py.snap b/src/isort/snapshots/ruff__isort__tests__deduplicate_imports.py.snap new file mode 100644 index 0000000000000..15d6208421142 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__deduplicate_imports.py.snap @@ -0,0 +1,22 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 5 + column: 0 + fix: + patch: + content: "import os\nimport os as os1\nimport os as os2\n" + location: + row: 1 + column: 0 + end_location: + row: 5 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__fit_line_length.py.snap b/src/isort/snapshots/ruff__isort__tests__fit_line_length.py.snap new file mode 100644 index 0000000000000..6b7d975e4e336 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__fit_line_length.py.snap @@ -0,0 +1,6 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +[] + diff --git a/src/isort/snapshots/ruff__isort__tests__import_from_after_import.py.snap b/src/isort/snapshots/ruff__isort__tests__import_from_after_import.py.snap new file mode 100644 index 0000000000000..7b456e2bb5e8b --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__import_from_after_import.py.snap @@ -0,0 +1,22 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 3 + column: 0 + fix: + patch: + content: "import os\nfrom collections import Collection\n" + location: + row: 1 + column: 0 + end_location: + row: 3 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__leading_prefix.py.snap b/src/isort/snapshots/ruff__isort__tests__leading_prefix.py.snap new file mode 100644 index 0000000000000..cdac8665fbc78 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__leading_prefix.py.snap @@ -0,0 +1,39 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 7 + end_location: + row: 2 + column: 9 + fix: + patch: + content: "\nimport os\nimport sys\n" + location: + row: 1 + column: 7 + end_location: + row: 3 + column: 0 + applied: false +- kind: UnsortedImports + location: + row: 5 + column: 11 + end_location: + row: 6 + column: 13 + fix: + patch: + content: "\n import os\n import sys\n" + location: + row: 5 + column: 11 + end_location: + row: 7 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__no_reorder_within_section.py.snap b/src/isort/snapshots/ruff__isort__tests__no_reorder_within_section.py.snap new file mode 100644 index 0000000000000..6b7d975e4e336 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__no_reorder_within_section.py.snap @@ -0,0 +1,6 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +[] + diff --git a/src/isort/snapshots/ruff__isort__tests__preserve_indentation.py.snap b/src/isort/snapshots/ruff__isort__tests__preserve_indentation.py.snap new file mode 100644 index 0000000000000..a8862db9032d7 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__preserve_indentation.py.snap @@ -0,0 +1,39 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 2 + column: 0 + end_location: + row: 4 + column: 0 + fix: + patch: + content: " import os\n import sys\n" + location: + row: 2 + column: 0 + end_location: + row: 4 + column: 0 + applied: false +- kind: UnsortedImports + location: + row: 5 + column: 0 + end_location: + row: 7 + column: 0 + fix: + patch: + content: " import os\n import sys\n" + location: + row: 5 + column: 0 + end_location: + row: 7 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__reorder_within_section.py.snap b/src/isort/snapshots/ruff__isort__tests__reorder_within_section.py.snap new file mode 100644 index 0000000000000..fa55fb28ea994 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__reorder_within_section.py.snap @@ -0,0 +1,22 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 3 + column: 0 + fix: + patch: + content: "import os\nimport sys\n" + location: + row: 1 + column: 0 + end_location: + row: 3 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__separate_first_party_imports.py.snap b/src/isort/snapshots/ruff__isort__tests__separate_first_party_imports.py.snap new file mode 100644 index 0000000000000..550de04b09586 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__separate_first_party_imports.py.snap @@ -0,0 +1,22 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 6 + column: 0 + fix: + patch: + content: "import os\nimport sys\n\nimport numpy as np\n\nimport leading_prefix\nfrom leading_prefix import Class\n" + location: + row: 1 + column: 0 + end_location: + row: 6 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__separate_future_imports.py.snap b/src/isort/snapshots/ruff__isort__tests__separate_future_imports.py.snap new file mode 100644 index 0000000000000..b6a38b733f8cb --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__separate_future_imports.py.snap @@ -0,0 +1,22 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 4 + column: 0 + fix: + patch: + content: "from __future__ import annotations\n\nimport os\nimport sys\n" + location: + row: 1 + column: 0 + end_location: + row: 4 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__separate_third_party_imports.py.snap b/src/isort/snapshots/ruff__isort__tests__separate_third_party_imports.py.snap new file mode 100644 index 0000000000000..e9f90d64d0720 --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__separate_third_party_imports.py.snap @@ -0,0 +1,22 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 5 + column: 0 + fix: + patch: + content: "import os\nimport sys\n\nimport numpy as np\nimport pandas as pd\n" + location: + row: 1 + column: 0 + end_location: + row: 5 + column: 0 + applied: false + diff --git a/src/isort/snapshots/ruff__isort__tests__trailing_suffix.py.snap b/src/isort/snapshots/ruff__isort__tests__trailing_suffix.py.snap new file mode 100644 index 0000000000000..c5384f748b5ca --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__trailing_suffix.py.snap @@ -0,0 +1,39 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 2 + column: 9 + fix: + patch: + content: "import os\nimport sys\n" + location: + row: 1 + column: 0 + end_location: + row: 3 + column: 0 + applied: false +- kind: UnsortedImports + location: + row: 5 + column: 4 + end_location: + row: 6 + column: 13 + fix: + patch: + content: " import os\n import sys\n" + location: + row: 5 + column: 0 + end_location: + row: 7 + column: 0 + applied: false + diff --git a/src/isort/track.rs b/src/isort/track.rs index 35705039562ab..9bda76a4e6d21 100644 --- a/src/isort/track.rs +++ b/src/isort/track.rs @@ -158,15 +158,25 @@ where _ => {} } } + fn visit_annotation(&mut self, _: &'b Expr) {} + fn visit_expr(&mut self, _: &'b Expr) {} + fn visit_constant(&mut self, _: &'b Constant) {} + fn visit_expr_context(&mut self, _: &'b ExprContext) {} + fn visit_boolop(&mut self, _: &'b Boolop) {} + fn visit_operator(&mut self, _: &'b Operator) {} + fn visit_unaryop(&mut self, _: &'b Unaryop) {} + fn visit_cmpop(&mut self, _: &'b Cmpop) {} + fn visit_comprehension(&mut self, _: &'b Comprehension) {} + fn visit_excepthandler(&mut self, excepthandler: &'b Excepthandler) { let ExcepthandlerKind::ExceptHandler { body, .. } = &excepthandler.node; for stmt in body { @@ -174,16 +184,23 @@ where } self.finalize(); } + fn visit_arguments(&mut self, _: &'b Arguments) {} + fn visit_arg(&mut self, _: &'b Arg) {} + fn visit_keyword(&mut self, _: &'b Keyword) {} + fn visit_alias(&mut self, _: &'b Alias) {} + fn visit_withitem(&mut self, _: &'b Withitem) {} + fn visit_match_case(&mut self, match_case: &'b MatchCase) { for stmt in &match_case.body { self.visit_stmt(stmt); } self.finalize(); } + fn visit_pattern(&mut self, _: &'b Pattern) {} } diff --git a/src/linter.rs b/src/linter.rs index b61f5c5809523..83769bc727a15 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -60,7 +60,6 @@ pub(crate) fn check_path( settings: &Settings, autofix: &fixer::Mode, ) -> Result> { - println!("{:?}", path); // Aggregate all checks. let mut checks: Vec = vec![]; diff --git a/src/settings/configuration.rs b/src/settings/configuration.rs index 7085e006f0e22..b87b4553a3536 100644 --- a/src/settings/configuration.rs +++ b/src/settings/configuration.rs @@ -26,7 +26,7 @@ pub struct Configuration { pub line_length: usize, pub per_file_ignores: Vec, pub select: Vec, - pub src_paths: Vec, + pub src: Vec, pub target_version: PythonVersion, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, @@ -74,11 +74,10 @@ impl Configuration { .map_err(|e| anyhow!("Invalid dummy-variable-rgx value: {e}"))?, None => DEFAULT_DUMMY_VARIABLE_RGX.clone(), }, - src_paths: options - .src_paths - .map(|src_paths| { - src_paths - .iter() + src: options + .src + .map(|src| { + src.iter() .map(|path| { let path = Path::new(path); match project_root { diff --git a/src/settings/mod.rs b/src/settings/mod.rs index 91cf079023906..eade10b63e1dc 100644 --- a/src/settings/mod.rs +++ b/src/settings/mod.rs @@ -29,7 +29,7 @@ pub struct Settings { pub extend_exclude: Vec, pub line_length: usize, pub per_file_ignores: Vec, - pub src_paths: Vec, + pub src: Vec, pub target_version: PythonVersion, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, @@ -56,7 +56,7 @@ impl Settings { line_length: config.line_length, pep8_naming: config.pep8_naming, per_file_ignores: config.per_file_ignores, - src_paths: config.src_paths, + src: config.src, target_version: config.target_version, } } @@ -69,7 +69,7 @@ impl Settings { extend_exclude: Default::default(), line_length: 88, per_file_ignores: Default::default(), - src_paths: vec![path_dedot::CWD.clone()], + src: vec![path_dedot::CWD.clone()], target_version: PythonVersion::Py310, flake8_annotations: Default::default(), flake8_quotes: Default::default(), @@ -86,7 +86,7 @@ impl Settings { extend_exclude: Default::default(), line_length: 88, per_file_ignores: Default::default(), - src_paths: vec![path_dedot::CWD.clone()], + src: vec![path_dedot::CWD.clone()], target_version: PythonVersion::Py310, flake8_annotations: Default::default(), flake8_quotes: Default::default(), diff --git a/src/settings/options.rs b/src/settings/options.rs index ccd6d40e363c9..dcf087fefd1fd 100644 --- a/src/settings/options.rs +++ b/src/settings/options.rs @@ -21,7 +21,7 @@ pub struct Options { pub line_length: Option, pub per_file_ignores: Option>>, pub select: Option>, - pub src_paths: Option>, + pub src: Option>, pub target_version: Option, // Plugins pub flake8_annotations: Option, diff --git a/src/settings/pyproject.rs b/src/settings/pyproject.rs index 9ad2a0243fb4f..802c64a9d0dd6 100644 --- a/src/settings/pyproject.rs +++ b/src/settings/pyproject.rs @@ -143,7 +143,7 @@ mod tests { extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, - src_paths: None, + src: None, target_version: None, flake8_annotations: None, flake8_quotes: None, @@ -174,7 +174,7 @@ line-length = 79 extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, - src_paths: None, + src: None, target_version: None, flake8_annotations: None, flake8_quotes: None, @@ -205,7 +205,7 @@ exclude = ["foo.py"] extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, - src_paths: None, + src: None, target_version: None, flake8_annotations: None, flake8_quotes: None, @@ -236,7 +236,7 @@ select = ["E501"] extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, - src_paths: None, + src: None, target_version: None, flake8_annotations: None, flake8_quotes: None, @@ -268,7 +268,7 @@ ignore = ["E501"] extend_ignore: None, per_file_ignores: None, dummy_variable_rgx: None, - src_paths: None, + src: None, target_version: None, flake8_annotations: None, flake8_quotes: None, @@ -346,7 +346,7 @@ other-attribute = 1 vec![CheckCodePrefix::F401] ),])), dummy_variable_rgx: None, - src_paths: None, + src: None, target_version: None, flake8_annotations: None, flake8_quotes: Some(flake8_quotes::settings::Options { diff --git a/src/settings/user.rs b/src/settings/user.rs index 62ca411baa9af..b2352cbfe9cd9 100644 --- a/src/settings/user.rs +++ b/src/settings/user.rs @@ -45,7 +45,7 @@ pub struct UserConfiguration { pub line_length: usize, pub per_file_ignores: Vec<(Exclusion, Vec)>, pub select: Vec, - pub src_paths: Vec, + pub src: Vec, pub target_version: PythonVersion, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, @@ -91,7 +91,7 @@ impl UserConfiguration { }) .collect(), select: configuration.select, - src_paths: configuration.src_paths, + src: configuration.src, target_version: configuration.target_version, flake8_annotations: configuration.flake8_annotations, flake8_quotes: configuration.flake8_quotes, diff --git a/test/foo.py b/test/foo.py deleted file mode 100644 index f6f6669ec1dc8..0000000000000 --- a/test/foo.py +++ /dev/null @@ -1,41 +0,0 @@ -import local -from local_from import ( - import1, - import2, - import3, - import4, - import5, - import6, - import7, - import8, - import9, -) -import __future__.bar -from __future__ import annotations -# Comment 1 -import os -import baz.bar -import baz.foo - -# Comment 2 - -# Comment 3 (line 1) -# Comment 3 (line 2) -from bop import bar # Comment 4 -from bop import foo # Comment 5 -from boo import ( - import1, - import2, - import3, - import4, - import5, - import6, - import7, - import8, - import9, -) - -x = 1 - -import collections -import typing diff --git a/test/local.py b/test/local.py deleted file mode 100644 index dfd0cd3227d48..0000000000000 --- a/test/local.py +++ /dev/null @@ -1,16 +0,0 @@ -# [isort] -# import __future__.bar as foo -# import __future__.baz -# -# import bar -# import wop -# from wop import wop -# -# import foo -# import foo as bar -# import foo as baz -# import foo.f1 as g1 -# import foo.f1 as g2 -# -# from . import bar -# from .boop import bar diff --git a/test/local_from.py b/test/local_from.py deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/test/pyproject.toml b/test/pyproject.toml deleted file mode 100644 index fb3315de0281b..0000000000000 --- a/test/pyproject.toml +++ /dev/null @@ -1,6 +0,0 @@ -[tool.ruff.isort] -src-paths = ['test'] - -[tool.isort] -line_length = 88 -profile = 'black' diff --git a/test_cases/after/comment.py b/test_cases/after/comment.py deleted file mode 100644 index d103bde8da272..0000000000000 --- a/test_cases/after/comment.py +++ /dev/null @@ -1,3 +0,0 @@ -import abc -import os # Line 2 -import sys diff --git a/test_cases/after/foo.py b/test_cases/after/foo.py deleted file mode 100644 index 6a444c8f18e4d..0000000000000 --- a/test_cases/after/foo.py +++ /dev/null @@ -1,6 +0,0 @@ -from __future__ import annotations - -import os -import sys - -import bar diff --git a/test_cases/before/bar.py b/test_cases/before/bar.py deleted file mode 100644 index e8adff0d1372f..0000000000000 --- a/test_cases/before/bar.py +++ /dev/null @@ -1,5 +0,0 @@ -try: - import sys - import os -except: - import os diff --git a/test_cases/before/comment.py b/test_cases/before/comment.py deleted file mode 100644 index 59a33c913ce62..0000000000000 --- a/test_cases/before/comment.py +++ /dev/null @@ -1,3 +0,0 @@ -import sys -import os # Line 2 -import abc diff --git a/test_cases/before/prefix.py b/test_cases/before/prefix.py deleted file mode 100644 index 364a6d8dc8ae7..0000000000000 --- a/test_cases/before/prefix.py +++ /dev/null @@ -1,2 +0,0 @@ -x=1;import sys -import os diff --git a/test_cases/before/suffix.py b/test_cases/before/suffix.py deleted file mode 100644 index 9866710ba90a4..0000000000000 --- a/test_cases/before/suffix.py +++ /dev/null @@ -1,2 +0,0 @@ -import sys -import os; x = 1