From 754672193e3f6f8206466c9e4f727da944a8fb4b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 26 Jul 2024 12:57:46 -0400 Subject: [PATCH] Ignore --- .../fixtures/isort/required_imports/unused.py | 20 ++++++++ crates/ruff_linter/src/rules/isort/mod.rs | 34 +++++++++++++ ...ort__tests__required_import_unused.py.snap | 40 ++++++++++++++++ .../src/rules/pyflakes/rules/unused_import.rs | 16 ++++++- crates/ruff_python_semantic/src/imports.rs | 48 +++++++++++++++++++ 5 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/isort/required_imports/unused.py create mode 100644 crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__required_import_unused.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/isort/required_imports/unused.py b/crates/ruff_linter/resources/test/fixtures/isort/required_imports/unused.py new file mode 100644 index 0000000000000..78d3e89775185 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/isort/required_imports/unused.py @@ -0,0 +1,20 @@ +# Unused, but marked as required. +import os + +# Unused, _not_ marked as required. +import sys + +# Unused, _not_ marked as required (due to the alias). +import pathlib as non_alias + +# Unused, marked as required. +import shelve as alias + +# Unused, but marked as required. +from typing import List + +# Unused, but marked as required. +from typing import Set as SetAlias + +# Unused, but marked as required. +import urllib.parse diff --git a/crates/ruff_linter/src/rules/isort/mod.rs b/crates/ruff_linter/src/rules/isort/mod.rs index e10913435f763..c2a7d3764c32b 100644 --- a/crates/ruff_linter/src/rules/isort/mod.rs +++ b/crates/ruff_linter/src/rules/isort/mod.rs @@ -906,6 +906,40 @@ mod tests { Ok(()) } + #[test_case(Path::new("unused.py"))] + fn required_import_unused(path: &Path) -> Result<()> { + let snapshot = format!("required_import_{}", path.to_string_lossy()); + let diagnostics = test_path( + Path::new("isort/required_imports").join(path).as_path(), + &LinterSettings { + src: vec![test_resource_path("fixtures/isort")], + isort: super::settings::Settings { + required_imports: BTreeSet::from_iter([ + NameImport::Import(ModuleNameImport::module("os".to_string())), + NameImport::Import(ModuleNameImport::alias( + "shelve".to_string(), + "alias".to_string(), + )), + NameImport::ImportFrom(MemberNameImport::member( + "typing".to_string(), + "List".to_string(), + )), + NameImport::ImportFrom(MemberNameImport::alias( + "typing".to_string(), + "Set".to_string(), + "SetAlias".to_string(), + )), + NameImport::Import(ModuleNameImport::module("urllib.parse".to_string())), + ]), + ..super::settings::Settings::default() + }, + ..LinterSettings::for_rules([Rule::MissingRequiredImport, Rule::UnusedImport]) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + #[test_case(Path::new("from_first.py"))] fn from_first(path: &Path) -> Result<()> { let snapshot = format!("from_first_{}", path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__required_import_unused.py.snap b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__required_import_unused.py.snap new file mode 100644 index 0000000000000..0d79276e0e4ff --- /dev/null +++ b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__required_import_unused.py.snap @@ -0,0 +1,40 @@ +--- +source: crates/ruff_linter/src/rules/isort/mod.rs +--- +unused.py:5:8: F401 [*] `sys` imported but unused + | +4 | # Unused, _not_ marked as required. +5 | import sys + | ^^^ F401 +6 | +7 | # Unused, _not_ marked as required (due to the alias). + | + = help: Remove unused import: `sys` + +ℹ Safe fix +2 2 | import os +3 3 | +4 4 | # Unused, _not_ marked as required. +5 |-import sys +6 5 | +7 6 | # Unused, _not_ marked as required (due to the alias). +8 7 | import pathlib as non_alias + +unused.py:8:19: F401 [*] `pathlib` imported but unused + | + 7 | # Unused, _not_ marked as required (due to the alias). + 8 | import pathlib as non_alias + | ^^^^^^^^^ F401 + 9 | +10 | # Unused, marked as required. + | + = help: Remove unused import: `pathlib` + +ℹ Safe fix +5 5 | import sys +6 6 | +7 7 | # Unused, _not_ marked as required (due to the alias). +8 |-import pathlib as non_alias +9 8 | +10 9 | # Unused, marked as required. +11 10 | import shelve as alias 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 60347b06d95b6..bfa884801ccf2 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs @@ -242,8 +242,22 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut continue; }; + let name = binding.name(checker.locator()); + + // If an import is marked as required, avoid treating it as unused, regardless of whether + // it was _actually_ used. + if checker + .settings + .isort + .required_imports + .iter() + .any(|required_import| required_import.matches(name, &import)) + { + continue; + } + let import = ImportBinding { - name: binding.name(checker.locator()), + name, import, range: binding.range(), parent_range: binding.parent_range(checker.semantic()), diff --git a/crates/ruff_python_semantic/src/imports.rs b/crates/ruff_python_semantic/src/imports.rs index b7811910f1354..6e7d1a0eaacc9 100644 --- a/crates/ruff_python_semantic/src/imports.rs +++ b/crates/ruff_python_semantic/src/imports.rs @@ -1,4 +1,8 @@ use ruff_macros::CacheKey; +use ruff_python_ast::helpers::collect_import_from_member; +use ruff_python_ast::name::QualifiedName; + +use crate::{AnyImport, Imported}; /// A list of names imported via any import statement. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, CacheKey)] @@ -37,6 +41,41 @@ impl NameImports { } } +impl NameImport { + /// Returns the name under which the member is bound (e.g., given `from foo import bar as baz`, returns `baz`). + fn bound_name(&self) -> &str { + match self { + NameImport::Import(import) => { + import.name.as_name.as_deref().unwrap_or(&import.name.name) + } + NameImport::ImportFrom(import_from) => import_from + .name + .as_name + .as_deref() + .unwrap_or(&import_from.name.name), + } + } + + /// Returns the [`QualifiedName`] of the imported name (e.g., given `import foo import bar as baz`, returns `["foo", "bar"]`). + fn qualified_name(&self) -> QualifiedName { + match self { + NameImport::Import(import) => QualifiedName::user_defined(&import.name.name), + NameImport::ImportFrom(import_from) => collect_import_from_member( + import_from.level, + import_from.module.as_deref(), + import_from.name.name.as_str(), + ), + } + } +} + +impl NameImport { + /// Returns `true` if the [`NameImport`] matches the specified name and binding. + pub fn matches(&self, name: &str, binding: &AnyImport) -> bool { + name == self.bound_name() && self.qualified_name() == *binding.qualified_name() + } +} + impl ModuleNameImport { /// Creates a new `Import` to import the specified module. pub fn module(name: String) -> Self { @@ -47,6 +86,15 @@ impl ModuleNameImport { }, } } + + pub fn alias(name: String, as_name: String) -> Self { + Self { + name: Alias { + name, + as_name: Some(as_name), + }, + } + } } impl MemberNameImport {