Move import-name matching into methods on BindingKind (#4818)

This commit is contained in:
Charlie Marsh 2023-06-03 15:01:27 -04:00 committed by GitHub
parent 2c41c54e0c
commit 935094c2ff
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 96 additions and 134 deletions

View file

@ -2,7 +2,7 @@ from __future__ import annotations
def f():
# Even in strict mode, this shouldn't rase an error, since `pkg` is used at runtime,
# Even in strict mode, this shouldn't raise an error, since `pkg` is used at runtime,
# and implicitly imports `pkg.bar`.
import pkg
import pkg.bar
@ -12,7 +12,7 @@ def f():
def f():
# Even in strict mode, this shouldn't rase an error, since `pkg.bar` is used at
# Even in strict mode, this shouldn't raise an error, since `pkg.bar` is used at
# runtime, and implicitly imports `pkg`.
import pkg
import pkg.bar
@ -22,7 +22,7 @@ def f():
def f():
# In un-strict mode, this shouldn't rase an error, since `pkg` is used at runtime.
# In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
import pkg
from pkg import A
@ -31,7 +31,7 @@ def f():
def f():
# In un-strict mode, this shouldn't rase an error, since `pkg` is used at runtime.
# In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
from pkg import A, B
def test(value: A):
@ -39,7 +39,7 @@ def f():
def f():
# Even in strict mode, this shouldn't rase an error, since `pkg.baz` is used at
# Even in strict mode, this shouldn't raise an error, since `pkg.baz` is used at
# runtime, and implicitly imports `pkg.bar`.
import pkg.bar
import pkg.baz
@ -49,7 +49,7 @@ def f():
def f():
# In un-strict mode, this _should_ rase an error, since `pkg` is used at runtime.
# In un-strict mode, this _should_ raise an error, since `pkg.bar` isn't used at runtime
import pkg
from pkg.bar import A
@ -58,7 +58,7 @@ def f():
def f():
# In un-strict mode, this shouldn't rase an error, since `pkg.bar` is used at runtime.
# In un-strict mode, this shouldn't raise an error, since `pkg.bar` is used at runtime.
import pkg
import pkg.bar as B
@ -67,7 +67,7 @@ def f():
def f():
# In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime.
# In un-strict mode, this shouldn't raise an error, since `pkg.foo.bar` is used at runtime.
import pkg.foo as F
import pkg.foo.bar as B
@ -76,7 +76,7 @@ def f():
def f():
# In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime.
# In un-strict mode, this shouldn't raise an error, since `pkg.foo.bar` is used at runtime.
import pkg
import pkg.foo.bar as B
@ -85,7 +85,7 @@ def f():
def f():
# In un-strict mode, this _should_ rase an error, since `pkgfoo.bar` is used at runtime.
# In un-strict mode, this _should_ raise an error, since `pkg` isn't used at runtime.
# Note that `pkg` is a prefix of `pkgfoo` which are both different modules. This is
# testing the implementation.
import pkg
@ -96,7 +96,7 @@ def f():
def f():
# In un-strict mode, this shouldn't raise an error, since `pkg.bar` is used at runtime.
# In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
import pkg.bar as B
import pkg.foo as F

View file

@ -1,8 +1,6 @@
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::binding::{
Binding, BindingKind, FromImportation, Importation, SubmoduleImportation,
};
use ruff_python_semantic::binding::Binding;
use crate::autofix;
use crate::checkers::ast::Checker;
@ -66,11 +64,8 @@ pub(crate) fn runtime_import_in_type_checking_block(
binding: &Binding,
diagnostics: &mut Vec<Diagnostic>,
) {
let full_name = match &binding.kind {
BindingKind::Importation(Importation { full_name }) => full_name,
BindingKind::FromImportation(FromImportation { full_name }) => full_name.as_str(),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => full_name,
_ => return,
let Some(qualified_name) = binding.qualified_name() else {
return;
};
let Some(reference_id) = binding.references.first() else {
@ -89,7 +84,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
{
let mut diagnostic = Diagnostic::new(
RuntimeImportInTypeCheckingBlock {
full_name: full_name.to_string(),
full_name: qualified_name.to_string(),
},
binding.range,
);
@ -102,7 +97,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
let stmt = checker.semantic_model().stmts[source];
let parent = checker.semantic_model().stmts.parent(stmt);
let remove_import_edit = autofix::edits::remove_unused_imports(
std::iter::once(full_name),
std::iter::once(qualified_name),
stmt,
parent,
checker.locator,
@ -113,7 +108,10 @@ pub(crate) fn runtime_import_in_type_checking_block(
// Step 2) Add the import to the top-level.
let reference = checker.semantic_model().references.resolve(*reference_id);
let add_import_edit = checker.importer.runtime_import_edit(
&StmtImport { stmt, full_name },
&StmtImport {
stmt,
full_name: qualified_name,
},
reference.range().start(),
)?;

View file

@ -1,8 +1,6 @@
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::binding::{
Binding, BindingKind, FromImportation, Importation, SubmoduleImportation,
};
use ruff_python_semantic::binding::Binding;
use crate::autofix;
use crate::checkers::ast::Checker;
@ -180,65 +178,13 @@ impl Violation for TypingOnlyStandardLibraryImport {
/// Return `true` if `this` is implicitly loaded via importing `that`.
fn is_implicit_import(this: &Binding, that: &Binding) -> bool {
match &this.kind {
BindingKind::Importation(Importation {
full_name: this_name,
})
| BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: this_name,
}) => match &that.kind {
BindingKind::FromImportation(FromImportation {
full_name: that_name,
}) => {
// Ex) `pkg.A` vs. `pkg`
let this_name = this_name.split('.').next().unwrap_or(this_name);
that_name
.rfind('.')
.map_or(false, |i| that_name[..i] == *this_name)
}
BindingKind::Importation(Importation {
full_name: that_name,
})
| BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: that_name,
}) => {
// Submodule importation with an alias (`import pkg.A as B`)
// are represented as `Importation`.
let this_name = this_name.split('.').next().unwrap_or(this_name);
let that_name = that_name.split('.').next().unwrap_or(that_name);
this_name == that_name
}
_ => false,
},
BindingKind::FromImportation(FromImportation {
full_name: this_name,
}) => match &that.kind {
BindingKind::Importation(Importation {
full_name: that_name,
})
| BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: that_name,
}) => {
// Ex) `pkg.A` vs. `pkg`
let that_name = that_name.split('.').next().unwrap_or(that_name);
this_name
.rfind('.')
.map_or(false, |i| &this_name[..i] == that_name)
}
BindingKind::FromImportation(FromImportation {
full_name: that_name,
}) => {
// Ex) `pkg.A` vs. `pkg.B`
this_name.rfind('.').map_or(false, |i| {
that_name
.rfind('.')
.map_or(false, |j| this_name[..i] == that_name[..j])
})
}
_ => false,
},
_ => false,
}
let Some(this_module) = this.module_name() else {
return false;
};
let Some(that_module) = that.module_name() else {
return false;
};
this_module == that_module
}
/// Return `true` if `name` is exempt from typing-only enforcement.
@ -274,15 +220,12 @@ pub(crate) fn typing_only_runtime_import(
return;
}
let full_name = match &binding.kind {
BindingKind::Importation(Importation { full_name }) => full_name,
BindingKind::FromImportation(FromImportation { full_name }) => full_name.as_str(),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => full_name,
_ => return,
let Some(qualified_name) = binding.qualified_name() else {
return;
};
if is_exempt(
full_name,
qualified_name,
&checker
.settings
.flake8_type_checking
@ -312,7 +255,7 @@ pub(crate) fn typing_only_runtime_import(
// Extract the module base and level from the full name.
// Ex) `foo.bar.baz` -> `foo`, `0`
// Ex) `.foo.bar.baz` -> `foo`, `1`
let level = full_name
let level = qualified_name
.chars()
.take_while(|c| *c == '.')
.count()
@ -321,7 +264,7 @@ pub(crate) fn typing_only_runtime_import(
// Categorize the import.
let mut diagnostic = match categorize(
full_name,
qualified_name,
Some(level),
&checker.settings.src,
checker.package(),
@ -331,7 +274,7 @@ pub(crate) fn typing_only_runtime_import(
ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => {
Diagnostic::new(
TypingOnlyFirstPartyImport {
full_name: full_name.to_string(),
full_name: qualified_name.to_string(),
},
binding.range,
)
@ -339,14 +282,14 @@ pub(crate) fn typing_only_runtime_import(
ImportSection::Known(ImportType::ThirdParty) | ImportSection::UserDefined(_) => {
Diagnostic::new(
TypingOnlyThirdPartyImport {
full_name: full_name.to_string(),
full_name: qualified_name.to_string(),
},
binding.range,
)
}
ImportSection::Known(ImportType::StandardLibrary) => Diagnostic::new(
TypingOnlyStandardLibraryImport {
full_name: full_name.to_string(),
full_name: qualified_name.to_string(),
},
binding.range,
),
@ -363,7 +306,7 @@ pub(crate) fn typing_only_runtime_import(
let stmt = checker.semantic_model().stmts[source];
let parent = checker.semantic_model().stmts.parent(stmt);
let remove_import_edit = autofix::edits::remove_unused_imports(
std::iter::once(full_name),
std::iter::once(qualified_name),
stmt,
parent,
checker.locator,
@ -374,7 +317,10 @@ pub(crate) fn typing_only_runtime_import(
// Step 2) Add the import to a `TYPE_CHECKING` block.
let reference = checker.semantic_model().references.resolve(*reference_id);
let add_import_edit = checker.importer.typing_import_edit(
&StmtImport { stmt, full_name },
&StmtImport {
stmt,
full_name: qualified_name,
},
reference.range().start(),
checker.semantic_model(),
)?;

View file

@ -3,7 +3,7 @@ source: crates/ruff/src/rules/flake8_type_checking/mod.rs
---
strict.py:27:21: TCH002 [*] Move third-party import `pkg.A` into a type-checking block
|
27 | # In un-strict mode, this shouldn't rase an error, since `pkg` is used at runtime.
27 | # In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
28 | import pkg
29 | from pkg import A
| ^ TCH002
@ -23,7 +23,7 @@ strict.py:27:21: TCH002 [*] Move third-party import `pkg.A` into a type-checking
4 8 | def f():
--------------------------------------------------------------------------------
24 28 | def f():
25 29 | # In un-strict mode, this shouldn't rase an error, since `pkg` is used at runtime.
25 29 | # In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
26 30 | import pkg
27 |- from pkg import A
28 31 |
@ -33,7 +33,7 @@ strict.py:27:21: TCH002 [*] Move third-party import `pkg.A` into a type-checking
strict.py:35:21: TCH002 [*] Move third-party import `pkg.A` into a type-checking block
|
35 | def f():
36 | # In un-strict mode, this shouldn't rase an error, since `pkg` is used at runtime.
36 | # In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
37 | from pkg import A, B
| ^ TCH002
38 |
@ -53,7 +53,7 @@ strict.py:35:21: TCH002 [*] Move third-party import `pkg.A` into a type-checking
--------------------------------------------------------------------------------
32 36 |
33 37 | def f():
34 38 | # In un-strict mode, this shouldn't rase an error, since `pkg` is used at runtime.
34 38 | # In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
35 |- from pkg import A, B
39 |+ from pkg import B
36 40 |
@ -62,7 +62,7 @@ strict.py:35:21: TCH002 [*] Move third-party import `pkg.A` into a type-checking
strict.py:54:25: TCH002 [*] Move third-party import `pkg.bar.A` into a type-checking block
|
54 | # In un-strict mode, this _should_ rase an error, since `pkg` is used at runtime.
54 | # In un-strict mode, this _should_ raise an error, since `pkg.bar` isn't used at runtime
55 | import pkg
56 | from pkg.bar import A
| ^ TCH002
@ -82,7 +82,7 @@ strict.py:54:25: TCH002 [*] Move third-party import `pkg.bar.A` into a type-chec
4 8 | def f():
--------------------------------------------------------------------------------
51 55 | def f():
52 56 | # In un-strict mode, this _should_ rase an error, since `pkg` is used at runtime.
52 56 | # In un-strict mode, this _should_ raise an error, since `pkg.bar` isn't used at runtime
53 57 | import pkg
54 |- from pkg.bar import A
55 58 |
@ -92,7 +92,7 @@ strict.py:54:25: TCH002 [*] Move third-party import `pkg.bar.A` into a type-chec
strict.py:62:12: TCH002 [*] Move third-party import `pkg` into a type-checking block
|
62 | def f():
63 | # In un-strict mode, this shouldn't rase an error, since `pkg.bar` is used at runtime.
63 | # In un-strict mode, this shouldn't raise an error, since `pkg.bar` is used at runtime.
64 | import pkg
| ^^^ TCH002
65 | import pkg.bar as B
@ -111,7 +111,7 @@ strict.py:62:12: TCH002 [*] Move third-party import `pkg` into a type-checking b
--------------------------------------------------------------------------------
59 63 |
60 64 | def f():
61 65 | # In un-strict mode, this shouldn't rase an error, since `pkg.bar` is used at runtime.
61 65 | # In un-strict mode, this shouldn't raise an error, since `pkg.bar` is used at runtime.
62 |- import pkg
63 66 | import pkg.bar as B
64 67 |
@ -120,7 +120,7 @@ strict.py:62:12: TCH002 [*] Move third-party import `pkg` into a type-checking b
strict.py:71:12: TCH002 [*] Move third-party import `pkg.foo` into a type-checking block
|
71 | def f():
72 | # In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime.
72 | # In un-strict mode, this shouldn't raise an error, since `pkg.foo.bar` is used at runtime.
73 | import pkg.foo as F
| ^^^^^^^^^^^^ TCH002
74 | import pkg.foo.bar as B
@ -139,7 +139,7 @@ strict.py:71:12: TCH002 [*] Move third-party import `pkg.foo` into a type-checki
--------------------------------------------------------------------------------
68 72 |
69 73 | def f():
70 74 | # In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime.
70 74 | # In un-strict mode, this shouldn't raise an error, since `pkg.foo.bar` is used at runtime.
71 |- import pkg.foo as F
72 75 | import pkg.foo.bar as B
73 76 |
@ -148,7 +148,7 @@ strict.py:71:12: TCH002 [*] Move third-party import `pkg.foo` into a type-checki
strict.py:80:12: TCH002 [*] Move third-party import `pkg` into a type-checking block
|
80 | def f():
81 | # In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime.
81 | # In un-strict mode, this shouldn't raise an error, since `pkg.foo.bar` is used at runtime.
82 | import pkg
| ^^^ TCH002
83 | import pkg.foo.bar as B
@ -167,7 +167,7 @@ strict.py:80:12: TCH002 [*] Move third-party import `pkg` into a type-checking b
--------------------------------------------------------------------------------
77 81 |
78 82 | def f():
79 83 | # In un-strict mode, this shouldn't rase an error, since `pkg.foo.bar` is used at runtime.
79 83 | # In un-strict mode, this shouldn't raise an error, since `pkg.foo.bar` is used at runtime.
80 |- import pkg
81 84 | import pkg.foo.bar as B
82 85 |
@ -193,7 +193,7 @@ strict.py:91:12: TCH002 [*] Move third-party import `pkg` into a type-checking b
3 7 |
4 8 | def f():
--------------------------------------------------------------------------------
88 92 | # In un-strict mode, this _should_ rase an error, since `pkgfoo.bar` is used at runtime.
88 92 | # In un-strict mode, this _should_ raise an error, since `pkg` isn't used at runtime.
89 93 | # Note that `pkg` is a prefix of `pkgfoo` which are both different modules. This is
90 94 | # testing the implementation.
91 |- import pkg
@ -203,7 +203,7 @@ strict.py:91:12: TCH002 [*] Move third-party import `pkg` into a type-checking b
strict.py:101:12: TCH002 [*] Move third-party import `pkg.foo` into a type-checking block
|
101 | # In un-strict mode, this shouldn't raise an error, since `pkg.bar` is used at runtime.
101 | # In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
102 | import pkg.bar as B
103 | import pkg.foo as F
| ^^^^^^^^^^^^ TCH002
@ -223,7 +223,7 @@ strict.py:101:12: TCH002 [*] Move third-party import `pkg.foo` into a type-check
4 8 | def f():
--------------------------------------------------------------------------------
98 102 | def f():
99 103 | # In un-strict mode, this shouldn't raise an error, since `pkg.bar` is used at runtime.
99 103 | # In un-strict mode, this shouldn't raise an error, since `pkg` is used at runtime.
100 104 | import pkg.bar as B
101 |- import pkg.foo as F
102 105 |

View file

@ -3,7 +3,7 @@ source: crates/ruff/src/rules/flake8_type_checking/mod.rs
---
strict.py:54:25: TCH002 [*] Move third-party import `pkg.bar.A` into a type-checking block
|
54 | # In un-strict mode, this _should_ rase an error, since `pkg` is used at runtime.
54 | # In un-strict mode, this _should_ raise an error, since `pkg.bar` isn't used at runtime
55 | import pkg
56 | from pkg.bar import A
| ^ TCH002
@ -23,7 +23,7 @@ strict.py:54:25: TCH002 [*] Move third-party import `pkg.bar.A` into a type-chec
4 8 | def f():
--------------------------------------------------------------------------------
51 55 | def f():
52 56 | # In un-strict mode, this _should_ rase an error, since `pkg` is used at runtime.
52 56 | # In un-strict mode, this _should_ raise an error, since `pkg.bar` isn't used at runtime
53 57 | import pkg
54 |- from pkg.bar import A
55 58 |
@ -50,7 +50,7 @@ strict.py:91:12: TCH002 [*] Move third-party import `pkg` into a type-checking b
3 7 |
4 8 | def f():
--------------------------------------------------------------------------------
88 92 | # In un-strict mode, this _should_ rase an error, since `pkgfoo.bar` is used at runtime.
88 92 | # In un-strict mode, this _should_ raise an error, since `pkg` isn't used at runtime.
89 93 | # Note that `pkg` is a prefix of `pkgfoo` which are both different modules. This is
90 94 | # testing the implementation.
91 |- import pkg

View file

@ -5,9 +5,7 @@ use rustpython_parser::ast::Ranged;
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, IsolationLevel, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::binding::{
BindingKind, Exceptions, FromImportation, Importation, SubmoduleImportation,
};
use ruff_python_semantic::binding::Exceptions;
use ruff_python_semantic::node::NodeId;
use ruff_python_semantic::scope::Scope;
@ -117,11 +115,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
continue;
}
let full_name = match &binding.kind {
BindingKind::Importation(Importation { full_name }) => full_name,
BindingKind::FromImportation(FromImportation { full_name }) => full_name.as_str(),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => full_name,
_ => continue,
let Some(qualified_name) = binding.qualified_name() else {
continue;
};
let stmt_id = binding.source.unwrap();
@ -144,12 +139,12 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
ignored
.entry((stmt_id, parent_id, exceptions))
.or_default()
.push((full_name, &binding.range));
.push((qualified_name, &binding.range));
} else {
unused
.entry((stmt_id, parent_id, exceptions))
.or_default()
.push((full_name, &binding.range));
.push((qualified_name, &binding.range));
}
}

View file

@ -101,6 +101,35 @@ impl<'a> Binding<'a> {
)
}
/// Returns the fully-qualified symbol name, if this symbol was imported from another module.
pub fn qualified_name(&self) -> Option<&str> {
match &self.kind {
BindingKind::Importation(Importation { full_name }) => Some(full_name),
BindingKind::FromImportation(FromImportation { full_name }) => Some(full_name),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => {
Some(full_name)
}
_ => None,
}
}
/// Returns the fully-qualified name of the module from which this symbol was imported, if this
/// symbol was imported from another module.
pub fn module_name(&self) -> Option<&str> {
match &self.kind {
BindingKind::Importation(Importation { full_name })
| BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => {
Some(full_name.split('.').next().unwrap_or(full_name))
}
BindingKind::FromImportation(FromImportation { full_name }) => Some(
full_name
.rsplit_once('.')
.map_or(full_name, |(module, _)| module),
),
_ => None,
}
}
/// Returns the appropriate visual range for highlighting this binding.
pub fn trimmed_range(&self, semantic_model: &SemanticModel, locator: &Locator) -> TextRange {
match self.kind {

View file

@ -256,14 +256,8 @@ impl<'a> SemanticModel<'a> {
// import pyarrow.csv
// print(pa.csv.read_csv("test.csv"))
// ```
let full_name = match &self.bindings[binding_id].kind {
BindingKind::Importation(Importation { full_name }) => *full_name,
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name }) => *full_name,
BindingKind::FromImportation(FromImportation { full_name }) => full_name.as_str(),
_ => return None,
};
let has_alias = full_name
let qualified_name = self.bindings[binding_id].qualified_name()?;
let has_alias = qualified_name
.split('.')
.last()
.map(|segment| segment != symbol)
@ -272,7 +266,7 @@ impl<'a> SemanticModel<'a> {
return None;
}
self.scopes[scope_id].get(full_name)
self.scopes[scope_id].get(qualified_name)
}
/// Resolves the [`Expr`] to a fully-qualified symbol-name, if `value` resolves to an imported