Extract shared helpers for visibility-call processing

Author: alexcrocha

rust/rubydex/src/indexing/ruby_indexer.rs

@@ -1193,69 +1193,22 @@ impl<'a> RubyIndexer<'a> {
     }
 
     fn handle_constant_visibility(&mut self, node: &ruby_prism::CallNode, visibility: Visibility) {
-        let receiver = node.receiver();
-
-        let receiver_name_id = match receiver {
-            Some(ruby_prism::Node::ConstantPathNode { .. } | ruby_prism::Node::ConstantReadNode { .. }) => {
-                self.index_constant_reference(&receiver.unwrap(), true)
-            }
-            Some(ruby_prism::Node::SelfNode { .. }) | None => match self.nesting_stack.last() {
-                Some(Nesting::Method(_)) => {
-                    // Dynamic private constant (called from a method), we ignore it but don't report an error since it's valid Ruby
-                    // if being called from a singleton method.
-
-                    return;
-                }
-                None => {
-                    self.local_graph.add_diagnostic(
-                        Rule::InvalidPrivateConstant,
-                        Offset::from_prism_location(&node.location()),
-                        "Private constant called at top level".to_string(),
-                    );
-
-                    return;
-                }
-                _ => None,
-            },
-            _ => {
-                self.local_graph.add_diagnostic(
-                    Rule::InvalidPrivateConstant,
-                    Offset::from_prism_location(&node.location()),
-                    "Dynamic receiver for private constant".to_string(),
-                );
-
-                return;
-            }
+        let Some(receiver_name_id) = self.resolve_visibility_receiver(node, Rule::InvalidPrivateConstant) else {
+            return;
         };
 
         let Some(arguments) = node.arguments() else {
             return;
         };
 
         for argument in &arguments.arguments() {
-            let (name, location) = match argument {
-                ruby_prism::Node::SymbolNode { .. } => {
-                    let symbol = argument.as_symbol_node().unwrap();
-                    if let Some(value_loc) = symbol.value_loc() {
-                        (Self::location_to_string(&value_loc), value_loc)
-                    } else {
-                        continue;
-                    }
-                }
-                ruby_prism::Node::StringNode { .. } => {
-                    let string = argument.as_string_node().unwrap();
-                    let name = String::from_utf8_lossy(string.unescaped()).to_string();
-                    (name, argument.location())
-                }
-                _ => {
-                    self.local_graph.add_diagnostic(
-                        Rule::InvalidPrivateConstant,
-                        Offset::from_prism_location(&argument.location()),
-                        "Private constant called with non-symbol argument".to_string(),
-                    );
-
-                    return;
-                }
+            let Some((name, location)) = Self::extract_literal_name(&argument) else {
+                self.local_graph.add_diagnostic(
+                    Rule::InvalidPrivateConstant,
+                    Offset::from_prism_location(&argument.location()),
+                    "Private constant called with non-symbol argument".to_string(),
+                );
+                return;
             };
 
             let str_id = self.local_graph.intern_string(name);
@@ -1284,32 +1237,9 @@ impl<'a> RubyIndexer<'a> {
         visibility: Visibility,
         call_name: &str,
     ) {
-        let receiver = node.receiver();
-
-        let receiver_name_id = match receiver {
-            Some(ruby_prism::Node::ConstantPathNode { .. } | ruby_prism::Node::ConstantReadNode { .. }) => {
-                self.index_constant_reference(&receiver.unwrap(), true)
-            }
-            Some(ruby_prism::Node::SelfNode { .. }) | None => match self.nesting_stack.last() {
-                Some(Nesting::Method(_)) => return,
-                None => {
-                    self.local_graph.add_diagnostic(
-                        Rule::InvalidSingletonMethodVisibility,
-                        Offset::from_prism_location(&node.location()),
-                        format!("`{call_name}` called at top level"),
-                    );
-                    return;
-                }
-                _ => None,
-            },
-            _ => {
-                self.local_graph.add_diagnostic(
-                    Rule::InvalidSingletonMethodVisibility,
-                    Offset::from_prism_location(&node.location()),
-                    format!("Dynamic receiver for `{call_name}`"),
-                );
-                return;
-            }
+        let Some(receiver_name_id) = self.resolve_visibility_receiver(node, Rule::InvalidSingletonMethodVisibility)
+        else {
+            return;
         };
 
         let Some(arguments) = node.arguments() else {
@@ -1466,6 +1396,38 @@ impl<'a> RubyIndexer<'a> {
         }
     }
 
+    #[allow(clippy::option_option)]
+    fn resolve_visibility_receiver(&mut self, node: &ruby_prism::CallNode, rule: Rule) -> Option<Option<NameId>> {
+        let receiver = node.receiver();
+        match &receiver {
+            Some(ruby_prism::Node::ConstantPathNode { .. } | ruby_prism::Node::ConstantReadNode { .. }) => {
+                Some(self.index_constant_reference(receiver.as_ref().unwrap(), true))
+            }
+            Some(ruby_prism::Node::SelfNode { .. }) | None => match self.nesting_stack.last() {
+                Some(Nesting::Method(_)) => None,
+                None => {
+                    let call_name = String::from_utf8_lossy(node.name().as_slice());
+                    self.local_graph.add_diagnostic(
+                        rule,
+                        Offset::from_prism_location(&node.location()),
+                        format!("`{call_name}` called at top level"),
+                    );
+                    None
+                }
+                _ => Some(None),
+            },
+            Some(other) => {
+                let call_name = String::from_utf8_lossy(node.name().as_slice());
+                self.local_graph.add_diagnostic(
+                    rule,
+                    Offset::from_prism_location(&other.location()),
+                    format!("Dynamic receiver for `{call_name}`"),
+                );
+                None
+            }
+        }
+    }
+
     fn is_attr_call(arg: &ruby_prism::Node) -> bool {
         arg.as_call_node().is_some_and(|call| {
             let receiver = call.receiver();
@@ -1520,21 +1482,8 @@ impl<'a> RubyIndexer<'a> {
     }
 
     fn create_method_visibility_definition(&mut self, arg: &ruby_prism::Node, visibility: Visibility) {
-        let (name, location) = match arg {
-            ruby_prism::Node::SymbolNode { .. } => {
-                let symbol = arg.as_symbol_node().unwrap();
-                if let Some(value_loc) = symbol.value_loc() {
-                    (Self::location_to_string(&value_loc), value_loc)
-                } else {
-                    return;
-                }
-            }
-            ruby_prism::Node::StringNode { .. } => {
-                let string = arg.as_string_node().unwrap();
-                let name = String::from_utf8_lossy(string.unescaped()).to_string();
-                (name, arg.location())
-            }
-            _ => return,
+        let Some((name, location)) = Self::extract_literal_name(arg) else {
+            return;
         };
 
         let str_id = self.local_graph.intern_string(format!("{name}()"));

rust/rubydex/src/indexing/ruby_indexer_tests.rs

@@ -3333,9 +3333,9 @@ mod visibility_tests {
         assert_local_diagnostics_eq!(
             &context,
             vec![
-                "invalid-private-constant: Private constant called at top level (1:1-1:30)",
-                "invalid-private-constant: Private constant called at top level (2:1-2:35)",
-                "invalid-private-constant: Dynamic receiver for private constant (3:1-3:34)",
+                "invalid-private-constant: `private_constant` called at top level (1:1-1:30)",
+                "invalid-private-constant: `private_constant` called at top level (2:1-2:35)",
+                "invalid-private-constant: Dynamic receiver for `private_constant` (3:1-3:4)",
                 "invalid-private-constant: Private constant called with non-symbol argument (6:20-6:31)",
             ]
         );
@@ -3823,7 +3823,7 @@ mod visibility_tests {
             vec![
                 "invalid-singleton-method-visibility: `private_class_method` called at top level (1:1-1:34)",
                 "invalid-singleton-method-visibility: `private_class_method` called at top level (2:1-2:39)",
-                "invalid-singleton-method-visibility: Dynamic receiver for `private_class_method` (3:1-3:38)",
+                "invalid-singleton-method-visibility: Dynamic receiver for `private_class_method` (3:1-3:4)",
                 "invalid-singleton-method-visibility: `private_class_method` called with a non-literal argument (6:24-6:35)",
                 "invalid-singleton-method-visibility: `private_class_method` does not accept `attr_*` arguments (8:24-8:41)",
                 "invalid-singleton-method-visibility: `private_class_method` requires a singleton method definition (9:24-9:39)",