mirror of https://github.com/astral-sh/ruff
F401 - Recommend adding unused import bindings to `__all__` (#11314)
Followup on #11168 and resolve #10391 # User facing changes * F401 now recommends a fix to add unused import bindings to to `__all__` if a single `__all__` list or tuple is found in `__init__.py`. * If there are no `__all__` found in the file, fall back to recommending redundant-aliases. * If there are multiple `__all__` or only one but of the wrong type (non list or tuple) then diagnostics are generated without fixes. * `fix_title` is updated to reflect what the fix/recommendation is. Subtlety: For a renamed import such as `import foo as bees`, we can generate a fix to add `bees` to `__all__` but cannot generate a fix to produce a redundant import (because that would break uses of the binding `bees`). # Implementation changes * Add `name` field to `ImportBinding` to contain the name of the _binding_ we want to add to `__all__` (important for the `import foo as bees` case). It previously only contained the `AnyImport` which can give us information about the import but not the binding. * Add `binding` field to `UnusedImport` to contain the same. (Naming note: the field `name` field already existed on `UnusedImport` and contains the qualified name of the imported symbol/module) * Change `fix_by_reexporting` to branch on the size of `dunder_all: Vec<&Expr>` * For length 0 call the edit-producing function `make_redundant_alias`. * For length 1 call edit-producing function `add_to_dunder_all`. * Otherwise, produce no fix. * Implement the edit-producing function `add_to_dunder_all` and add unit tests. * Implement several fixture tests: empty `__all__ = []`, nonempty `__all__ = ["foo"]`, mis-typed `__all__ = None`, plus-eq `__all__ += ["foo"]` * `UnusedImportContext::Init` variant now has two fields: whether the fix is in `__init__.py` and how many `__all__` were found. # Other changes * Remove a spurious pattern match and instead use field lookups b/c the addition of a field would have required changing the unrelated pattern. * Tweak input type of `make_redundant_alias` --------- Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This commit is contained in:
parent
96f6288622
commit
da882b6657
|
|
@ -1,4 +1,4 @@
|
||||||
"""__init__.py with __all__
|
"""__init__.py with nonempty __all__
|
||||||
|
|
||||||
Unused stdlib and third party imports are unsafe removals
|
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 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"]
|
__all__ = ["argparse", "exported"]
|
||||||
11
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/__init__.py
vendored
Normal file
11
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/__init__.py
vendored
Normal file
|
|
@ -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__ = []
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/renamed.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/renamed.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/unused.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_26__all_empty/unused.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
11
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/__init__.py
vendored
Normal file
11
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/__init__.py
vendored
Normal file
|
|
@ -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
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/renamed.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/renamed.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/unused.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_27__all_mistyped/unused.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
8
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/__init__.py
vendored
Normal file
8
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/__init__.py
vendored
Normal file
|
|
@ -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__ = [];
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/renamed.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/renamed.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/unused.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_28__all_multiple/unused.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
16
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/__init__.py
vendored
Normal file
16
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/__init__.py
vendored
Normal file
|
|
@ -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"]
|
||||||
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/exported.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/exported.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/renamed.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/renamed.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/unused.py
vendored
Normal file
1
crates/ruff_linter/resources/test/fixtures/pyflakes/F401_29__all_conditional/unused.py
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
# empty module imported by __init__.py for test fixture
|
||||||
|
|
@ -1,10 +1,12 @@
|
||||||
//! Interface for generating fix edits from higher-level actions (e.g., "remove an argument").
|
//! Interface for generating fix edits from higher-level actions (e.g., "remove an argument").
|
||||||
|
|
||||||
|
use std::borrow::Cow;
|
||||||
|
|
||||||
use anyhow::{Context, Result};
|
use anyhow::{Context, Result};
|
||||||
|
|
||||||
use ruff_diagnostics::Edit;
|
use ruff_diagnostics::Edit;
|
||||||
use ruff_python_ast::parenthesize::parenthesized_range;
|
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_ast::{AnyNodeRef, ArgOrKeyword};
|
||||||
use ruff_python_codegen::Stylist;
|
use ruff_python_codegen::Stylist;
|
||||||
use ruff_python_index::Indexer;
|
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`.
|
/// Edits to make the specified imports explicit, e.g. change `import x` to `import x as x`.
|
||||||
pub(crate) fn make_redundant_alias<'a>(
|
pub(crate) fn make_redundant_alias<'a>(
|
||||||
member_names: impl Iterator<Item = &'a str>,
|
member_names: impl Iterator<Item = Cow<'a, str>>,
|
||||||
stmt: &Stmt,
|
stmt: &Stmt,
|
||||||
) -> Vec<Edit> {
|
) -> Vec<Edit> {
|
||||||
let aliases = match stmt {
|
let aliases = match stmt {
|
||||||
|
|
@ -144,6 +146,53 @@ pub(crate) fn make_redundant_alias<'a>(
|
||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Fix to add the specified imports to the `__all__` export list.
|
||||||
|
pub(crate) fn add_to_dunder_all<'a>(
|
||||||
|
names: impl Iterator<Item = &'a str>,
|
||||||
|
expr: &Expr,
|
||||||
|
stylist: &Stylist,
|
||||||
|
) -> Vec<Edit> {
|
||||||
|
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)]
|
#[derive(Debug, Copy, Clone)]
|
||||||
pub(crate) enum Parentheses {
|
pub(crate) enum Parentheses {
|
||||||
/// Remove parentheses, if the removed argument is the only argument left.
|
/// Remove parentheses, if the removed argument is the only argument left.
|
||||||
|
|
@ -477,14 +526,20 @@ fn all_lines_fit(
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use anyhow::Result;
|
use anyhow::{anyhow, Result};
|
||||||
|
use std::borrow::Cow;
|
||||||
|
use test_case::test_case;
|
||||||
|
|
||||||
use ruff_diagnostics::Edit;
|
use ruff_diagnostics::{Diagnostic, Edit, Fix};
|
||||||
use ruff_python_parser::parse_suite;
|
use ruff_python_codegen::Stylist;
|
||||||
|
use ruff_python_parser::{lexer, parse_expression, parse_suite, Mode};
|
||||||
use ruff_source_file::Locator;
|
use ruff_source_file::Locator;
|
||||||
use ruff_text_size::{Ranged, TextRange, TextSize};
|
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]
|
#[test]
|
||||||
fn find_semicolon() -> Result<()> {
|
fn find_semicolon() -> Result<()> {
|
||||||
|
|
@ -562,7 +617,7 @@ x = 1 \
|
||||||
let program = parse_suite(contents).unwrap();
|
let program = parse_suite(contents).unwrap();
|
||||||
let stmt = program.first().unwrap();
|
let stmt = program.first().unwrap();
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
make_redundant_alias(["x"].into_iter(), stmt),
|
make_redundant_alias(["x"].into_iter().map(Cow::from), stmt),
|
||||||
vec![Edit::range_replacement(
|
vec![Edit::range_replacement(
|
||||||
String::from("x as x"),
|
String::from("x as x"),
|
||||||
TextRange::new(TextSize::new(7), TextSize::new(8)),
|
TextRange::new(TextSize::new(7), TextSize::new(8)),
|
||||||
|
|
@ -570,7 +625,7 @@ x = 1 \
|
||||||
"make just one item redundant"
|
"make just one item redundant"
|
||||||
);
|
);
|
||||||
assert_eq!(
|
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(
|
vec![Edit::range_replacement(
|
||||||
String::from("x as x"),
|
String::from("x as x"),
|
||||||
TextRange::new(TextSize::new(7), TextSize::new(8)),
|
TextRange::new(TextSize::new(7), TextSize::new(8)),
|
||||||
|
|
@ -578,7 +633,7 @@ x = 1 \
|
||||||
"the second item is already a redundant alias"
|
"the second item is already a redundant alias"
|
||||||
);
|
);
|
||||||
assert_eq!(
|
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(
|
vec![Edit::range_replacement(
|
||||||
String::from("x as x"),
|
String::from("x as x"),
|
||||||
TextRange::new(TextSize::new(7), TextSize::new(8)),
|
TextRange::new(TextSize::new(7), TextSize::new(8)),
|
||||||
|
|
@ -586,4 +641,47 @@ x = 1 \
|
||||||
"the third item is already aliased to something else"
|
"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::<Vec<_>>(),
|
||||||
|
&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(())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -207,7 +207,11 @@ mod tests {
|
||||||
#[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))]
|
#[test_case(Rule::UnusedVariable, Path::new("F841_4.py"))]
|
||||||
#[test_case(Rule::UnusedImport, Path::new("__init__.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_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<()> {
|
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!(
|
let snapshot = format!(
|
||||||
"preview__{}_{}",
|
"preview__{}_{}",
|
||||||
|
|
|
||||||
|
|
@ -6,8 +6,11 @@ use rustc_hash::FxHashMap;
|
||||||
|
|
||||||
use ruff_diagnostics::{Applicability, Diagnostic, Fix, FixAvailability, Violation};
|
use ruff_diagnostics::{Applicability, Diagnostic, Fix, FixAvailability, Violation};
|
||||||
use ruff_macros::{derive_message_formats, violation};
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
use ruff_python_ast as ast;
|
||||||
use ruff_python_ast::{Stmt, StmtImportFrom};
|
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 ruff_text_size::{Ranged, TextRange};
|
||||||
|
|
||||||
use crate::checkers::ast::Checker;
|
use crate::checkers::ast::Checker;
|
||||||
|
|
@ -18,7 +21,10 @@ use crate::rules::{isort, isort::ImportSection, isort::ImportType};
|
||||||
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
|
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
|
||||||
enum UnusedImportContext {
|
enum UnusedImportContext {
|
||||||
ExceptHandler,
|
ExceptHandler,
|
||||||
Init { first_party: bool },
|
Init {
|
||||||
|
first_party: bool,
|
||||||
|
dunder_all_count: usize,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
/// ## What it does
|
/// ## 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)
|
/// - [Typing documentation: interface conventions](https://typing.readthedocs.io/en/latest/source/libraries.html#library-interface-public-and-private-symbols)
|
||||||
#[violation]
|
#[violation]
|
||||||
pub struct UnusedImport {
|
pub struct UnusedImport {
|
||||||
|
/// Qualified name of the import
|
||||||
name: String,
|
name: String,
|
||||||
|
/// Name of the import binding
|
||||||
|
binding: String,
|
||||||
context: Option<UnusedImportContext>,
|
context: Option<UnusedImportContext>,
|
||||||
multiple: bool,
|
multiple: bool,
|
||||||
}
|
}
|
||||||
|
|
@ -106,16 +115,31 @@ impl Violation for UnusedImport {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn fix_title(&self) -> Option<String> {
|
fn fix_title(&self) -> Option<String> {
|
||||||
let UnusedImport { name, multiple, .. } = self;
|
let UnusedImport {
|
||||||
let resolution = match self.context {
|
name,
|
||||||
Some(UnusedImportContext::Init { first_party: true }) => "Use a redundant alias",
|
binding,
|
||||||
_ => "Remove unused import",
|
multiple,
|
||||||
};
|
..
|
||||||
Some(if *multiple {
|
} = self;
|
||||||
resolution.to_string()
|
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 {
|
} else {
|
||||||
format!("{resolution}: `{name}`")
|
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...
|
/// 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 ∧ stdlib → unsafe, remove
|
||||||
/// __init__.py ∧ 3rdpty → 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 {
|
let import = ImportBinding {
|
||||||
|
name: binding.name(checker.locator()),
|
||||||
import,
|
import,
|
||||||
range: binding.range(),
|
range: binding.range(),
|
||||||
parent_range: binding.parent_range(checker.semantic()),
|
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 in_init = checker.path().ends_with("__init__.py");
|
||||||
let fix_init = checker.settings.preview.is_enabled();
|
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
|
// Generate a diagnostic for every import, but share fixes across all imports within the same
|
||||||
// statement (excluding those that are ignored).
|
// statement (excluding those that are ignored).
|
||||||
|
|
@ -225,6 +274,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
|
||||||
level,
|
level,
|
||||||
checker,
|
checker,
|
||||||
),
|
),
|
||||||
|
dunder_all_count: dunder_all_exprs.len(),
|
||||||
})
|
})
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
|
|
@ -234,7 +284,10 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
|
||||||
.partition(|(_, context)| {
|
.partition(|(_, context)| {
|
||||||
matches!(
|
matches!(
|
||||||
context,
|
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(
|
fix_by_reexporting(
|
||||||
checker,
|
checker,
|
||||||
import_statement,
|
import_statement,
|
||||||
to_reexport.iter().map(|(binding, _)| binding),
|
&to_reexport.iter().map(|(b, _)| b).collect::<Vec<_>>(),
|
||||||
|
&dunder_all_exprs,
|
||||||
)
|
)
|
||||||
.ok(),
|
.ok(),
|
||||||
)
|
)
|
||||||
|
|
@ -266,6 +320,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
|
||||||
let mut diagnostic = Diagnostic::new(
|
let mut diagnostic = Diagnostic::new(
|
||||||
UnusedImport {
|
UnusedImport {
|
||||||
name: binding.import.qualified_name().to_string(),
|
name: binding.import.qualified_name().to_string(),
|
||||||
|
binding: binding.name.to_string(),
|
||||||
context,
|
context,
|
||||||
multiple,
|
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
|
// Separately, generate a diagnostic for every _ignored_ import, to ensure that the
|
||||||
// suppression comments aren't marked as unused.
|
// suppression comments aren't marked as unused.
|
||||||
for ImportBinding {
|
for binding in ignored.into_values().flatten() {
|
||||||
import,
|
|
||||||
range,
|
|
||||||
parent_range,
|
|
||||||
} in ignored.into_values().flatten()
|
|
||||||
{
|
|
||||||
let mut diagnostic = Diagnostic::new(
|
let mut diagnostic = Diagnostic::new(
|
||||||
UnusedImport {
|
UnusedImport {
|
||||||
name: import.qualified_name().to_string(),
|
name: binding.import.qualified_name().to_string(),
|
||||||
|
binding: binding.name.to_string(),
|
||||||
context: None,
|
context: None,
|
||||||
multiple: false,
|
multiple: false,
|
||||||
},
|
},
|
||||||
range,
|
binding.range,
|
||||||
);
|
);
|
||||||
if let Some(range) = parent_range {
|
if let Some(range) = binding.parent_range {
|
||||||
diagnostic.set_parent(range.start());
|
diagnostic.set_parent(range.start());
|
||||||
}
|
}
|
||||||
diagnostics.push(diagnostic);
|
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.
|
/// An unused import with its surrounding context.
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
struct ImportBinding<'a> {
|
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`).
|
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
|
||||||
import: AnyImport<'a, 'a>,
|
import: AnyImport<'a, 'a>,
|
||||||
/// The trimmed range of the import (e.g., `List` in `from typing import List`).
|
/// 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
|
/// Generate a [`Fix`] to make bindings in a statement explicit, either by adding them to `__all__`
|
||||||
/// `import a as a`.
|
/// or changing them from `import a` to `import a as a`.
|
||||||
fn fix_by_reexporting<'a>(
|
fn fix_by_reexporting(
|
||||||
checker: &Checker,
|
checker: &Checker,
|
||||||
node_id: NodeId,
|
node_id: NodeId,
|
||||||
imports: impl Iterator<Item = &'a ImportBinding<'a>>,
|
imports: &[&ImportBinding],
|
||||||
|
dunder_all_exprs: &[&ast::Expr],
|
||||||
) -> Result<Fix> {
|
) -> Result<Fix> {
|
||||||
let statement = checker.semantic().statement(node_id);
|
let statement = checker.semantic().statement(node_id);
|
||||||
|
if imports.is_empty() {
|
||||||
let member_names = imports
|
|
||||||
.map(|binding| binding.import.member_name())
|
|
||||||
.collect::<Vec<_>>();
|
|
||||||
if member_names.is_empty() {
|
|
||||||
bail!("Expected import bindings");
|
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
|
// Only emit a fix if there are edits
|
||||||
let mut tail = edits.into_iter();
|
let mut tail = edits.into_iter();
|
||||||
|
|
|
||||||
|
|
@ -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
|
33 | from . import unused # F401: change to redundant alias
|
||||||
| ^^^^^^ F401
|
| ^^^^^^ F401
|
||||||
|
|
|
|
||||||
= help: Use a redundant alias: `.unused`
|
= help: Use an explicit re-export: `unused as unused`
|
||||||
|
|
||||||
ℹ Safe fix
|
ℹ Safe fix
|
||||||
30 30 | from . import aliased as aliased # Ok: is redundant alias
|
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
|
36 | from . import renamed as bees # F401: no fix
|
||||||
| ^^^^ F401
|
| ^^^^ F401
|
||||||
|
|
|
|
||||||
= help: Use a redundant alias: `.renamed`
|
= help: Use an explicit re-export: `bees as bees`
|
||||||
|
|
|
||||||
|
|
@ -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
|
|
||||||
|
|
@ -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"]
|
||||||
|
|
@ -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"]
|
||||||
|
|
@ -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__
|
||||||
|
|
@ -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"];
|
||||||
|
|
@ -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
|
||||||
Loading…
Reference in New Issue