diff --git a/crates/ruff/src/rules/flake8_boolean_trap/rules.rs b/crates/ruff/src/rules/flake8_boolean_trap/rules.rs index 671a5a0be6..bafebf2142 100644 --- a/crates/ruff/src/rules/flake8_boolean_trap/rules.rs +++ b/crates/ruff/src/rules/flake8_boolean_trap/rules.rs @@ -107,8 +107,7 @@ pub fn check_positional_boolean_in_def( } if decorator_list.iter().any(|expr| { - let call_path = collect_call_path(expr); - call_path.as_slice() == [name, "setter"] + collect_call_path(expr).map_or(false, |call_path| call_path.as_slice() == [name, "setter"]) }) { return; } @@ -151,8 +150,7 @@ pub fn check_boolean_default_value_in_function_definition( } if decorator_list.iter().any(|expr| { - let call_path = collect_call_path(expr); - call_path.as_slice() == [name, "setter"] + collect_call_path(expr).map_or(false, |call_path| call_path.as_slice() == [name, "setter"]) }) { return; } diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs b/crates/ruff/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs index 3ec998b2fe..55c054a688 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs @@ -70,8 +70,7 @@ fn duplicate_handler_exceptions<'a>( let mut duplicates: FxHashSet = FxHashSet::default(); let mut unique_elts: Vec<&Expr> = Vec::default(); for type_ in elts { - let call_path = call_path::collect_call_path(type_); - if !call_path.is_empty() { + if let Some(call_path) = call_path::collect_call_path(type_) { if seen.contains_key(&call_path) { duplicates.insert(call_path); } else { @@ -125,8 +124,7 @@ pub fn duplicate_exceptions(checker: &mut Checker, handlers: &[Excepthandler]) { }; match &type_.node { ExprKind::Attribute { .. } | ExprKind::Name { .. } => { - let call_path = call_path::collect_call_path(type_); - if !call_path.is_empty() { + if let Some(call_path) = call_path::collect_call_path(type_) { if seen.contains(&call_path) { duplicates.entry(call_path).or_default().push(type_); } else { diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs b/crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs index b3d40e6bc9..f9a39150cf 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs @@ -3,7 +3,7 @@ use rustpython_parser::ast::{Arguments, Constant, Expr, ExprKind}; use ruff_diagnostics::Violation; use ruff_diagnostics::{Diagnostic, DiagnosticKind}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::call_path::to_call_path; +use ruff_python_ast::call_path::from_qualified_name; use ruff_python_ast::call_path::{compose_call_path, CallPath}; use ruff_python_ast::types::Range; use ruff_python_ast::visitor; @@ -123,7 +123,7 @@ pub fn function_call_argument_default(checker: &mut Checker, arguments: &Argumen .flake8_bugbear .extend_immutable_calls .iter() - .map(|target| to_call_path(target)) + .map(|target| from_qualified_name(target)) .collect(); let diagnostics = { let mut visitor = ArgumentDefaultVisitor { diff --git a/crates/ruff/src/rules/flake8_debugger/rules.rs b/crates/ruff/src/rules/flake8_debugger/rules.rs index 0818150662..00abd923cb 100644 --- a/crates/ruff/src/rules/flake8_debugger/rules.rs +++ b/crates/ruff/src/rules/flake8_debugger/rules.rs @@ -2,14 +2,11 @@ use rustpython_parser::ast::{Expr, Stmt}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::call_path::format_call_path; +use ruff_python_ast::call_path::{format_call_path, from_unqualified_name, CallPath}; use ruff_python_ast::types::Range; use crate::checkers::ast::Checker; - -use super::types::DebuggerUsingType; - -// flake8-debugger +use crate::rules::flake8_debugger::types::DebuggerUsingType; #[violation] pub struct Debugger { @@ -70,9 +67,12 @@ pub fn debugger_import(stmt: &Stmt, module: Option<&str>, name: &str) -> Option< } if let Some(module) = module { - let mut call_path = module.split('.').collect::>(); + let mut call_path: CallPath = from_unqualified_name(module); call_path.push(name); - if DEBUGGERS.iter().any(|target| call_path == **target) { + if DEBUGGERS + .iter() + .any(|target| call_path.as_slice() == *target) + { return Some(Diagnostic::new( Debugger { using_type: DebuggerUsingType::Import(format_call_path(&call_path)), @@ -81,10 +81,10 @@ pub fn debugger_import(stmt: &Stmt, module: Option<&str>, name: &str) -> Option< )); } } else { - let parts = name.split('.').collect::>(); + let parts: CallPath = from_unqualified_name(name); if DEBUGGERS .iter() - .any(|call_path| call_path[..call_path.len() - 1] == parts) + .any(|call_path| &call_path[..call_path.len() - 1] == parts.as_slice()) { return Some(Diagnostic::new( Debugger { diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs index 17f929ff50..138906eec1 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/fixture.rs @@ -16,8 +16,8 @@ use crate::checkers::ast::Checker; use crate::registry::{AsRule, Rule}; use super::helpers::{ - get_mark_decorators, get_mark_name, is_abstractmethod_decorator, is_pytest_fixture, - is_pytest_yield_fixture, keyword_is_literal, + get_mark_decorators, is_abstractmethod_decorator, is_pytest_fixture, is_pytest_yield_fixture, + keyword_is_literal, }; #[violation] @@ -212,7 +212,9 @@ where } } ExprKind::Call { func, .. } => { - if collect_call_path(func).as_slice() == ["request", "addfinalizer"] { + if collect_call_path(func).map_or(false, |call_path| { + call_path.as_slice() == ["request", "addfinalizer"] + }) { self.addfinalizer_call = Some(expr); }; visitor::walk_expr(self, expr); @@ -466,20 +468,19 @@ fn check_fixture_addfinalizer(checker: &mut Checker, args: &Arguments, body: &[S /// PT024, PT025 fn check_fixture_marks(checker: &mut Checker, decorators: &[Expr]) { - for mark in get_mark_decorators(decorators) { - let name = get_mark_name(mark); - + for (expr, call_path) in get_mark_decorators(decorators) { + let name = call_path.last().expect("Expected a mark name"); if checker .settings .rules .enabled(Rule::PytestUnnecessaryAsyncioMarkOnFixture) { - if name == "asyncio" { + if *name == "asyncio" { let mut diagnostic = - Diagnostic::new(PytestUnnecessaryAsyncioMarkOnFixture, Range::from(mark)); + Diagnostic::new(PytestUnnecessaryAsyncioMarkOnFixture, Range::from(expr)); if checker.patch(diagnostic.kind.rule()) { - let start = Location::new(mark.location.row(), 0); - let end = Location::new(mark.end_location.unwrap().row() + 1, 0); + let start = Location::new(expr.location.row(), 0); + let end = Location::new(expr.end_location.unwrap().row() + 1, 0); diagnostic.set_fix(Edit::deletion(start, end)); } checker.diagnostics.push(diagnostic); @@ -491,12 +492,12 @@ fn check_fixture_marks(checker: &mut Checker, decorators: &[Expr]) { .rules .enabled(Rule::PytestErroneousUseFixturesOnFixture) { - if name == "usefixtures" { + if *name == "usefixtures" { let mut diagnostic = - Diagnostic::new(PytestErroneousUseFixturesOnFixture, Range::from(mark)); + Diagnostic::new(PytestErroneousUseFixturesOnFixture, Range::from(expr)); if checker.patch(diagnostic.kind.rule()) { - let start = Location::new(mark.location.row(), 0); - let end = Location::new(mark.end_location.unwrap().row() + 1, 0); + let start = Location::new(expr.location.row(), 0); + let end = Location::new(expr.end_location.unwrap().row() + 1, 0); diagnostic.set_fix(Edit::deletion(start, end)); } checker.diagnostics.push(diagnostic); diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs index 79820f7ae4..e3b83fe3e0 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs @@ -1,5 +1,5 @@ use num_traits::identities::Zero; -use ruff_python_ast::call_path::collect_call_path; +use ruff_python_ast::call_path::{collect_call_path, CallPath}; use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword}; use ruff_python_ast::helpers::map_callable; @@ -8,14 +8,17 @@ use crate::checkers::ast::Checker; const ITERABLE_INITIALIZERS: &[&str] = &["dict", "frozenset", "list", "tuple", "set"]; -pub fn get_mark_decorators(decorators: &[Expr]) -> impl Iterator { - decorators - .iter() - .filter(|decorator| is_pytest_mark(decorator)) -} - -pub fn get_mark_name(decorator: &Expr) -> &str { - collect_call_path(map_callable(decorator)).last().unwrap() +pub fn get_mark_decorators(decorators: &[Expr]) -> impl Iterator { + decorators.iter().filter_map(|decorator| { + let Some(call_path) = collect_call_path(map_callable(decorator)) else { + return None; + }; + if call_path.len() > 2 && call_path.as_slice()[..2] == ["pytest", "mark"] { + Some((decorator, call_path)) + } else { + None + } + }) } pub fn is_pytest_fail(call: &Expr, checker: &Checker) -> bool { @@ -40,15 +43,6 @@ pub fn is_pytest_fixture(decorator: &Expr, checker: &Checker) -> bool { }) } -pub fn is_pytest_mark(decorator: &Expr) -> bool { - let segments = collect_call_path(map_callable(decorator)); - if segments.len() > 2 { - segments[0] == "pytest" && segments[1] == "mark" - } else { - false - } -} - pub fn is_pytest_yield_fixture(decorator: &Expr, checker: &Checker) -> bool { checker .ctx diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/marks.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/marks.rs index 9c07b8b807..acf52fcdc1 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/marks.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/marks.rs @@ -2,12 +2,13 @@ use rustpython_parser::ast::{Expr, ExprKind, Location}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::call_path::CallPath; use ruff_python_ast::types::Range; use crate::checkers::ast::Checker; use crate::registry::{AsRule, Rule}; -use super::helpers::{get_mark_decorators, get_mark_name}; +use super::helpers::get_mark_decorators; #[violation] pub struct PytestIncorrectMarkParenthesesStyle { @@ -52,13 +53,14 @@ impl AlwaysAutofixableViolation for PytestUseFixturesWithoutParameters { fn pytest_mark_parentheses( checker: &mut Checker, decorator: &Expr, + call_path: &CallPath, fix: Edit, preferred: &str, actual: &str, ) { let mut diagnostic = Diagnostic::new( PytestIncorrectMarkParenthesesStyle { - mark_name: get_mark_name(decorator).to_string(), + mark_name: (*call_path.last().unwrap()).to_string(), expected_parens: preferred.to_string(), actual_parens: actual.to_string(), }, @@ -70,7 +72,7 @@ fn pytest_mark_parentheses( checker.diagnostics.push(diagnostic); } -fn check_mark_parentheses(checker: &mut Checker, decorator: &Expr) { +fn check_mark_parentheses(checker: &mut Checker, decorator: &Expr, call_path: &CallPath) { match &decorator.node { ExprKind::Call { func, @@ -84,20 +86,20 @@ fn check_mark_parentheses(checker: &mut Checker, decorator: &Expr) { { let fix = Edit::deletion(func.end_location.unwrap(), decorator.end_location.unwrap()); - pytest_mark_parentheses(checker, decorator, fix, "", "()"); + pytest_mark_parentheses(checker, decorator, call_path, fix, "", "()"); } } _ => { if checker.settings.flake8_pytest_style.mark_parentheses { let fix = Edit::insertion("()".to_string(), decorator.end_location.unwrap()); - pytest_mark_parentheses(checker, decorator, fix, "()", ""); + pytest_mark_parentheses(checker, decorator, call_path, fix, "()", ""); } } } } -fn check_useless_usefixtures(checker: &mut Checker, decorator: &Expr) { - if get_mark_name(decorator) != "usefixtures" { +fn check_useless_usefixtures(checker: &mut Checker, decorator: &Expr, call_path: &CallPath) { + if *call_path.last().unwrap() != "usefixtures" { return; } @@ -130,12 +132,12 @@ pub fn marks(checker: &mut Checker, decorators: &[Expr]) { .rules .enabled(Rule::PytestUseFixturesWithoutParameters); - for mark in get_mark_decorators(decorators) { + for (expr, call_path) in get_mark_decorators(decorators) { if enforce_parentheses { - check_mark_parentheses(checker, mark); + check_mark_parentheses(checker, expr, &call_path); } if enforce_useless_usefixtures { - check_useless_usefixtures(checker, mark); + check_useless_usefixtures(checker, expr, &call_path); } } } diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/raises.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/raises.rs index b03075744e..732795fe5a 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/raises.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/raises.rs @@ -3,7 +3,7 @@ use rustpython_parser::ast::{Expr, ExprKind, Keyword, Stmt, StmtKind, Withitem}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::format_call_path; -use ruff_python_ast::call_path::to_call_path; +use ruff_python_ast::call_path::from_qualified_name; use ruff_python_ast::types::Range; use crate::checkers::ast::Checker; @@ -157,7 +157,7 @@ fn exception_needs_match(checker: &mut Checker, exception: &Expr) { .flake8_pytest_style .raises_extend_require_match_for, ) - .any(|target| call_path == to_call_path(target)); + .any(|target| call_path == from_qualified_name(target)); if is_broad_exception { Some(format_call_path(&call_path)) } else { diff --git a/crates/ruff/src/rules/flake8_self/rules/private_member_access.rs b/crates/ruff/src/rules/flake8_self/rules/private_member_access.rs index 1b600dcaa5..f7616fdc8f 100644 --- a/crates/ruff/src/rules/flake8_self/rules/private_member_access.rs +++ b/crates/ruff/src/rules/flake8_self/rules/private_member_access.rs @@ -73,46 +73,48 @@ pub fn private_member_access(checker: &mut Checker, expr: &Expr) { if let ExprKind::Call { func, .. } = &value.node { // Ignore `super()` calls. - let call_path = collect_call_path(func); - if call_path.as_slice() == ["super"] { - return; + if let Some(call_path) = collect_call_path(func) { + if call_path.as_slice() == ["super"] { + return; + } } } else { // Ignore `self` and `cls` accesses. - let call_path = collect_call_path(value); - if call_path.as_slice() == ["self"] - || call_path.as_slice() == ["cls"] - || call_path.as_slice() == ["mcs"] - { - return; - } + if let Some(call_path) = collect_call_path(value) { + if call_path.as_slice() == ["self"] + || call_path.as_slice() == ["cls"] + || call_path.as_slice() == ["mcs"] + { + return; + } - // Ignore accesses on class members from _within_ the class. - if checker - .ctx - .scopes - .iter() - .rev() - .find_map(|scope| match &scope.kind { - ScopeKind::Class(class_def) => Some(class_def), - _ => None, - }) - .map_or(false, |class_def| { - if call_path.as_slice() == [class_def.name] { - checker - .ctx - .find_binding(class_def.name) - .map_or(false, |binding| { - // TODO(charlie): Could the name ever be bound to a _different_ - // class here? - binding.kind.is_class_definition() - }) - } else { - false - } - }) - { - return; + // Ignore accesses on class members from _within_ the class. + if checker + .ctx + .scopes + .iter() + .rev() + .find_map(|scope| match &scope.kind { + ScopeKind::Class(class_def) => Some(class_def), + _ => None, + }) + .map_or(false, |class_def| { + if call_path.as_slice() == [class_def.name] { + checker + .ctx + .find_binding(class_def.name) + .map_or(false, |binding| { + // TODO(charlie): Could the name ever be bound to a + // _different_ class here? + binding.kind.is_class_definition() + }) + } else { + false + } + }) + { + return; + } } } diff --git a/crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs b/crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs index 69348ad8ed..c6cc5389bb 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs +++ b/crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation, CacheKey}; -use ruff_python_ast::call_path::CallPath; +use ruff_python_ast::call_path::from_qualified_name; use ruff_python_ast::types::Range; use crate::checkers::ast::Checker; @@ -103,7 +103,7 @@ pub fn banned_attribute_access(checker: &mut Checker, expr: &Expr) { .flake8_tidy_imports .banned_api .iter() - .find(|(banned_path, ..)| call_path == banned_path.split('.').collect::()) + .find(|(banned_path, ..)| call_path == from_qualified_name(banned_path)) }) { checker.diagnostics.push(Diagnostic::new( BannedApi { diff --git a/crates/ruff/src/rules/flake8_type_checking/helpers.rs b/crates/ruff/src/rules/flake8_type_checking/helpers.rs index 51cc28f305..94d41e9f93 100644 --- a/crates/ruff/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff/src/rules/flake8_type_checking/helpers.rs @@ -2,7 +2,7 @@ use num_traits::Zero; use rustpython_parser::ast::{Constant, Expr, ExprKind}; use ruff_python_ast::binding::{Binding, BindingKind, ExecutionContext}; -use ruff_python_ast::call_path::to_call_path; +use ruff_python_ast::call_path::from_qualified_name; use ruff_python_ast::context::Context; use ruff_python_ast::helpers::map_callable; use ruff_python_ast::scope::ScopeKind; @@ -78,7 +78,7 @@ fn runtime_evaluated_base_class(context: &Context, base_classes: &[String]) -> b if let Some(call_path) = context.resolve_call_path(base) { if base_classes .iter() - .any(|base_class| to_call_path(base_class) == call_path) + .any(|base_class| from_qualified_name(base_class) == call_path) { return true; } @@ -94,7 +94,7 @@ fn runtime_evaluated_decorators(context: &Context, decorators: &[String]) -> boo if let Some(call_path) = context.resolve_call_path(map_callable(decorator)) { if decorators .iter() - .any(|decorator| to_call_path(decorator) == call_path) + .any(|decorator| from_qualified_name(decorator) == call_path) { return true; } diff --git a/crates/ruff/src/rules/pydocstyle/helpers.rs b/crates/ruff/src/rules/pydocstyle/helpers.rs index 5d14b52c90..196fa42f2f 100644 --- a/crates/ruff/src/rules/pydocstyle/helpers.rs +++ b/crates/ruff/src/rules/pydocstyle/helpers.rs @@ -1,4 +1,4 @@ -use ruff_python_ast::call_path::to_call_path; +use ruff_python_ast::call_path::from_qualified_name; use std::collections::BTreeSet; use ruff_python_ast::cast; @@ -53,7 +53,7 @@ pub(crate) fn should_ignore_definition( if let Some(call_path) = checker.ctx.resolve_call_path(map_callable(decorator)) { if ignore_decorators .iter() - .any(|decorator| to_call_path(decorator) == call_path) + .any(|decorator| from_qualified_name(decorator) == call_path) { return true; } diff --git a/crates/ruff/src/rules/pydocstyle/rules/non_imperative_mood.rs b/crates/ruff/src/rules/pydocstyle/rules/non_imperative_mood.rs index e25652f1dc..a869efdb3d 100644 --- a/crates/ruff/src/rules/pydocstyle/rules/non_imperative_mood.rs +++ b/crates/ruff/src/rules/pydocstyle/rules/non_imperative_mood.rs @@ -5,7 +5,7 @@ use once_cell::sync::Lazy; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::call_path::{to_call_path, CallPath}; +use ruff_python_ast::call_path::{from_qualified_name, CallPath}; use ruff_python_ast::cast; use ruff_python_ast::newlines::StrExt; use ruff_python_ast::types::Range; @@ -33,7 +33,7 @@ pub fn non_imperative_mood( let property_decorators = property_decorators .iter() - .map(|decorator| to_call_path(decorator)) + .map(|decorator| from_qualified_name(decorator)) .collect::>(); if is_test(cast::name(parent)) diff --git a/crates/ruff/src/rules/pyupgrade/rules/datetime_utc_alias.rs b/crates/ruff/src/rules/pyupgrade/rules/datetime_utc_alias.rs index 06deb05d7a..86f109de62 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/datetime_utc_alias.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/datetime_utc_alias.rs @@ -39,7 +39,9 @@ pub fn datetime_utc_alias(checker: &mut Checker, expr: &Expr) { call_path.as_slice() == ["datetime", "timezone", "utc"] }) { - let straight_import = collect_call_path(expr).as_slice() == ["datetime", "timezone", "utc"]; + let straight_import = collect_call_path(expr).map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "timezone", "utc"] + }); let mut diagnostic = Diagnostic::new(DatetimeTimezoneUTC { straight_import }, Range::from(expr)); if checker.patch(diagnostic.kind.rule()) { diff --git a/crates/ruff/src/rules/pyupgrade/rules/deprecated_mock_import.rs b/crates/ruff/src/rules/pyupgrade/rules/deprecated_mock_import.rs index a86d69673f..2bf1aaa787 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/deprecated_mock_import.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/deprecated_mock_import.rs @@ -248,7 +248,9 @@ fn format_import_from( /// UP026 pub fn deprecated_mock_attribute(checker: &mut Checker, expr: &Expr) { if let ExprKind::Attribute { value, .. } = &expr.node { - if collect_call_path(value).as_slice() == ["mock", "mock"] { + if collect_call_path(value) + .map_or(false, |call_path| call_path.as_slice() == ["mock", "mock"]) + { let mut diagnostic = Diagnostic::new( DeprecatedMockImport { reference_type: MockReference::Attribute, diff --git a/crates/ruff_python_ast/src/call_path.rs b/crates/ruff_python_ast/src/call_path.rs index 59b4d56c9a..24cbee3e63 100644 --- a/crates/ruff_python_ast/src/call_path.rs +++ b/crates/ruff_python_ast/src/call_path.rs @@ -23,20 +23,14 @@ fn collect_call_path_inner<'a>(expr: &'a Expr, parts: &mut CallPath<'a>) -> bool } /// Convert an `Expr` to its [`CallPath`] segments (like `["typing", "List"]`). -pub fn collect_call_path(expr: &Expr) -> CallPath { +pub fn collect_call_path(expr: &Expr) -> Option { let mut segments = smallvec![]; - collect_call_path_inner(expr, &mut segments); - segments + collect_call_path_inner(expr, &mut segments).then_some(segments) } /// Convert an `Expr` to its call path (like `List`, or `typing.List`). pub fn compose_call_path(expr: &Expr) -> Option { - let call_path = collect_call_path(expr); - if call_path.is_empty() { - None - } else { - Some(format_call_path(&call_path)) - } + collect_call_path(expr).map(|call_path| format_call_path(&call_path)) } /// Format a call path for display. @@ -52,12 +46,33 @@ pub fn format_call_path(call_path: &[&str]) -> String { } } -/// Split a fully-qualified name (like `typing.List`) into (`typing`, `List`). -pub fn to_call_path(target: &str) -> CallPath { - if target.contains('.') { - target.split('.').collect() +/// Create a [`CallPath`] from an unqualified name. +/// +/// ```rust +/// # use smallvec::smallvec; +/// # use ruff_python_ast::call_path::from_unqualified_name; +/// +/// assert_eq!(from_unqualified_name("typing.List").as_slice(), ["typing", "List"]); +/// assert_eq!(from_unqualified_name("list").as_slice(), ["list"]); +/// ``` +pub fn from_unqualified_name(name: &str) -> CallPath { + name.split('.').collect() +} + +/// Create a [`CallPath`] from a fully-qualified name. +/// +/// ```rust +/// # use smallvec::smallvec; +/// # use ruff_python_ast::call_path::from_qualified_name; +/// +/// assert_eq!(from_qualified_name("typing.List").as_slice(), ["typing", "List"]); +/// assert_eq!(from_qualified_name("list").as_slice(), ["", "list"]); +/// ``` +pub fn from_qualified_name(name: &str) -> CallPath { + if name.contains('.') { + name.split('.').collect() } else { // Special-case: for builtins, return `["", "int"]` instead of `["int"]`. - smallvec!["", target] + smallvec!["", name] } } diff --git a/crates/ruff_python_ast/src/context.rs b/crates/ruff_python_ast/src/context.rs index cba1617889..c01b65572b 100644 --- a/crates/ruff_python_ast/src/context.rs +++ b/crates/ruff_python_ast/src/context.rs @@ -12,7 +12,7 @@ use crate::binding::{ Binding, BindingId, BindingKind, Bindings, Exceptions, ExecutionContext, FromImportation, Importation, SubmoduleImportation, }; -use crate::call_path::{collect_call_path, CallPath}; +use crate::call_path::{collect_call_path, from_unqualified_name, CallPath}; use crate::helpers::from_relative_import; use crate::scope::{Scope, ScopeId, ScopeKind, ScopeStack, Scopes}; use crate::types::RefEquality; @@ -117,7 +117,7 @@ impl<'a> Context<'a> { } if self.typing_modules.iter().any(|module| { - let mut module: CallPath = module.split('.').collect(); + let mut module: CallPath = from_unqualified_name(module); module.push(target); *call_path == module }) { @@ -156,7 +156,9 @@ impl<'a> Context<'a> { where 'b: 'a, { - let call_path = collect_call_path(value); + let Some(call_path) = collect_call_path(value) else { + return None; + }; let Some(head) = call_path.first() else { return None; }; @@ -177,7 +179,7 @@ impl<'a> Context<'a> { None } } else { - let mut source_path: CallPath = name.split('.').collect(); + let mut source_path: CallPath = from_unqualified_name(name); source_path.extend(call_path.into_iter().skip(1)); Some(source_path) } @@ -194,7 +196,7 @@ impl<'a> Context<'a> { None } } else { - let mut source_path: CallPath = name.split('.').collect(); + let mut source_path: CallPath = from_unqualified_name(name); source_path.extend(call_path.into_iter().skip(1)); Some(source_path) } diff --git a/crates/ruff_python_ast/src/function_type.rs b/crates/ruff_python_ast/src/function_type.rs index df60e3b32e..913a66d4cc 100644 --- a/crates/ruff_python_ast/src/function_type.rs +++ b/crates/ruff_python_ast/src/function_type.rs @@ -1,6 +1,6 @@ use rustpython_parser::ast::Expr; -use crate::call_path::to_call_path; +use crate::call_path::from_qualified_name; use crate::context::Context; use crate::helpers::map_callable; use crate::scope::{Scope, ScopeKind}; @@ -36,7 +36,7 @@ pub fn classify( call_path.as_slice() == ["", "staticmethod"] || staticmethod_decorators .iter() - .any(|decorator| call_path == to_call_path(decorator)) + .any(|decorator| call_path == from_qualified_name(decorator)) }) }) { FunctionType::StaticMethod @@ -56,7 +56,7 @@ pub fn classify( call_path.as_slice() == ["", "classmethod"] || classmethod_decorators .iter() - .any(|decorator| call_path == to_call_path(decorator)) + .any(|decorator| call_path == from_qualified_name(decorator)) }) }) { diff --git a/crates/ruff_python_ast/src/logging.rs b/crates/ruff_python_ast/src/logging.rs index 35e93eca7e..d189ac9ea9 100644 --- a/crates/ruff_python_ast/src/logging.rs +++ b/crates/ruff_python_ast/src/logging.rs @@ -43,9 +43,11 @@ impl LoggingLevel { /// ``` pub fn is_logger_candidate(context: &Context, func: &Expr) -> bool { if let ExprKind::Attribute { value, .. } = &func.node { - let call_path = context + let Some(call_path) = context .resolve_call_path(value) - .unwrap_or_else(|| collect_call_path(value)); + .or_else(|| collect_call_path(value)) else { + return false; + }; if let Some(tail) = call_path.last() { if tail.starts_with("log") || tail.ends_with("logger") || tail.ends_with("logging") { return true; diff --git a/crates/ruff_python_ast/src/typing.rs b/crates/ruff_python_ast/src/typing.rs index 3fce602670..729c904891 100644 --- a/crates/ruff_python_ast/src/typing.rs +++ b/crates/ruff_python_ast/src/typing.rs @@ -4,6 +4,7 @@ use rustpython_parser::ast::{Expr, ExprKind, Location}; use ruff_python_stdlib::typing::{PEP_585_BUILTINS_ELIGIBLE, PEP_593_SUBSCRIPTS, SUBSCRIPTS}; +use crate::call_path::{from_unqualified_name, CallPath}; use crate::context::Context; use crate::relocate::relocate_expr; use crate::source_code::Locator; @@ -47,7 +48,7 @@ pub fn match_annotated_subscript<'a>( } for module in typing_modules { - let module_call_path = module.split('.').collect::>(); + let module_call_path: CallPath = from_unqualified_name(module); if call_path.starts_with(&module_call_path) { for subscript in SUBSCRIPTS.iter() { if call_path.last() == subscript.last() { diff --git a/crates/ruff_python_ast/src/visibility.rs b/crates/ruff_python_ast/src/visibility.rs index 1b596b6479..f65653872b 100644 --- a/crates/ruff_python_ast/src/visibility.rs +++ b/crates/ruff_python_ast/src/visibility.rs @@ -183,9 +183,10 @@ pub fn method_visibility(stmt: &Stmt) -> Visibility { } => { // Is this a setter or deleter? if decorator_list.iter().any(|expr| { - let call_path = collect_call_path(expr); - call_path.as_slice() == [name, "setter"] - || call_path.as_slice() == [name, "deleter"] + collect_call_path(expr).map_or(false, |call_path| { + call_path.as_slice() == [name, "setter"] + || call_path.as_slice() == [name, "deleter"] + }) }) { return Visibility::Private; }