Support array and inline-def arguments to private_class_method

Author: alexcrocha

rust/rubydex/src/indexing/ruby_indexer.rs

@@ -1277,6 +1277,7 @@ impl<'a> RubyIndexer<'a> {
         }
     }
 
+    #[allow(clippy::too_many_lines)]
     fn handle_singleton_method_visibility(
         &mut self,
         node: &ruby_prism::CallNode,
@@ -1316,46 +1317,125 @@ impl<'a> RubyIndexer<'a> {
         };
 
         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 {
+            match argument {
+                ruby_prism::Node::SymbolNode { .. } | ruby_prism::Node::StringNode { .. } => {
+                    let Some((name, location)) = Self::extract_visibility_literal(&argument) else {
                         continue;
+                    };
+                    let str_id = self.local_graph.intern_string(format!("{name}()"));
+                    let offset = Offset::from_prism_location(&location);
+                    let definition =
+                        Definition::SingletonMethodVisibility(Box::new(SingletonMethodVisibilityDefinition::new(
+                            receiver_name_id,
+                            str_id,
+                            visibility,
+                            self.uri_id,
+                            offset,
+                            Box::default(),
+                            DefinitionFlags::empty(),
+                            self.current_nesting_definition_id(),
+                        )));
+                    let definition_id = self.local_graph.add_definition(definition);
+                    self.add_member_to_current_owner(definition_id);
+                }
+                ruby_prism::Node::ArrayNode { .. } => {
+                    let array = argument.as_array_node().unwrap();
+                    for element in &array.elements() {
+                        match element {
+                            ruby_prism::Node::SymbolNode { .. } | ruby_prism::Node::StringNode { .. } => {
+                                let Some((name, location)) = Self::extract_visibility_literal(&element) else {
+                                    continue;
+                                };
+                                let str_id = self.local_graph.intern_string(format!("{name}()"));
+                                let offset = Offset::from_prism_location(&location);
+                                let definition = Definition::SingletonMethodVisibility(Box::new(
+                                    SingletonMethodVisibilityDefinition::new(
+                                        receiver_name_id,
+                                        str_id,
+                                        visibility,
+                                        self.uri_id,
+                                        offset,
+                                        Box::default(),
+                                        DefinitionFlags::empty(),
+                                        self.current_nesting_definition_id(),
+                                    ),
+                                ));
+                                let definition_id = self.local_graph.add_definition(definition);
+                                self.add_member_to_current_owner(definition_id);
+                            }
+                            _ => {
+                                self.local_graph.add_diagnostic(
+                                    Rule::InvalidSingletonMethodVisibility,
+                                    Offset::from_prism_location(&element.location()),
+                                    format!("`{call_name}` array element must be a Symbol or String"),
+                                );
+                                self.visit(&element);
+                            }
+                        }
                     }
                 }
-                ruby_prism::Node::StringNode { .. } => {
-                    let string = argument.as_string_node().unwrap();
-                    let name = String::from_utf8_lossy(string.unescaped()).to_string();
-                    (name, argument.location())
+                ruby_prism::Node::DefNode { .. } => {
+                    let def_node = argument.as_def_node().unwrap();
+                    if def_node.receiver().is_none() {
+                        self.local_graph.add_diagnostic(
+                            Rule::InvalidSingletonMethodVisibility,
+                            Offset::from_prism_location(&argument.location()),
+                            format!("`{call_name}` requires a singleton method definition"),
+                        );
+                    } else {
+                        let name_loc = def_node.name_loc();
+                        let name = Self::location_to_string(&name_loc);
+                        let str_id = self.local_graph.intern_string(format!("{name}()"));
+                        let offset = Offset::from_prism_location(&name_loc);
+                        let definition =
+                            Definition::SingletonMethodVisibility(Box::new(SingletonMethodVisibilityDefinition::new(
+                                receiver_name_id,
+                                str_id,
+                                visibility,
+                                self.uri_id,
+                                offset,
+                                Box::default(),
+                                DefinitionFlags::empty(),
+                                self.current_nesting_definition_id(),
+                            )));
+                        let definition_id = self.local_graph.add_definition(definition);
+                        self.add_member_to_current_owner(definition_id);
+                    }
+                    self.visit(&argument);
+                }
+                arg if Self::is_attr_call(&arg) => {
+                    self.local_graph.add_diagnostic(
+                        Rule::InvalidSingletonMethodVisibility,
+                        Offset::from_prism_location(&arg.location()),
+                        format!("`{call_name}` does not accept `attr_*` arguments"),
+                    );
+                    self.visit(&arg);
                 }
                 _ => {
                     self.local_graph.add_diagnostic(
                         Rule::InvalidSingletonMethodVisibility,
                         Offset::from_prism_location(&argument.location()),
                         format!("`{call_name}` called with a non-literal argument"),
                     );
-                    return;
+                    self.visit(&argument);
                 }
-            };
-
-            let str_id = self.local_graph.intern_string(format!("{name}()"));
-            let offset = Offset::from_prism_location(&location);
-            let definition = Definition::SingletonMethodVisibility(Box::new(SingletonMethodVisibilityDefinition::new(
-                receiver_name_id,
-                str_id,
-                visibility,
-                self.uri_id,
-                offset,
-                Box::default(),
-                DefinitionFlags::empty(),
-                self.current_nesting_definition_id(),
-            )));
-
-            let definition_id = self.local_graph.add_definition(definition);
+            }
+        }
+    }
 
