diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/__init__.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/__init__.py similarity index 77% rename from crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/__init__.py rename to crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/__init__.py index 3d6f86c663..874b9bde48 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/__init__.py +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/__init__.py @@ -1,4 +1,4 @@ -"""__init__.py with __all__ +"""__init__.py with nonempty __all__ Unused stdlib and third party imports are unsafe removals @@ -33,10 +33,10 @@ from . import aliased as aliased # Ok: is redundant alias from . import exported # Ok: is exported in __all__ -# from . import unused # F401: add to __all__ +from . import unused # F401: add to __all__ -# from . import renamed as bees # F401: add to __all__ +from . import renamed as bees # F401: add to __all__ __all__ = ["argparse", "exported"] diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/aliased.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/aliased.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/aliased.py rename to crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/aliased.py diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/exported.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/exported.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/exported.py rename to crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/exported.py diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/renamed.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/renamed.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/renamed.py rename to crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/renamed.py diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/unused.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/unused.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/unused.py rename to crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/unused.py diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/used.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/used.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all/used.py rename to crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25__all_nonempty/used.py diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/__init__.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/__init__.py new file mode 100644 index 0000000000..1ef75a30d6 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/__init__.py @@ -0,0 +1,11 @@ +"""__init__.py with empty __all__ +""" + + +from . import unused # F401: add to __all__ + + +from . import renamed as bees # F401: add to __all__ + + +__all__ = [] diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/renamed.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/renamed.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/renamed.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/unused.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/unused.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/unused.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/__init__.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/__init__.py new file mode 100644 index 0000000000..dc913f390f --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/__init__.py @@ -0,0 +1,11 @@ +"""__init__.py with mis-typed __all__ +""" + + +from . import unused # F401: recommend add to all w/o fix + + +from . import renamed as bees # F401: recommend add to all w/o fix + + +__all__ = None diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/renamed.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/renamed.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/renamed.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/unused.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/unused.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/unused.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/__init__.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/__init__.py new file mode 100644 index 0000000000..28dbb9f022 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/__init__.py @@ -0,0 +1,8 @@ +"""__init__.py with multiple imports added to all in one edit +""" + + +from . import unused, renamed as bees # F401: add to __all__ + + +__all__ = []; diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/renamed.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/renamed.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/renamed.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/unused.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/unused.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/unused.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/__init__.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/__init__.py new file mode 100644 index 0000000000..4802749b47 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/__init__.py @@ -0,0 +1,16 @@ +"""__init__.py with __all__ populated by conditional plus-eq + +multiple __all__ so cannot offer a fix to add to them +""" + +import sys + +from . import unused, exported, renamed as bees + +if sys.version_info > (3, 9): + from . import also_exported + +__all__ = ["exported"] + +if sys.version_info >= (3, 9): + __all__ += ["also_exported"] diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/also_exported.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/also_exported.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/also_exported.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/exported.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/exported.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/exported.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/renamed.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/renamed.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/renamed.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/unused.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/unused.py new file mode 100644 index 0000000000..7391604add --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/unused.py @@ -0,0 +1 @@ +# empty module imported by __init__.py for test fixture diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs index 3ce660b70c..3d45f1ea01 100644 --- a/crates/ruff_linter/src/fix/edits.rs +++ b/crates/ruff_linter/src/fix/edits.rs @@ -1,10 +1,12 @@ //! Interface for generating fix edits from higher-level actions (e.g., "remove an argument"). +use std::borrow::Cow; + use anyhow::{Context, Result}; use ruff_diagnostics::Edit; use ruff_python_ast::parenthesize::parenthesized_range; -use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Stmt}; +use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Expr, ExprList, Stmt}; use ruff_python_ast::{AnyNodeRef, ArgOrKeyword}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; @@ -124,7 +126,7 @@ pub(crate) fn remove_unused_imports<'a>( /// Edits to make the specified imports explicit, e.g. change `import x` to `import x as x`. pub(crate) fn make_redundant_alias<'a>( - member_names: impl Iterator, + member_names: impl Iterator>, stmt: &Stmt, ) -> Vec { let aliases = match stmt { @@ -144,6 +146,53 @@ pub(crate) fn make_redundant_alias<'a>( .collect() } +/// Fix to add the specified imports to the `__all__` export list. +pub(crate) fn add_to_dunder_all<'a>( + names: impl Iterator, + expr: &Expr, + stylist: &Stylist, +) -> Vec { + let (insertion_point, export_prefix_length) = match expr { + Expr::List(ExprList { elts, range, .. }) => ( + elts.last() + .map_or(range.end() - "]".text_len(), Ranged::end), + elts.len(), + ), + Expr::Tuple(tup) if tup.parenthesized => ( + tup.elts + .last() + .map_or(tup.end() - ")".text_len(), Ranged::end), + tup.elts.len(), + ), + Expr::Tuple(tup) if !tup.parenthesized => ( + tup.elts + .last() + .expect("unparenthesized empty tuple is not possible") + .range() + .end(), + tup.elts.len(), + ), + _ => { + // we don't know how to insert into this expression + return vec![]; + } + }; + let quote = stylist.quote(); + let mut edits: Vec<_> = names + .enumerate() + .map(|(offset, name)| match export_prefix_length + offset { + 0 => Edit::insertion(format!("{quote}{name}{quote}"), insertion_point), + _ => Edit::insertion(format!(", {quote}{name}{quote}"), insertion_point), + }) + .collect(); + if let Expr::Tuple(tup) = expr { + if tup.parenthesized && export_prefix_length + edits.len() == 1 { + edits.push(Edit::insertion(",".to_string(), insertion_point)); + } + } + edits +} + #[derive(Debug, Copy, Clone)] pub(crate) enum Parentheses { /// Remove parentheses, if the removed argument is the only argument left. @@ -477,14 +526,20 @@ fn all_lines_fit( #[cfg(test)] mod tests { - use anyhow::Result; + use anyhow::{anyhow, Result}; + use std::borrow::Cow; + use test_case::test_case; - use ruff_diagnostics::Edit; - use ruff_python_parser::parse_suite; + use ruff_diagnostics::{Diagnostic, Edit, Fix}; + use ruff_python_codegen::Stylist; + use ruff_python_parser::{lexer, parse_expression, parse_suite, Mode}; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange, TextSize}; - use crate::fix::edits::{make_redundant_alias, next_stmt_break, trailing_semicolon}; + use crate::fix::apply_fixes; + use crate::fix::edits::{ + add_to_dunder_all, make_redundant_alias, next_stmt_break, trailing_semicolon, + }; #[test] fn find_semicolon() -> Result<()> { @@ -562,7 +617,7 @@ x = 1 \ let program = parse_suite(contents).unwrap(); let stmt = program.first().unwrap(); assert_eq!( - make_redundant_alias(["x"].into_iter(), stmt), + make_redundant_alias(["x"].into_iter().map(Cow::from), stmt), vec![Edit::range_replacement( String::from("x as x"), TextRange::new(TextSize::new(7), TextSize::new(8)), @@ -570,7 +625,7 @@ x = 1 \ "make just one item redundant" ); assert_eq!( - make_redundant_alias(vec!["x", "y"].into_iter(), stmt), + make_redundant_alias(vec!["x", "y"].into_iter().map(Cow::from), stmt), vec![Edit::range_replacement( String::from("x as x"), TextRange::new(TextSize::new(7), TextSize::new(8)), @@ -578,7 +633,7 @@ x = 1 \ "the second item is already a redundant alias" ); assert_eq!( - make_redundant_alias(vec!["x", "z"].into_iter(), stmt), + make_redundant_alias(vec!["x", "z"].into_iter().map(Cow::from), stmt), vec![Edit::range_replacement( String::from("x as x"), TextRange::new(TextSize::new(7), TextSize::new(8)), @@ -586,4 +641,47 @@ x = 1 \ "the third item is already aliased to something else" ); } + + #[test_case("()", &["x", "y"], r#"("x", "y")"# ; "2 into empty tuple")] + #[test_case("()", &["x"], r#"("x",)"# ; "1 into empty tuple adding a trailing comma")] + #[test_case("[]", &["x", "y"], r#"["x", "y"]"# ; "2 into empty list")] + #[test_case("[]", &["x"], r#"["x"]"# ; "1 into empty list")] + #[test_case(r#""a", "b""#, &["x", "y"], r#""a", "b", "x", "y""# ; "2 into unparenthesized tuple")] + #[test_case(r#""a", "b""#, &["x"], r#""a", "b", "x""# ; "1 into unparenthesized tuple")] + #[test_case(r#""a", "b","#, &["x", "y"], r#""a", "b", "x", "y","# ; "2 into unparenthesized tuple w/trailing comma")] + #[test_case(r#""a", "b","#, &["x"], r#""a", "b", "x","# ; "1 into unparenthesized tuple w/trailing comma")] + #[test_case(r#"("a", "b")"#, &["x", "y"], r#"("a", "b", "x", "y")"# ; "2 into nonempty tuple")] + #[test_case(r#"("a", "b")"#, &["x"], r#"("a", "b", "x")"# ; "1 into nonempty tuple")] + #[test_case(r#"("a", "b",)"#, &["x", "y"], r#"("a", "b", "x", "y",)"# ; "2 into nonempty tuple w/trailing comma")] + #[test_case(r#"("a", "b",)"#, &["x"], r#"("a", "b", "x",)"# ; "1 into nonempty tuple w/trailing comma")] + #[test_case(r#"["a", "b",]"#, &["x", "y"], r#"["a", "b", "x", "y",]"# ; "2 into nonempty list w/trailing comma")] + #[test_case(r#"["a", "b",]"#, &["x"], r#"["a", "b", "x",]"# ; "1 into nonempty list w/trailing comma")] + #[test_case(r#"["a", "b"]"#, &["x", "y"], r#"["a", "b", "x", "y"]"# ; "2 into nonempty list")] + #[test_case(r#"["a", "b"]"#, &["x"], r#"["a", "b", "x"]"# ; "1 into nonempty list")] + fn add_to_dunder_all_test(raw: &str, names: &[&str], expect: &str) -> Result<()> { + let locator = Locator::new(raw); + let edits = { + let expr = parse_expression(raw)?; + let stylist = Stylist::from_tokens( + &lexer::lex(raw, Mode::Expression).collect::>(), + &locator, + ); + // SUT + add_to_dunder_all(names.iter().copied(), &expr, &stylist) + }; + let diag = { + use crate::rules::pycodestyle::rules::MissingNewlineAtEndOfFile; + let mut iter = edits.into_iter(); + Diagnostic::new( + MissingNewlineAtEndOfFile, // The choice of rule here is arbitrary. + TextRange::default(), + ) + .with_fix(Fix::safe_edits( + iter.next().ok_or(anyhow!("expected edits nonempty"))?, + iter, + )) + }; + assert_eq!(apply_fixes([diag].iter(), &locator).code, expect); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index b4f3618098..4d31234106 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -207,7 +207,11 @@ mod tests { #[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))] #[test_case(Rule::UnusedImport, Path::new("__init__.py"))] #[test_case(Rule::UnusedImport, Path::new("F401_24/__init__.py"))] - #[test_case(Rule::UnusedImport, Path::new("F401_25__all/__init__.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_25__all_nonempty/__init__.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_26__all_empty/__init__.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_27__all_mistyped/__init__.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_28__all_multiple/__init__.py"))] + #[test_case(Rule::UnusedImport, Path::new("F401_29__all_conditional/__init__.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs index 963bc8dd2e..6fe44cf88a 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs @@ -6,8 +6,11 @@ use rustc_hash::FxHashMap; use ruff_diagnostics::{Applicability, Diagnostic, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; use ruff_python_ast::{Stmt, StmtImportFrom}; -use ruff_python_semantic::{AnyImport, Exceptions, Imported, NodeId, Scope}; +use ruff_python_semantic::{ + AnyImport, BindingKind, Exceptions, Imported, NodeId, Scope, SemanticModel, +}; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -18,7 +21,10 @@ use crate::rules::{isort, isort::ImportSection, isort::ImportType}; #[derive(Debug, Copy, Clone, Eq, PartialEq)] enum UnusedImportContext { ExceptHandler, - Init { first_party: bool }, + Init { + first_party: bool, + dunder_all_count: usize, + }, } /// ## What it does @@ -79,7 +85,10 @@ enum UnusedImportContext { /// - [Typing documentation: interface conventions](https://typing.readthedocs.io/en/latest/source/libraries.html#library-interface-public-and-private-symbols) #[violation] pub struct UnusedImport { + /// Qualified name of the import name: String, + /// Name of the import binding + binding: String, context: Option, multiple: bool, } @@ -106,16 +115,31 @@ impl Violation for UnusedImport { } fn fix_title(&self) -> Option { - let UnusedImport { name, multiple, .. } = self; - let resolution = match self.context { - Some(UnusedImportContext::Init { first_party: true }) => "Use a redundant alias", - _ => "Remove unused import", - }; - Some(if *multiple { - resolution.to_string() - } else { - format!("{resolution}: `{name}`") - }) + let UnusedImport { + name, + binding, + multiple, + .. + } = self; + match self.context { + Some(UnusedImportContext::Init { + first_party: true, + dunder_all_count: 1, + }) => Some(format!("Add unused import `{binding}` to __all__")), + + Some(UnusedImportContext::Init { + first_party: true, + dunder_all_count: 0, + }) => Some(format!( + "Use an explicit re-export: `{binding} as {binding}`" + )), + + _ => Some(if *multiple { + "Remove unused import".to_string() + } else { + format!("Remove unused import: `{name}`") + }), + } } } @@ -138,9 +162,32 @@ fn is_first_party(qualified_name: &str, level: u32, checker: &Checker) -> bool { } } +/// Find the `Expr` for top level `__all__` bindings. +fn find_dunder_all_exprs<'a>(semantic: &'a SemanticModel) -> Vec<&'a ast::Expr> { + semantic + .global_scope() + .get_all("__all__") + .filter_map(|binding_id| { + let binding = semantic.binding(binding_id); + let stmt = match binding.kind { + BindingKind::Export(_) => binding.statement(semantic), + _ => None, + }?; + match stmt { + Stmt::Assign(ast::StmtAssign { value, .. }) => Some(&**value), + Stmt::AnnAssign(ast::StmtAnnAssign { value, .. }) => value.as_deref(), + Stmt::AugAssign(ast::StmtAugAssign { value, .. }) => Some(&**value), + _ => None, + } + }) + .collect() +} + /// For some unused binding in an import statement... /// -/// __init__.py ∧ 1stpty → safe, convert to redundant-alias +/// __init__.py ∧ 1stpty → safe, if one __all__, add to __all__ +/// safe, if no __all__, convert to redundant-alias +/// n/a, if multiple __all__, offer no fix /// __init__.py ∧ stdlib → unsafe, remove /// __init__.py ∧ 3rdpty → unsafe, remove /// @@ -173,6 +220,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut }; let import = ImportBinding { + name: binding.name(checker.locator()), import, range: binding.range(), parent_range: binding.parent_range(checker.semantic()), @@ -197,6 +245,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut let in_init = checker.path().ends_with("__init__.py"); let fix_init = checker.settings.preview.is_enabled(); + let dunder_all_exprs = find_dunder_all_exprs(checker.semantic()); // Generate a diagnostic for every import, but share fixes across all imports within the same // statement (excluding those that are ignored). @@ -225,6 +274,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut level, checker, ), + dunder_all_count: dunder_all_exprs.len(), }) } else { None @@ -234,7 +284,10 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut .partition(|(_, context)| { matches!( context, - Some(UnusedImportContext::Init { first_party: true }) + Some(UnusedImportContext::Init { + first_party: true, + .. + }) ) }); @@ -251,7 +304,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut fix_by_reexporting( checker, import_statement, - to_reexport.iter().map(|(binding, _)| binding), + &to_reexport.iter().map(|(b, _)| b).collect::>(), + &dunder_all_exprs, ) .ok(), ) @@ -266,6 +320,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut let mut diagnostic = Diagnostic::new( UnusedImport { name: binding.import.qualified_name().to_string(), + binding: binding.name.to_string(), context, multiple, }, @@ -285,21 +340,17 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut // Separately, generate a diagnostic for every _ignored_ import, to ensure that the // suppression comments aren't marked as unused. - for ImportBinding { - import, - range, - parent_range, - } in ignored.into_values().flatten() - { + for binding in ignored.into_values().flatten() { let mut diagnostic = Diagnostic::new( UnusedImport { - name: import.qualified_name().to_string(), + name: binding.import.qualified_name().to_string(), + binding: binding.name.to_string(), context: None, multiple: false, }, - range, + binding.range, ); - if let Some(range) = parent_range { + if let Some(range) = binding.parent_range { diagnostic.set_parent(range.start()); } diagnostics.push(diagnostic); @@ -309,6 +360,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut /// An unused import with its surrounding context. #[derive(Debug)] struct ImportBinding<'a> { + /// Name of the binding, which for renamed imports will differ from the qualified name. + name: &'a str, /// The qualified name of the import (e.g., `typing.List` for `from typing import List`). import: AnyImport<'a, 'a>, /// The trimmed range of the import (e.g., `List` in `from typing import List`). @@ -364,23 +417,31 @@ fn fix_by_removing_imports<'a>( ) } -/// Generate a [`Fix`] to make bindings in a statement explicit, by changing from `import a` to -/// `import a as a`. -fn fix_by_reexporting<'a>( +/// Generate a [`Fix`] to make bindings in a statement explicit, either by adding them to `__all__` +/// or changing them from `import a` to `import a as a`. +fn fix_by_reexporting( checker: &Checker, node_id: NodeId, - imports: impl Iterator>, + imports: &[&ImportBinding], + dunder_all_exprs: &[&ast::Expr], ) -> Result { let statement = checker.semantic().statement(node_id); - - let member_names = imports - .map(|binding| binding.import.member_name()) - .collect::>(); - if member_names.is_empty() { + if imports.is_empty() { bail!("Expected import bindings"); } - let edits = fix::edits::make_redundant_alias(member_names.iter().map(AsRef::as_ref), statement); + let edits = match dunder_all_exprs { + [] => fix::edits::make_redundant_alias( + imports.iter().map(|b| b.import.member_name()), + statement, + ), + [dunder_all] => fix::edits::add_to_dunder_all( + imports.iter().map(|b| b.name), + dunder_all, + checker.stylist(), + ), + _ => bail!("Cannot offer a fix when there are multiple __all__ definitions"), + }; // Only emit a fix if there are edits let mut tail = edits.into_iter(); diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_24____init__.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_24____init__.py.snap index 2a5f807104..46850ca661 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_24____init__.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_24____init__.py.snap @@ -22,7 +22,7 @@ __init__.py:33:15: F401 [*] `.unused` imported but unused; consider removing, ad 33 | from . import unused # F401: change to redundant alias | ^^^^^^ F401 | - = help: Use a redundant alias: `.unused` + = help: Use an explicit re-export: `unused as unused` ℹ Safe fix 30 30 | from . import aliased as aliased # Ok: is redundant alias @@ -39,4 +39,4 @@ __init__.py:36:26: F401 `.renamed` imported but unused; consider removing, addin 36 | from . import renamed as bees # F401: no fix | ^^^^ F401 | - = help: Use a redundant alias: `.renamed` + = help: Use an explicit re-export: `bees as bees` diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_25__all____init__.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_25__all____init__.py.snap deleted file mode 100644 index 43c550ba55..0000000000 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_25__all____init__.py.snap +++ /dev/null @@ -1,18 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/pyflakes/mod.rs ---- -__init__.py:19:8: F401 [*] `sys` imported but unused; consider removing, adding to `__all__`, or using a redundant alias - | -19 | import sys # F401: remove unused - | ^^^ F401 - | - = help: Remove unused import: `sys` - -ℹ Unsafe fix -16 16 | import argparse # Ok: is exported in __all__ -17 17 | -18 18 | -19 |-import sys # F401: remove unused -20 19 | -21 20 | -22 21 | # first-party diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_25__all_nonempty____init__.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_25__all_nonempty____init__.py.snap new file mode 100644 index 0000000000..9d04194da7 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_25__all_nonempty____init__.py.snap @@ -0,0 +1,46 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +__init__.py:19:8: F401 [*] `sys` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +19 | import sys # F401: remove unused + | ^^^ F401 + | + = help: Remove unused import: `sys` + +ℹ Unsafe fix +16 16 | import argparse # Ok: is exported in __all__ +17 17 | +18 18 | +19 |-import sys # F401: remove unused +20 19 | +21 20 | +22 21 | # first-party + +__init__.py:36:15: F401 [*] `.unused` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +36 | from . import unused # F401: add to __all__ + | ^^^^^^ F401 + | + = help: Add unused import `unused` to __all__ + +ℹ Safe fix +39 39 | from . import renamed as bees # F401: add to __all__ +40 40 | +41 41 | +42 |-__all__ = ["argparse", "exported"] + 42 |+__all__ = ["argparse", "exported", "unused"] + +__init__.py:39:26: F401 [*] `.renamed` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +39 | from . import renamed as bees # F401: add to __all__ + | ^^^^ F401 + | + = help: Add unused import `bees` to __all__ + +ℹ Safe fix +39 39 | from . import renamed as bees # F401: add to __all__ +40 40 | +41 41 | +42 |-__all__ = ["argparse", "exported"] + 42 |+__all__ = ["argparse", "exported", "bees"] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_26__all_empty____init__.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_26__all_empty____init__.py.snap new file mode 100644 index 0000000000..6c393f0fbf --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_26__all_empty____init__.py.snap @@ -0,0 +1,30 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +__init__.py:5:15: F401 [*] `.unused` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +5 | from . import unused # F401: add to __all__ + | ^^^^^^ F401 + | + = help: Add unused import `unused` to __all__ + +ℹ Safe fix +8 8 | from . import renamed as bees # F401: add to __all__ +9 9 | +10 10 | +11 |-__all__ = [] + 11 |+__all__ = ["unused"] + +__init__.py:8:26: F401 [*] `.renamed` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +8 | from . import renamed as bees # F401: add to __all__ + | ^^^^ F401 + | + = help: Add unused import `bees` to __all__ + +ℹ Safe fix +8 8 | from . import renamed as bees # F401: add to __all__ +9 9 | +10 10 | +11 |-__all__ = [] + 11 |+__all__ = ["bees"] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_27__all_mistyped____init__.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_27__all_mistyped____init__.py.snap new file mode 100644 index 0000000000..c665795df9 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_27__all_mistyped____init__.py.snap @@ -0,0 +1,16 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +__init__.py:5:15: F401 `.unused` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +5 | from . import unused # F401: recommend add to all w/o fix + | ^^^^^^ F401 + | + = help: Add unused import `unused` to __all__ + +__init__.py:8:26: F401 `.renamed` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +8 | from . import renamed as bees # F401: recommend add to all w/o fix + | ^^^^ F401 + | + = help: Add unused import `bees` to __all__ diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_28__all_multiple____init__.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_28__all_multiple____init__.py.snap new file mode 100644 index 0000000000..a77b95641d --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_28__all_multiple____init__.py.snap @@ -0,0 +1,30 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +__init__.py:5:15: F401 [*] `.unused` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +5 | from . import unused, renamed as bees # F401: add to __all__ + | ^^^^^^ F401 + | + = help: Add unused import `unused` to __all__ + +ℹ Safe fix +5 5 | from . import unused, renamed as bees # F401: add to __all__ +6 6 | +7 7 | +8 |-__all__ = []; + 8 |+__all__ = ["bees", "unused"]; + +__init__.py:5:34: F401 [*] `.renamed` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | +5 | from . import unused, renamed as bees # F401: add to __all__ + | ^^^^ F401 + | + = help: Add unused import `bees` to __all__ + +ℹ Safe fix +5 5 | from . import unused, renamed as bees # F401: add to __all__ +6 6 | +7 7 | +8 |-__all__ = []; + 8 |+__all__ = ["bees", "unused"]; diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_29__all_conditional____init__.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_29__all_conditional____init__.py.snap new file mode 100644 index 0000000000..0100397db3 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__preview__F401_F401_29__all_conditional____init__.py.snap @@ -0,0 +1,24 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +__init__.py:8:15: F401 `.unused` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | + 6 | import sys + 7 | + 8 | from . import unused, exported, renamed as bees + | ^^^^^^ F401 + 9 | +10 | if sys.version_info > (3, 9): + | + = help: Remove unused import + +__init__.py:8:44: F401 `.renamed` imported but unused; consider removing, adding to `__all__`, or using a redundant alias + | + 6 | import sys + 7 | + 8 | from . import unused, exported, renamed as bees + | ^^^^ F401 + 9 | +10 | if sys.version_info > (3, 9): + | + = help: Remove unused import