diff --git a/crates/ruff/resources/test/fixtures/isort/required_imports/docstring.pyi b/crates/ruff/resources/test/fixtures/isort/required_imports/docstring.pyi new file mode 100644 index 0000000000..bf333153bc --- /dev/null +++ b/crates/ruff/resources/test/fixtures/isort/required_imports/docstring.pyi @@ -0,0 +1,3 @@ +"""Hello, world!""" + +x = 1 diff --git a/crates/ruff/src/checkers/imports.rs b/crates/ruff/src/checkers/imports.rs index 1e2a6dc6dc..ca1ffc6813 100644 --- a/crates/ruff/src/checkers/imports.rs +++ b/crates/ruff/src/checkers/imports.rs @@ -9,6 +9,7 @@ use ruff_python_ast::helpers::to_module_path; use ruff_python_ast::imports::{ImportMap, ModuleImport}; use ruff_python_ast::source_code::{Indexer, Locator, Stylist}; use ruff_python_ast::visitor::Visitor; +use ruff_python_stdlib::path::is_python_stub_file; use crate::directives::IsortDirectives; use crate::registry::Rule; @@ -88,9 +89,11 @@ pub fn check_imports( path: &Path, package: Option<&Path>, ) -> (Vec, Option) { + let is_stub = is_python_stub_file(path); + // Extract all imports from the AST. let tracker = { - let mut tracker = ImportTracker::new(locator, directives, path); + let mut tracker = ImportTracker::new(locator, directives, is_stub); tracker.visit_body(python_ast); tracker }; @@ -111,7 +114,7 @@ pub fn check_imports( } if settings.rules.enabled(Rule::MissingRequiredImport) { diagnostics.extend(isort::rules::add_required_imports( - &blocks, python_ast, locator, stylist, settings, autofix, + &blocks, python_ast, locator, stylist, settings, autofix, is_stub, )); } diff --git a/crates/ruff/src/rules/isort/mod.rs b/crates/ruff/src/rules/isort/mod.rs index 211238bf46..b5bfb65433 100644 --- a/crates/ruff/src/rules/isort/mod.rs +++ b/crates/ruff/src/rules/isort/mod.rs @@ -714,6 +714,7 @@ mod tests { } #[test_case(Path::new("docstring.py"))] + #[test_case(Path::new("docstring.pyi"))] #[test_case(Path::new("docstring_only.py"))] #[test_case(Path::new("multiline_docstring.py"))] #[test_case(Path::new("empty.py"))] @@ -737,6 +738,7 @@ mod tests { } #[test_case(Path::new("docstring.py"))] + #[test_case(Path::new("docstring.pyi"))] #[test_case(Path::new("docstring_only.py"))] #[test_case(Path::new("empty.py"))] fn required_imports(path: &Path) -> Result<()> { @@ -760,6 +762,7 @@ mod tests { } #[test_case(Path::new("docstring.py"))] + #[test_case(Path::new("docstring.pyi"))] #[test_case(Path::new("docstring_only.py"))] #[test_case(Path::new("empty.py"))] fn combined_required_imports(path: &Path) -> Result<()> { @@ -782,6 +785,7 @@ mod tests { } #[test_case(Path::new("docstring.py"))] + #[test_case(Path::new("docstring.pyi"))] #[test_case(Path::new("docstring_only.py"))] #[test_case(Path::new("empty.py"))] fn straight_required_import(path: &Path) -> Result<()> { diff --git a/crates/ruff/src/rules/isort/rules/add_required_imports.rs b/crates/ruff/src/rules/isort/rules/add_required_imports.rs index b3c9975e79..a02d5b509d 100644 --- a/crates/ruff/src/rules/isort/rules/add_required_imports.rs +++ b/crates/ruff/src/rules/isort/rules/add_required_imports.rs @@ -5,7 +5,7 @@ use rustpython_parser::ast::{Location, StmtKind, Suite}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::is_docstring_stmt; -use ruff_python_ast::imports::{Alias, AnyImport, Import, ImportFrom}; +use ruff_python_ast::imports::{Alias, AnyImport, FutureImport, Import, ImportFrom}; use ruff_python_ast::source_code::{Locator, Stylist}; use ruff_python_ast::types::Range; @@ -84,6 +84,7 @@ fn contains(block: &Block, required_import: &AnyImport) -> bool { }) } +#[allow(clippy::too_many_arguments)] fn add_required_import( required_import: &AnyImport, blocks: &[&Block], @@ -92,6 +93,7 @@ fn add_required_import( stylist: &Stylist, settings: &Settings, autofix: flags::Autofix, + is_stub: bool, ) -> Option { // If the import is already present in a top-level block, don't add it. if blocks @@ -107,6 +109,11 @@ fn add_required_import( return None; } + // We don't need to add `__future__` imports to stubs. + if is_stub && required_import.is_future_import() { + return None; + } + // Always insert the diagnostic at top-of-file. let mut diagnostic = Diagnostic::new( MissingRequiredImport(required_import.to_string()), @@ -126,6 +133,7 @@ pub fn add_required_imports( stylist: &Stylist, settings: &Settings, autofix: flags::Autofix, + is_stub: bool, ) -> Vec { settings .isort @@ -167,6 +175,7 @@ pub fn add_required_imports( stylist, settings, autofix, + is_stub, ) }) .collect(), @@ -186,6 +195,7 @@ pub fn add_required_imports( stylist, settings, autofix, + is_stub, ) }) .collect(), diff --git a/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__combined_required_imports_docstring.pyi.snap b/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__combined_required_imports_docstring.pyi.snap new file mode 100644 index 0000000000..94aa1559b8 --- /dev/null +++ b/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__combined_required_imports_docstring.pyi.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/isort/mod.rs +--- + diff --git a/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__required_import_docstring.pyi.snap b/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__required_import_docstring.pyi.snap new file mode 100644 index 0000000000..94aa1559b8 --- /dev/null +++ b/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__required_import_docstring.pyi.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/isort/mod.rs +--- + diff --git a/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__required_imports_docstring.pyi.snap b/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__required_imports_docstring.pyi.snap new file mode 100644 index 0000000000..94aa1559b8 --- /dev/null +++ b/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__required_imports_docstring.pyi.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/isort/mod.rs +--- + diff --git a/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__straight_required_import_docstring.pyi.snap b/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__straight_required_import_docstring.pyi.snap new file mode 100644 index 0000000000..6225322977 --- /dev/null +++ b/crates/ruff/src/rules/isort/snapshots/ruff__rules__isort__tests__straight_required_import_docstring.pyi.snap @@ -0,0 +1,19 @@ +--- +source: crates/ruff/src/rules/isort/mod.rs +--- +docstring.pyi:1:1: I002 [*] Missing required import: `import os` + | +1 | """Hello, world!""" + | I002 +2 | +3 | x = 1 + | + = help: Insert required import: `import os` + +ℹ Suggested fix +1 1 | """Hello, world!""" + 2 |+import os +2 3 | +3 4 | x = 1 + + diff --git a/crates/ruff/src/rules/isort/track.rs b/crates/ruff/src/rules/isort/track.rs index f0aeaaa2b7..06405bffdb 100644 --- a/crates/ruff/src/rules/isort/track.rs +++ b/crates/ruff/src/rules/isort/track.rs @@ -1,5 +1,3 @@ -use std::path::Path; - use rustpython_parser::ast::{ Alias, Arg, Arguments, Boolop, Cmpop, Comprehension, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, Keyword, MatchCase, Operator, Pattern, Stmt, StmtKind, @@ -8,11 +6,9 @@ use rustpython_parser::ast::{ use ruff_python_ast::source_code::Locator; use ruff_python_ast::visitor::Visitor; -use ruff_python_stdlib::path::is_python_stub_file; use crate::directives::IsortDirectives; - -use super::helpers; +use crate::rules::isort::helpers; #[derive(Debug, Copy, Clone)] pub enum Trailer { @@ -38,11 +34,11 @@ pub struct ImportTracker<'a> { } impl<'a> ImportTracker<'a> { - pub fn new(locator: &'a Locator<'a>, directives: &'a IsortDirectives, path: &'a Path) -> Self { + pub fn new(locator: &'a Locator<'a>, directives: &'a IsortDirectives, is_stub: bool) -> Self { Self { locator, directives, - is_stub: is_python_stub_file(path), + is_stub, blocks: vec![Block::default()], split_index: 0, nested: false, diff --git a/crates/ruff_python_ast/src/imports.rs b/crates/ruff_python_ast/src/imports.rs index b6392ae19f..0cc4ddea68 100644 --- a/crates/ruff_python_ast/src/imports.rs +++ b/crates/ruff_python_ast/src/imports.rs @@ -75,6 +75,32 @@ impl std::fmt::Display for ImportFrom<'_> { } } +pub trait FutureImport { + /// Returns `true` if this import is from the `__future__` module. + fn is_future_import(&self) -> bool; +} + +impl FutureImport for Import<'_> { + fn is_future_import(&self) -> bool { + self.name.name == "__future__" + } +} + +impl FutureImport for ImportFrom<'_> { + fn is_future_import(&self) -> bool { + self.module == Some("__future__") + } +} + +impl FutureImport for AnyImport<'_> { + fn is_future_import(&self) -> bool { + match self { + AnyImport::Import(import) => import.is_future_import(), + AnyImport::ImportFrom(import_from) => import_from.is_future_import(), + } + } +} + /// A representation of a module reference in an import statement. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ModuleImport {