Add dedicated structs for BindingKind variants (#3672)

This commit is contained in:
Charlie Marsh 2023-03-22 19:08:48 -04:00 committed by GitHub
parent 615887a7fe
commit 189c9d4683
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 195 additions and 71 deletions

View file

@ -17,8 +17,9 @@ use ruff_python_ast::context::Context;
use ruff_python_ast::helpers::{binding_range, extract_handled_exceptions, to_module_path};
use ruff_python_ast::operations::{extract_all_names, AllNamesFlags};
use ruff_python_ast::scope::{
Binding, BindingId, BindingKind, ClassDef, Exceptions, ExecutionContext, FunctionDef, Lambda,
Scope, ScopeId, ScopeKind, ScopeStack,
Binding, BindingId, BindingKind, ClassDef, Exceptions, ExecutionContext, Export,
FromImportation, FunctionDef, Importation, Lambda, Scope, ScopeId, ScopeKind, ScopeStack,
StarImportation, SubmoduleImportation,
};
use ruff_python_ast::source_code::{Indexer, Locator, Stylist};
use ruff_python_ast::types::{Node, Range, RefEquality};
@ -870,7 +871,10 @@ where
self.add_binding(
name,
Binding {
kind: BindingKind::SubmoduleImportation(name, full_name),
kind: BindingKind::SubmoduleImportation(SubmoduleImportation {
name,
full_name,
}),
runtime_usage: None,
synthetic_usage: None,
typing_usage: None,
@ -889,15 +893,12 @@ where
.as_ref()
.map_or(false, |asname| asname == &alias.node.name);
// Given `import foo`, `name` and `full_name` would both be `foo`.
// Given `import foo as bar`, `name` would be `bar` and `full_name` would
// be `foo`.
let name = alias.node.asname.as_ref().unwrap_or(&alias.node.name);
let full_name = &alias.node.name;
self.add_binding(
name,
Binding {
kind: BindingKind::Importation(name, full_name),
kind: BindingKind::Importation(Importation { name, full_name }),
runtime_usage: None,
synthetic_usage: if is_explicit_reexport {
Some((self.ctx.scope_id(), Range::from(alias)))
@ -1191,7 +1192,10 @@ where
self.add_binding(
"*",
Binding {
kind: BindingKind::StarImportation(*level, module.clone()),
kind: BindingKind::StarImportation(StarImportation {
level: *level,
module: module.clone(),
}),
runtime_usage: None,
synthetic_usage: None,
typing_usage: None,
@ -1264,7 +1268,10 @@ where
self.add_binding(
name,
Binding {
kind: BindingKind::FromImportation(name, full_name),
kind: BindingKind::FromImportation(FromImportation {
name,
full_name,
}),
runtime_usage: None,
synthetic_usage: if is_explicit_reexport {
Some((self.ctx.scope_id(), Range::from(alias)))
@ -4163,8 +4170,9 @@ impl<'a> Checker<'a> {
// import pyarrow.csv
// print(pa.csv.read_csv("test.csv"))
match &self.ctx.bindings[*index].kind {
BindingKind::Importation(name, full_name)
| BindingKind::SubmoduleImportation(name, full_name) => {
BindingKind::Importation(Importation { name, full_name })
| BindingKind::SubmoduleImportation(SubmoduleImportation { name, full_name }) =>
{
let has_alias = full_name
.split('.')
.last()
@ -4181,7 +4189,7 @@ impl<'a> Checker<'a> {
}
}
}
BindingKind::FromImportation(name, full_name) => {
BindingKind::FromImportation(FromImportation { name, full_name }) => {
let has_alias = full_name
.split('.')
.last()
@ -4219,7 +4227,9 @@ impl<'a> Checker<'a> {
for scope_index in self.ctx.scope_stack.iter() {
let scope = &self.ctx.scopes[*scope_index];
for binding in scope.binding_ids().map(|index| &self.ctx.bindings[*index]) {
if let BindingKind::StarImportation(level, module) = &binding.kind {
if let BindingKind::StarImportation(StarImportation { level, module }) =
&binding.kind
{
from_list.push(helpers::format_import_from(
level.as_ref(),
module.as_deref(),
@ -4437,7 +4447,7 @@ impl<'a> Checker<'a> {
self.add_binding(
id,
Binding {
kind: BindingKind::Export(all_names),
kind: BindingKind::Export(Export { names: all_names }),
runtime_usage: None,
synthetic_usage: None,
typing_usage: None,
@ -4700,7 +4710,7 @@ impl<'a> Checker<'a> {
.get("__all__")
.map(|index| &self.ctx.bindings[*index])
.and_then(|binding| match &binding.kind {
BindingKind::Export(names) => Some((names, binding.range)),
BindingKind::Export(Export { names }) => Some((names, binding.range)),
_ => None,
});
@ -4732,7 +4742,7 @@ impl<'a> Checker<'a> {
.get("__all__")
.map(|index| &self.ctx.bindings[*index])
.and_then(|binding| match &binding.kind {
BindingKind::Export(names) => {
BindingKind::Export(Export { names }) => {
Some((names.iter().map(String::as_str).collect(), binding.range))
}
_ => None,
@ -4854,7 +4864,9 @@ impl<'a> Checker<'a> {
if let Some((names, range)) = &all_names {
let mut from_list = vec![];
for binding in scope.binding_ids().map(|index| &self.ctx.bindings[*index]) {
if let BindingKind::StarImportation(level, module) = &binding.kind {
if let BindingKind::StarImportation(StarImportation { level, module }) =
&binding.kind
{
from_list.push(helpers::format_import_from(
level.as_ref(),
module.as_deref(),
@ -4933,9 +4945,14 @@ impl<'a> Checker<'a> {
let binding = &self.ctx.bindings[*index];
let full_name = match &binding.kind {
BindingKind::Importation(.., full_name) => full_name,
BindingKind::FromImportation(.., full_name) => full_name.as_str(),
BindingKind::SubmoduleImportation(.., full_name) => full_name,
BindingKind::Importation(Importation { full_name, .. }) => full_name,
BindingKind::FromImportation(FromImportation { full_name, .. }) => {
full_name.as_str()
}
BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name,
..
}) => full_name,
_ => continue,
};

View file

@ -1,6 +1,8 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::scope::{Binding, BindingKind, ExecutionContext};
use ruff_python_ast::scope::{
Binding, BindingKind, ExecutionContext, FromImportation, Importation, SubmoduleImportation,
};
#[violation]
pub struct RuntimeImportInTypeCheckingBlock {
@ -21,9 +23,9 @@ impl Violation for RuntimeImportInTypeCheckingBlock {
/// TCH004
pub fn runtime_import_in_type_checking_block(binding: &Binding) -> Option<Diagnostic> {
let full_name = match &binding.kind {
BindingKind::Importation(.., full_name) => full_name,
BindingKind::FromImportation(.., full_name) => full_name.as_str(),
BindingKind::SubmoduleImportation(.., full_name) => full_name,
BindingKind::Importation(Importation { full_name, .. }) => full_name,
BindingKind::FromImportation(FromImportation { full_name, .. }) => full_name.as_str(),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name, .. }) => full_name,
_ => return None,
};

View file

@ -2,7 +2,9 @@ use std::path::Path;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::scope::{Binding, BindingKind, ExecutionContext};
use ruff_python_ast::scope::{
Binding, BindingKind, ExecutionContext, FromImportation, Importation, SubmoduleImportation,
};
use crate::rules::isort::{categorize, ImportType};
use crate::settings::Settings;
@ -55,30 +57,54 @@ 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(.., this_name)
| BindingKind::SubmoduleImportation(this_name, ..) => match &that.kind {
BindingKind::FromImportation(.., that_name) => {
BindingKind::Importation(Importation {
full_name: this_name,
..
})
| BindingKind::SubmoduleImportation(SubmoduleImportation {
name: this_name, ..
}) => match &that.kind {
BindingKind::FromImportation(FromImportation {
full_name: that_name,
..
}) => {
// Ex) `pkg.A` vs. `pkg`
this_name
.rfind('.')
.map_or(false, |i| this_name[..i] == *that_name)
}
BindingKind::Importation(.., that_name)
| BindingKind::SubmoduleImportation(that_name, ..) => {
BindingKind::Importation(Importation {
full_name: that_name,
..
})
| BindingKind::SubmoduleImportation(SubmoduleImportation {
name: that_name, ..
}) => {
// Ex) `pkg.A` vs. `pkg.B`
this_name == that_name
}
_ => false,
},
BindingKind::FromImportation(.., this_name) => match &that.kind {
BindingKind::Importation(.., that_name)
| BindingKind::SubmoduleImportation(that_name, ..) => {
BindingKind::FromImportation(FromImportation {
full_name: this_name,
..
}) => match &that.kind {
BindingKind::Importation(Importation {
full_name: that_name,
..
})
| BindingKind::SubmoduleImportation(SubmoduleImportation {
name: that_name, ..
}) => {
// Ex) `pkg.A` vs. `pkg`
this_name
.rfind('.')
.map_or(false, |i| &this_name[..i] == *that_name)
}
BindingKind::FromImportation(.., that_name) => {
BindingKind::FromImportation(FromImportation {
full_name: that_name,
..
}) => {
// Ex) `pkg.A` vs. `pkg.B`
this_name.rfind('.').map_or(false, |i| {
that_name
@ -126,9 +152,9 @@ pub fn typing_only_runtime_import(
}
let full_name = match &binding.kind {
BindingKind::Importation(.., full_name) => full_name,
BindingKind::FromImportation(.., full_name) => full_name.as_str(),
BindingKind::SubmoduleImportation(.., full_name) => full_name,
BindingKind::Importation(Importation { full_name, .. }) => full_name,
BindingKind::FromImportation(FromImportation { full_name, .. }) => full_name.as_str(),
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name, .. }) => full_name,
_ => return None,
};

View file

@ -3,7 +3,7 @@ use rustpython_parser::ast::{Expr, ExprKind};
use ruff_diagnostics::Violation;
use ruff_diagnostics::{Diagnostic, DiagnosticKind};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::scope::BindingKind;
use ruff_python_ast::scope::{BindingKind, Importation};
use ruff_python_ast::types::Range;
use crate::checkers::ast::Checker;
@ -86,7 +86,10 @@ pub fn check_call(checker: &mut Checker, func: &Expr) {
// irrelevant bindings (like non-Pandas imports).
if let ExprKind::Name { id, .. } = &value.node {
if checker.ctx.find_binding(id).map_or(true, |binding| {
if let BindingKind::Importation(.., module) = &binding.kind {
if let BindingKind::Importation(Importation {
full_name: module, ..
}) = &binding.kind
{
module != &"pandas"
} else {
matches!(

View file

@ -2,7 +2,9 @@ use rustpython_parser::ast::{Expr, ExprKind};
use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::scope::BindingKind;
use ruff_python_ast::scope::{
BindingKind, FromImportation, Importation, StarImportation, SubmoduleImportation,
};
use ruff_python_ast::types::Range;
use crate::checkers::ast::Checker;
@ -31,8 +33,10 @@ impl Violation for SysExitAlias {
fn is_module_star_imported(checker: &Checker, module: &str) -> bool {
checker.ctx.scopes().any(|scope| {
scope.binding_ids().any(|index| {
if let BindingKind::StarImportation(_, name) = &checker.ctx.bindings[*index].kind {
name.as_ref().map(|name| name == module).unwrap_or_default()
if let BindingKind::StarImportation(StarImportation { module: name, .. }) =
&checker.ctx.bindings[*index].kind
{
name.as_ref().map_or(false, |name| name == module)
} else {
false
}
@ -50,7 +54,7 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) -
// e.g. module=sys object=exit
// `import sys` -> `sys.exit`
// `import sys as sys2` -> `sys2.exit`
BindingKind::Importation(name, full_name) => {
BindingKind::Importation(Importation { name, full_name }) => {
if full_name == &module {
Some(format!("{name}.{member}"))
} else {
@ -60,7 +64,7 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) -
// e.g. module=os.path object=join
// `from os.path import join` -> `join`
// `from os.path import join as join2` -> `join2`
BindingKind::FromImportation(name, full_name) => {
BindingKind::FromImportation(FromImportation { name, full_name }) => {
let mut parts = full_name.split('.');
if parts.next() == Some(module)
&& parts.next() == Some(member)
@ -73,8 +77,8 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) -
}
// e.g. module=os.path object=join
// `from os.path import *` -> `join`
BindingKind::StarImportation(_, name) => {
if name.as_ref().map(|name| name == module).unwrap_or_default() {
BindingKind::StarImportation(StarImportation { module: name, .. }) => {
if name.as_ref().map_or(false, |name| name == module) {
Some(member.to_string())
} else {
None
@ -82,7 +86,7 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) -
}
// e.g. module=os.path object=join
// `import os.path ` -> `os.path.join`
BindingKind::SubmoduleImportation(_, full_name) => {
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name, .. }) => {
if full_name == &module {
Some(format!("{full_name}.{member}"))
} else {

View file

@ -10,8 +10,8 @@ use ruff_python_stdlib::typing::TYPING_EXTENSIONS;
use crate::helpers::{collect_call_path, from_relative_import};
use crate::scope::{
Binding, BindingId, BindingKind, Bindings, Exceptions, ExecutionContext, Scope, ScopeId,
ScopeKind, ScopeStack, Scopes,
Binding, BindingId, BindingKind, Bindings, Exceptions, ExecutionContext, FromImportation,
Importation, Scope, ScopeId, ScopeKind, ScopeStack, Scopes, SubmoduleImportation,
};
use crate::types::{CallPath, RefEquality};
use crate::typing::AnnotationKind;
@ -156,7 +156,10 @@ impl<'a> Context<'a> {
return None;
};
match &binding.kind {
BindingKind::Importation(.., name) | BindingKind::SubmoduleImportation(name, ..) => {
BindingKind::Importation(Importation {
full_name: name, ..
})
| BindingKind::SubmoduleImportation(SubmoduleImportation { name, .. }) => {
if name.starts_with('.') {
if let Some(module) = &self.module_path {
let mut source_path = from_relative_import(module, name);
@ -171,7 +174,9 @@ impl<'a> Context<'a> {
Some(source_path)
}
}
BindingKind::FromImportation(.., name) => {
BindingKind::FromImportation(FromImportation {
full_name: name, ..
}) => {
if name.starts_with('.') {
if let Some(module) = &self.module_path {
let mut source_path = from_relative_import(module, name);

View file

@ -5,7 +5,7 @@ use rustpython_parser::{lexer, Mode, Tok};
use crate::context::Context;
use crate::helpers::any_over_expr;
use crate::scope::{BindingKind, Scope};
use crate::scope::{BindingKind, Export, Scope};
use crate::visitor;
use crate::visitor::Visitor;
@ -96,7 +96,7 @@ pub fn extract_all_names(
// Grab the existing bound __all__ values.
if let StmtKind::AugAssign { .. } = &stmt.node {
if let Some(index) = scope.get("__all__") {
if let BindingKind::Export(existing) = &ctx.bindings[*index].kind {
if let BindingKind::Export(Export { names: existing }) = &ctx.bindings[*index].kind {
names.extend_from_slice(existing);
}
}

View file

@ -284,26 +284,45 @@ impl<'a> Binding<'a> {
pub fn redefines(&self, existing: &'a Binding) -> bool {
match &self.kind {
BindingKind::Importation(.., full_name) => {
if let BindingKind::SubmoduleImportation(.., existing) = &existing.kind {
BindingKind::Importation(Importation { full_name, .. }) => {
if let BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: existing,
..
}) = &existing.kind
{
return full_name == existing;
}
}
BindingKind::FromImportation(.., full_name) => {
if let BindingKind::SubmoduleImportation(.., existing) = &existing.kind {
BindingKind::FromImportation(FromImportation { full_name, .. }) => {
if let BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: existing,
..
}) = &existing.kind
{
return full_name == existing;
}
}
BindingKind::SubmoduleImportation(.., full_name) => match &existing.kind {
BindingKind::Importation(.., existing)
| BindingKind::SubmoduleImportation(.., existing) => {
BindingKind::SubmoduleImportation(SubmoduleImportation { full_name, .. }) => {
match &existing.kind {
BindingKind::Importation(Importation {
full_name: existing,
..
})
| BindingKind::SubmoduleImportation(SubmoduleImportation {
full_name: existing,
..
}) => {
return full_name == existing;
}
BindingKind::FromImportation(.., existing) => {
BindingKind::FromImportation(FromImportation {
full_name: existing,
..
}) => {
return full_name == existing;
}
_ => {}
},
}
}
BindingKind::Annotation => {
return false;
}
@ -366,6 +385,54 @@ impl nohash_hasher::IsEnabled for BindingId {}
// StarImportation
// FutureImportation
#[derive(Clone, Debug)]
pub struct Export {
/// The names of the bindings exported via `__all__`.
pub names: Vec<String>,
}
#[derive(Clone, Debug)]
pub struct StarImportation {
/// The level of the import. `None` or `Some(0)` indicate an absolute import.
pub level: Option<usize>,
/// The module being imported. `None` indicates a wildcard import.
pub module: Option<String>,
}
#[derive(Clone, Debug)]
pub struct Importation<'a> {
/// The name to which the import is bound.
/// Given `import foo`, `name` would be "foo".
/// Given `import foo as bar`, `name` would be "bar".
pub name: &'a str,
/// The full name of the module being imported.
/// Given `import foo`, `full_name` would be "foo".
/// Given `import foo as bar`, `full_name` would be "foo".
pub full_name: &'a str,
}
#[derive(Clone, Debug)]
pub struct FromImportation<'a> {
/// The name to which the import is bound.
/// Given `from foo import bar`, `name` would be "bar".
/// Given `from foo import bar as baz`, `name` would be "baz".
pub name: &'a str,
/// The full name of the module being imported.
/// Given `from foo import bar`, `full_name` would be "foo.bar".
/// Given `from foo import bar as baz`, `full_name` would be "foo.bar".
pub full_name: String,
}
#[derive(Clone, Debug)]
pub struct SubmoduleImportation<'a> {
/// The parent module imported by the submodule import.
/// Given `import foo.bar`, `module` would be "foo".
pub name: &'a str,
/// The full name of the submodule being imported.
/// Given `import foo.bar`, `full_name` would be "foo.bar".
pub full_name: &'a str,
}
#[derive(Clone, Debug, is_macro::Is)]
pub enum BindingKind<'a> {
Annotation,
@ -378,12 +445,12 @@ pub enum BindingKind<'a> {
Builtin,
ClassDefinition,
FunctionDefinition,
Export(Vec<String>),
Export(Export),
FutureImportation,
StarImportation(Option<usize>, Option<String>),
Importation(&'a str, &'a str),
FromImportation(&'a str, String),
SubmoduleImportation(&'a str, &'a str),
StarImportation(StarImportation),
Importation(Importation<'a>),
FromImportation(FromImportation<'a>),
SubmoduleImportation(SubmoduleImportation<'a>),
}
/// The bindings in a program.