Skip to content

Commit

Permalink
Add dedicated structs for BindingKind variants (#3672)
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh authored Mar 22, 2023
1 parent 615887a commit 189c9d4
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 71 deletions.
57 changes: 37 additions & 20 deletions crates/ruff/src/checkers/ast/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -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)))
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
});

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
};

Expand Down
10 changes: 6 additions & 4 deletions crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs
View file
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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,
};

Expand Down
52 changes: 39 additions & 13 deletions crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
};

Expand Down
7 changes: 5 additions & 2 deletions crates/ruff/src/rules/pandas_vet/rules/check_call.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!(
Expand Down
20 changes: 12 additions & 8 deletions crates/ruff/src/rules/pylint/rules/sys_exit_alias.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -73,16 +77,16 @@ 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
}
}
// 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 {
Expand Down
13 changes: 9 additions & 4 deletions crates/ruff_python_ast/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_python_ast/src/operations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}
Expand Down
Loading

0 comments on commit 189c9d4

Please sign in to comment.