Make collect_call_path return an Option (#3849)

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<CallPath> {
     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<String> {
-    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]
     }
 }

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)
                 }

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))
             })
         })
     {

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;

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::<Vec<_>>();
+            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() {

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;
             }