Skip to content

Commit

Permalink
Ignore
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jul 26, 2024
1 parent 6276b05 commit e6ea9f4
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -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
34 changes: 34 additions & 0 deletions crates/ruff_linter/src/rules/isort/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
@@ -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
16 changes: 15 additions & 1 deletion crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
54 changes: 54 additions & 0 deletions crates/ruff_python_semantic/src/imports.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -37,6 +41,47 @@ 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 {
println!("name: {}, binding: {:?}", name, binding);
println!(
"self.bound_name(): {}, self.qualified_name(): {:?}",
self.bound_name(),
self.qualified_name()
);
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 {
Expand All @@ -47,6 +92,15 @@ impl ModuleNameImport {
},
}
}

pub fn alias(name: String, as_name: String) -> Self {
Self {
name: Alias {
name,
as_name: Some(as_name),
},
}
}
}

impl MemberNameImport {
Expand Down

0 comments on commit e6ea9f4

Please sign in to comment.