Merge pull request #2546 from ksss/special-accessibility

Fix special methods accessibility.

Author: ksss

lib/rbs/definition_builder.rb

@@ -695,12 +695,14 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
           )
         end
 
+        accessibility = special_accessibility(original.instance?, original.new_name) || original_method.accessibility
+
         method_definition = Definition::Method.new(
           super_method: existing_method,
           defs: original_method.defs.map do |defn|
             defn.update(defined_in: defined_in, implemented_in: implemented_in)
           end,
-          accessibility: original_method.accessibility,
+          accessibility: accessibility,
           alias_of: original_method,
           alias_member: original
         )
@@ -727,13 +729,9 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
           end
         end
 
-        # @type var accessibility: RBS::Definition::accessibility
-        accessibility =
-          if original.instance? && [:initialize, :initialize_copy, :initialize_clone, :initialize_dup, :respond_to_missing?].include?(method.name)
-            :private
-          else
-            method.accessibility
-          end
+        # Respect the visibility of the original method definition.
+        accessibility = original.visibility || special_accessibility(original.instance?, method.name) || method.accessibility
+
         # Skip setting up `super_method` if `implemented_in` is `nil`, that means the type doesn't have implementation.
         # This typically happens if the type is an interface.
         if implemented_in
@@ -786,6 +784,9 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
           super_method = existing_method
         end
 
+        # Respect the visibility of the original method definition.
+        accessibility = original.visibility || special_accessibility(original.kind == :instance, method.name) || method.accessibility
+
         method_definition = Definition::Method.new(
           super_method: super_method,
           defs: [
@@ -796,7 +797,7 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
               implemented_in: implemented_in
             )
           ],
-          accessibility: method.accessibility,
+          accessibility: accessibility,
           alias_of: nil,
           alias_member: nil
         )
@@ -943,6 +944,12 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
       methods[method.name] = method_definition
     end
 
+    def special_accessibility(is_instance, method_name)
+      if is_instance && [:initialize, :initialize_copy, :initialize_clone, :initialize_dup, :respond_to_missing?].include?(method_name)
+        :private
+      end
+    end
+
     def try_cache(type_name, cache:)
       cache[type_name] ||= yield
     end

sig/definition_builder.rbs

@@ -97,6 +97,8 @@ module RBS
     #
     def interface_methods: (Array[Definition::Ancestor::Instance] interface_ancestors) -> interface_methods
 
+    def special_accessibility: (bool, Symbol) -> Definition::accessibility?
+
     def try_cache: (TypeName, cache: Hash[TypeName, Definition | false | nil]) { () -> Definition } -> Definition
 
     def validate_params_with: (Array[AST::TypeParam], result: VarianceCalculator::Result) { (AST::TypeParam) -> void } -> void

test/rbs/definition_builder_test.rb

@@ -1156,6 +1156,14 @@ def self.initialize_clone: (self) -> self
   def self.initialize_dup: (self) -> self
   def self.respond_to_missing?: () -> bool
 end
+
+class DirectPublic
+  public def initialize: () -> void
+  public def initialize_copy: (self) -> self
+  public def initialize_clone: (self) -> self
+  public def initialize_dup: (self) -> self
+  public def respond_to_missing?: () -> bool
+end
 EOF
 
       manager.build do |env|
@@ -1179,6 +1187,15 @@ def self.respond_to_missing?: () -> bool
           assert_method_definition definition.methods[:initialize_dup], ["(self) -> self"], accessibility: :public
           assert_method_definition definition.methods[:respond_to_missing?], ["() -> bool"], accessibility: :public
         end
+
+        builder.build_instance(type_name("::DirectPublic")).tap do |definition|
+          assert_instance_of Definition, definition
+          assert_method_definition definition.methods[:initialize], ["() -> void"], accessibility: :public
+          assert_method_definition definition.methods[:initialize_copy], ["(self) -> self"], accessibility: :public
+          assert_method_definition definition.methods[:initialize_clone], ["(self) -> self"], accessibility: :public
+          assert_method_definition definition.methods[:initialize_dup], ["(self) -> self"], accessibility: :public
+          assert_method_definition definition.methods[:respond_to_missing?], ["() -> bool"], accessibility: :public
+        end
       end
     end
   end
@@ -2215,6 +2232,60 @@ def self?.a: () -> void
     end
   end
 
+  def test_alias_visibility_with_special_method
+    SignatureManager.new do |manager|
+      manager.files.merge!(Pathname("foo.rbs") => <<-EOF)
+class C
+  def original: () -> void
+  alias initialize original
+
+  def self.original: () -> void
+  alias self.initialize self.original
+end
+      EOF
+      manager.build do |env|
+        builder = DefinitionBuilder.new(env: env)
+
+        builder.build_instance(type_name("::C")).tap do |definition|
+          assert_predicate definition.methods[:original], :public?
+          assert_predicate definition.methods[:initialize], :private?
+        end
+
+        builder.build_singleton(type_name("::C")).tap do |definition|
+          assert_predicate definition.methods[:original], :public?
+          assert_predicate definition.methods[:initialize], :public?
+        end
+      end
+    end
+  end
+
+  def test_attribute_visibility_with_special_method
+    SignatureManager.new do |manager|
+      manager.files.merge!(Pathname("foo.rbs") => <<-EOF)
+class C
+  attr_reader initialize: String
+  public attr_reader initialize_copy: String
+
+  attr_reader self.initialize: String
+  public attr_reader self.initialize_copy: String
+end
+      EOF
+      manager.build do |env|
+        builder = DefinitionBuilder.new(env: env)
+
+        builder.build_instance(type_name("::C")).tap do |definition|
+          assert_predicate definition.methods[:initialize], :private?
+          assert_predicate definition.methods[:initialize_copy], :public?
+        end
+
+        builder.build_singleton(type_name("::C")).tap do |definition|
+          assert_predicate definition.methods[:initialize], :public?
+          assert_predicate definition.methods[:initialize_copy], :public?
+        end
+      end
+    end
+  end
+
   def test_def_with_visibility_modifier
     SignatureManager.new do |manager|
       manager.files.merge!(Pathname("foo.rbs") => <<-EOF)