-            self.add_member_to_current_owner(definition_id);
+    fn extract_visibility_literal<'b>(arg: &ruby_prism::Node<'b>) -> Option<(String, ruby_prism::Location<'b>)> {
+        match arg {
+            ruby_prism::Node::SymbolNode { .. } => {
+                let symbol = arg.as_symbol_node().unwrap();
+                let value_loc = symbol.value_loc()?;
+                Some((Self::location_to_string(&value_loc), value_loc))
+            }
+            ruby_prism::Node::StringNode { .. } => {
+                let string = arg.as_string_node().unwrap();
+                let name = String::from_utf8_lossy(string.unescaped()).to_string();
+                Some((name, arg.location()))
+            }
+            _ => None,
         }
     }
 

rust/rubydex/src/indexing/ruby_indexer_tests.rs

@@ -3807,6 +3807,9 @@ mod visibility_tests {
 
             module Foo
               private_class_method NOT_INDEXED
+              attr_reader :a_attr_target
+              private_class_method attr_reader(:bad)
+              private_class_method def inline; end
 
               def self.qux
                 private_class_method :Bar
@@ -3822,9 +3825,90 @@ mod visibility_tests {
                 "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: `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)",
             ]
         );
     }
+
+    #[test]
+    fn index_private_class_method_inline_def() {
+        let context = index_source(
+            r"
+            class Foo
+              private_class_method def self.inline; end
+            end
+            ",
+        );
+
+        assert_no_local_diagnostics!(&context);
+
+        assert_definition_at!(&context, "2:33-2:39", SingletonMethodVisibility, |def| {
+            assert_string_eq!(&context, def.target(), "inline()");
+            assert!(def.receiver().is_none());
+            assert_eq!(def.visibility(), &Visibility::Private);
+        });
+    }
+
+    #[test]
+    fn index_private_class_method_array_form() {
+        let context = index_source(
+            r#"
+            class Foo
+              def self.flat; end
+              def self.flat2; end
+              def self.mixed; end
+
+              private_class_method [:flat, :flat2]
+              public_class_method [:mixed, "flat"]
+            end
+            "#,
+        );
+
+        assert_no_local_diagnostics!(&context);
+
+        assert_definition_at!(&context, "6:26-6:30", SingletonMethodVisibility, |def| {
+            assert_string_eq!(&context, def.target(), "flat()");
+            assert_eq!(def.visibility(), &Visibility::Private);
+        });
+        assert_definition_at!(&context, "6:33-6:38", SingletonMethodVisibility, |def| {
+            assert_string_eq!(&context, def.target(), "flat2()");
+            assert_eq!(def.visibility(), &Visibility::Private);
+        });
+        assert_definition_at!(&context, "7:25-7:30", SingletonMethodVisibility, |def| {
+            assert_string_eq!(&context, def.target(), "mixed()");
+            assert_eq!(def.visibility(), &Visibility::Public);
+        });
+        assert_definition_at!(&context, "7:32-7:38", SingletonMethodVisibility, |def| {
+            assert_string_eq!(&context, def.target(), "flat()");
+            assert_eq!(def.visibility(), &Visibility::Public);
+        });
+    }
+
+    #[test]
+    fn index_private_class_method_array_form_non_literal_element_diagnostic() {
+        let context = index_source(
+            r"
+            class Foo
+              def self.flat; end
+
+              private_class_method [:flat, SOME_CONST]
+            end
+            ",
+        );
+
+        assert_local_diagnostics_eq!(
+            &context,
+            vec![
+                "invalid-singleton-method-visibility: `private_class_method` array element must be a Symbol or String (4:32-4:42)"
+            ]
+        );
+
+        assert_definition_at!(&context, "4:26-4:30", SingletonMethodVisibility, |def| {
+            assert_string_eq!(&context, def.target(), "flat()");
+            assert_eq!(def.visibility(), &Visibility::Private);
+        });
+    }
 }
 
 mod attr_accessor_tests {

rust/rubydex/src/resolution_tests.rs

@@ -5457,4 +5457,45 @@ mod visibility_resolution_tests {
 
         assert_no_diagnostics!(&context);
     }
+
+    #[test]
+    fn retroactive_singleton_method_visibility_inline_def() {
+        let mut context = GraphTest::new();
+        context.index_uri(
+            "file:///foo.rb",
+            r"
+            class Foo
+              private_class_method def self.bar; end
+            end
+            ",
+        );
+        context.resolve();
+
+        assert_no_diagnostics!(&context);
+        assert_visibility_eq!(context, "Foo::<Foo>#bar()", Visibility::Private);
+    }
+
+    #[test]
+    fn retroactive_singleton_method_visibility_array_form() {
+        let mut context = GraphTest::new();
+        context.index_uri(
+            "file:///foo.rb",
+            r#"
+            class Foo
+              def self.a; end
+              def self.b; end
+              def self.c; end
+
+              private_class_method [:a, "b"]
+              public_class_method [:c]
+            end
+            "#,
+        );
+        context.resolve();
+
+        assert_no_diagnostics!(&context);
+        assert_visibility_eq!(context, "Foo::<Foo>#a()", Visibility::Private);
+        assert_visibility_eq!(context, "Foo::<Foo>#b()", Visibility::Private);
+        assert_visibility_eq!(context, "Foo::<Foo>#c()", Visibility::Public);
+    }
 }

test/declaration_test.rb

@@ -841,6 +841,41 @@ def self.bar; end
     end
   end
 
+  def test_class_method_visibility_inline_def
+    with_context do |context|
+      context.write!("file1.rb", <<~RUBY)
+        class Foo
+          private_class_method def self.bar; end
+        end
+      RUBY
+
+      graph = Rubydex::Graph.new
+      graph.index_all(context.glob("**/*.rb"))
+      graph.resolve
+
+      assert_equal(:private, graph["Foo::<Foo>#bar()"].visibility)
+    end
+  end
+
+  def test_class_method_visibility_array_form
+    with_context do |context|
+      context.write!("file1.rb", <<~RUBY)
+        class Foo
+          def self.a; end
+          def self.b; end
+          private_class_method [:a, "b"]
+        end
+      RUBY
+
+      graph = Rubydex::Graph.new
+      graph.index_all(context.glob("**/*.rb"))
+      graph.resolve
+
+      assert_equal(:private, graph["Foo::<Foo>#a()"].visibility)
+      assert_equal(:private, graph["Foo::<Foo>#b()"].visibility)
+    end
+  end
+
   def test_constant_alias_visibility
     with_context do |context|
       context.write!("file1.rb", <<~RUBY)