This commit is contained in:
Charlie Marsh 2023-07-27 16:59:56 -04:00
parent 5dbe4e4653
commit 3eec7de6e4
10 changed files with 86 additions and 292 deletions

View File

@ -83,6 +83,7 @@ pub(crate) fn remove_imports<'a>(
let trailing_comma = aliases.last().and_then(|alias| alias.comma.clone());
for import in imports {
// If it's an import-from, we only care about members.
let alias_index = aliases.iter().position(|alias| {
let qualified_name = match import_module {
Some((relative, module)) => {

View File

@ -39,7 +39,7 @@ use ruff_text_size::{TextRange, TextSize};
use ruff_diagnostics::{Diagnostic, IsolationLevel};
use ruff_python_ast::all::{extract_all_names, DunderAllFlags};
use ruff_python_ast::helpers::{
extract_handled_exceptions, from_relative_import_parts, to_module_path,
extract_handled_exceptions, from_relative_import_parts, literal_path, to_module_path,
};
use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::str::trailing_quote;
@ -390,12 +390,16 @@ where
// be "foo.bar". Given `from foo import bar as baz`, `name` would be "baz"
// and `qualified_name` would be "foo.bar".
let name = alias.asname.as_ref().unwrap_or(&alias.name);
if let Some(call_path) = from_relative_import_parts(
self.module_path.unwrap_or_default(),
level,
module,
&alias.name,
) {
// Attempt to resolve any relative imports; but if we don't know the current
// module path, or the relative import extends beyond the package root,
// fallback to a literal representation (e.g., `[".", "foo"]`).
let call_path = self
.module_path
.and_then(|module_path| {
from_relative_import_parts(module_path, level, module, &alias.name)
})
.unwrap_or_else(|| literal_path(level, module, &alias.name));
let call_path: Box<[&str]> = call_path.into_boxed_slice();
self.add_binding(
name,
@ -406,7 +410,6 @@ where
}
}
}
}
Stmt::Global(ast::StmtGlobal { names, range: _ }) => {
if !self.semantic.scope_id.is_global() {
for name in names {

View File

@ -206,6 +206,9 @@ fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result
.expect("Expected at least one import");
// Step 1) Remove the import.
// This could be a mix of imports...
// One framing could be: what's the name of the symbol being imported (e.g., `List` in `from typing import List`,
// or `foo` in `import foo`, or `foo.bar` in `import foo.bar`).
let remove_import_edit = autofix::edits::remove_unused_imports(
qualified_names.iter().map(|name| name.as_str()),
stmt,

View File

@ -111,7 +111,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
continue;
}
let Some(qualified_name) = binding.qualified_name() else {
let Some(member_name) = binding.member_name() else {
continue;
};
@ -120,7 +120,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
};
let import = Import {
qualified_name,
member_name,
range: binding.range,
parent_range: binding.parent_range(checker.semantic()),
};
@ -159,14 +159,14 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
};
for Import {
qualified_name,
member_name,
range,
parent_range,
} in imports
{
let mut diagnostic = Diagnostic::new(
UnusedImport {
name: qualified_name.to_string(),
name: member_name.to_string(),
context: if in_except_handler {
Some(UnusedImportContext::ExceptHandler)
} else if in_init {
@ -193,14 +193,14 @@ 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 Import {
qualified_name,
member_name,
range,
parent_range,
} in ignored.into_values().flatten()
{
let mut diagnostic = Diagnostic::new(
UnusedImport {
name: qualified_name.to_string(),
name: member_name.to_string(),
context: None,
multiple: false,
},
@ -216,7 +216,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
/// An unused import with its surrounding context.
struct Import {
/// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
qualified_name: String,
member_name: String,
/// The trimmed range of the import (e.g., `List` in `from typing import List`).
range: TextRange,
/// The range of the import's parent statement.
@ -230,7 +230,7 @@ fn fix_imports(checker: &Checker, stmt_id: NodeId, imports: &[Import]) -> Result
let edit = autofix::edits::remove_unused_imports(
imports
.iter()
.map(|Import { qualified_name, .. }| qualified_name.as_str()),
.map(|Import { member_name, .. }| member_name.as_str()),
stmt,
parent,
checker.locator(),

View File

@ -57,6 +57,8 @@ impl AlwaysAutofixableViolation for OSErrorAlias {
/// Return `true` if an [`Expr`] is an alias of `OSError`.
fn is_alias(expr: &Expr, semantic: &SemanticModel) -> bool {
semantic.resolve_call_path(expr).map_or(false, |call_path| {
println!("expr: {:?}", expr);
println!("call_path: {:?}", call_path);
matches!(
call_path.as_slice(),
["", "EnvironmentError" | "IOError" | "WindowsError"]

View File

@ -126,6 +126,7 @@ pub(crate) fn unnecessary_builtin_import(
let parent = checker.semantic().stmt_parent();
let unused_imports: Vec<String> = unused_imports
.iter()
// These are always ImportFrom.
.map(|alias| format!("{module}.{}", alias.name))
.collect();
let edit = autofix::edits::remove_unused_imports(

View File

@ -1,268 +0,0 @@
---
source: crates/ruff/src/rules/pyupgrade/mod.rs
assertion_line: 88
---
UP024_0.py:6:8: UP024 [*] Replace aliased errors with `OSError`
|
4 | try:
5 | pass
6 | except EnvironmentError:
| ^^^^^^^^^^^^^^^^ UP024
7 | pass
|
= help: Replace `EnvironmentError` with builtin `OSError`
Fix
3 3 | # These should be fixed
4 4 | try:
5 5 | pass
6 |-except EnvironmentError:
6 |+except OSError:
7 7 | pass
8 8 |
9 9 | try:
UP024_0.py:11:8: UP024 [*] Replace aliased errors with `OSError`
|
9 | try:
10 | pass
11 | except IOError:
| ^^^^^^^ UP024
12 | pass
|
= help: Replace `IOError` with builtin `OSError`
Fix
8 8 |
9 9 | try:
10 10 | pass
11 |-except IOError:
11 |+except OSError:
12 12 | pass
13 13 |
14 14 | try:
UP024_0.py:16:8: UP024 [*] Replace aliased errors with `OSError`
|
14 | try:
15 | pass
16 | except WindowsError:
| ^^^^^^^^^^^^ UP024
17 | pass
|
= help: Replace `WindowsError` with builtin `OSError`
Fix
13 13 |
14 14 | try:
15 15 | pass
16 |-except WindowsError:
16 |+except OSError:
17 17 | pass
18 18 |
19 19 | try:
UP024_0.py:21:8: UP024 [*] Replace aliased errors with `OSError`
|
19 | try:
20 | pass
21 | except mmap.error:
| ^^^^^^^^^^ UP024
22 | pass
|
= help: Replace `mmap.error` with builtin `OSError`
Fix
18 18 |
19 19 | try:
20 20 | pass
21 |-except mmap.error:
21 |+except OSError:
22 22 | pass
23 23 |
24 24 | try:
UP024_0.py:26:8: UP024 [*] Replace aliased errors with `OSError`
|
24 | try:
25 | pass
26 | except select.error:
| ^^^^^^^^^^^^ UP024
27 | pass
|
= help: Replace `select.error` with builtin `OSError`
Fix
23 23 |
24 24 | try:
25 25 | pass
26 |-except select.error:
26 |+except OSError:
27 27 | pass
28 28 |
29 29 | try:
UP024_0.py:31:8: UP024 [*] Replace aliased errors with `OSError`
|
29 | try:
30 | pass
31 | except socket.error:
| ^^^^^^^^^^^^ UP024
32 | pass
|
= help: Replace `socket.error` with builtin `OSError`
Fix
28 28 |
29 29 | try:
30 30 | pass
31 |-except socket.error:
31 |+except OSError:
32 32 | pass
33 33 |
34 34 | try:
UP024_0.py:36:8: UP024 [*] Replace aliased errors with `OSError`
|
34 | try:
35 | pass
36 | except error:
| ^^^^^ UP024
37 | pass
|
= help: Replace `error` with builtin `OSError`
Fix
33 33 |
34 34 | try:
35 35 | pass
36 |-except error:
36 |+except OSError:
37 37 | pass
38 38 |
39 39 | # Should NOT be in parentheses when replaced
UP024_0.py:43:8: UP024 [*] Replace aliased errors with `OSError`
|
41 | try:
42 | pass
43 | except (IOError,):
| ^^^^^^^^^^ UP024
44 | pass
45 | try:
|
= help: Replace with builtin `OSError`
Fix
40 40 |
41 41 | try:
42 42 | pass
43 |-except (IOError,):
43 |+except OSError:
44 44 | pass
45 45 | try:
46 46 | pass
UP024_0.py:47:8: UP024 [*] Replace aliased errors with `OSError`
|
45 | try:
46 | pass
47 | except (mmap.error,):
| ^^^^^^^^^^^^^ UP024
48 | pass
49 | try:
|
= help: Replace with builtin `OSError`
Fix
44 44 | pass
45 45 | try:
46 46 | pass
47 |-except (mmap.error,):
47 |+except OSError:
48 48 | pass
49 49 | try:
50 50 | pass
UP024_0.py:51:8: UP024 [*] Replace aliased errors with `OSError`
|
49 | try:
50 | pass
51 | except (EnvironmentError, IOError, OSError, select.error):
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP024
52 | pass
|
= help: Replace with builtin `OSError`
Fix
48 48 | pass
49 49 | try:
50 50 | pass
51 |-except (EnvironmentError, IOError, OSError, select.error):
51 |+except OSError:
52 52 | pass
53 53 |
54 54 | # Should be kept in parentheses (because multiple)
UP024_0.py:58:8: UP024 [*] Replace aliased errors with `OSError`
|
56 | try:
57 | pass
58 | except (IOError, KeyError, OSError):
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP024
59 | pass
|
= help: Replace with builtin `OSError`
Fix
55 55 |
56 56 | try:
57 57 | pass
58 |-except (IOError, KeyError, OSError):
58 |+except (KeyError, OSError):
59 59 | pass
60 60 |
61 61 | # First should change, second should not
UP024_0.py:65:8: UP024 [*] Replace aliased errors with `OSError`
|
63 | try:
64 | pass
65 | except (IOError, error):
| ^^^^^^^^^^^^^^^^ UP024
66 | pass
67 | # These should not change
|
= help: Replace with builtin `OSError`
Fix
62 62 | from .mmap import error
63 63 | try:
64 64 | pass
65 |-except (IOError, error):
65 |+except OSError:
66 66 | pass
67 67 | # These should not change
68 68 |
UP024_0.py:87:8: UP024 [*] Replace aliased errors with `OSError`
|
85 | try:
86 | pass
87 | except (mmap).error:
| ^^^^^^^^^^^^ UP024
88 | pass
|
= help: Replace `mmap.error` with builtin `OSError`
Fix
84 84 | pass
85 85 | try:
86 86 | pass
87 |-except (mmap).error:
87 |+except OSError:
88 88 | pass
89 89 |
90 90 | try:

View File

@ -909,7 +909,7 @@ pub fn from_relative_import_parts<'a>(
for _ in 0..level {
if call_path.is_empty() {
return None;
break;
}
call_path.pop();
}
@ -927,6 +927,39 @@ pub fn from_relative_import_parts<'a>(
Some(call_path)
}
pub fn literal_path<'a>(
level: Option<u32>,
module: Option<&'a str>,
member: &'a str,
) -> CallPath<'a> {
let mut call_path: CallPath = SmallVec::with_capacity(
level.unwrap_or_default() as usize
+ module
.map(|module| module.split('.').count())
.unwrap_or_default()
+ 1,
);
// Remove segments based on the number of dots.
if let Some(level) = level {
if level > 0 {
for _ in 0..level {
call_path.push(".");
}
}
}
// Add the remaining segments.
if let Some(module) = module {
call_path.extend(module.split('.'));
}
// Add the member.
call_path.push(member);
call_path
}
/// Given an imported module (based on its relative import level and module name), return the
/// fully-qualified module path.
pub fn resolve_imported_module_path<'a>(

View File

@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::ops::{Deref, DerefMut};
use bitflags::bitflags;
@ -204,6 +205,22 @@ impl<'a> Binding<'a> {
}
}
/// Returns the fully-qualified symbol name, if this symbol was imported from another module.
pub fn member_name(&self) -> Option<Cow<'a, str>> {
match &self.kind {
BindingKind::Import(Import { call_path }) => {
Some(Cow::Owned(format_call_path(call_path)))
}
BindingKind::FromImport(FromImport { call_path }) => {
call_path.last().map(|member| Cow::Borrowed(*member))
}
BindingKind::SubmoduleImport(SubmoduleImport { call_path }) => {
Some(Cow::Owned(format_call_path(call_path)))
}
_ => None,
}
}
/// Returns the name of the binding (e.g., `x` in `x = 1`).
pub fn name<'b>(&self, locator: &'b Locator) -> &'b str {
locator.slice(self.range)

2
foo.py Normal file
View File

@ -0,0 +1,2 @@
# F401 `datastructures.UploadFile` imported but unused
from .datastructures import UploadFile as FileUpload