Skip to content

Commit eaf59ca

Browse files
committed
Fix special methods accessibility.
1 parent 2508852 commit eaf59ca

File tree

3 files changed

+89
-9
lines changed

3 files changed

+89
-9
lines changed

lib/rbs/definition_builder.rb

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -659,12 +659,14 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
659659
)
660660
end
661661

662+
accessibility = special_accessibility(original.instance?, original.new_name) || original_method.accessibility
663+
662664
method_definition = Definition::Method.new(
663665
super_method: existing_method,
664666
defs: original_method.defs.map do |defn|
665667
defn.update(defined_in: defined_in, implemented_in: implemented_in)
666668
end,
667-
accessibility: original_method.accessibility,
669+
accessibility: accessibility,
668670
alias_of: original_method,
669671
alias_member: original
670672
)
@@ -691,13 +693,9 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
691693
end
692694
end
693695

694-
# @type var accessibility: RBS::Definition::accessibility
695-
accessibility =
696-
if original.instance? && [:initialize, :initialize_copy, :initialize_clone, :initialize_dup, :respond_to_missing?].include?(method.name)
697-
:private
698-
else
699-
method.accessibility
700-
end
696+
# Respect the visibility of the original method definition.
697+
accessibility = original.visibility || special_accessibility(original.instance?, method.name) || method.accessibility
698+
701699
# Skip setting up `super_method` if `implemented_in` is `nil`, that means the type doesn't have implementation.
702700
# This typically happens if the type is an interface.
703701
if implemented_in
@@ -750,6 +748,9 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
750748
super_method = existing_method
751749
end
752750

751+
# Respect the visibility of the original method definition.
752+
accessibility = original.visibility || special_accessibility(original.kind == :instance, method.name) || method.accessibility
753+
753754
method_definition = Definition::Method.new(
754755
super_method: super_method,
755756
defs: [
@@ -760,7 +761,7 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
760761
implemented_in: implemented_in
761762
)
762763
],
763-
accessibility: method.accessibility,
764+
accessibility: accessibility,
764765
alias_of: nil,
765766
alias_member: nil
766767
)
@@ -854,6 +855,12 @@ def define_method(methods, definition, method, subst, self_type_methods, defined
854855
methods[method.name] = method_definition
855856
end
856857

858+
def special_accessibility(is_instance, method_name)
859+
if is_instance && [:initialize, :initialize_copy, :initialize_clone, :initialize_dup, :respond_to_missing?].include?(method_name)
860+
:private
861+
end
862+
end
863+
857864
def try_cache(type_name, cache:)
858865
cache[type_name] ||= yield
859866
end

sig/definition_builder.rbs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ module RBS
9797
#
9898
def interface_methods: (Array[Definition::Ancestor::Instance] interface_ancestors) -> interface_methods
9999

100+
def special_accessibility: (bool, Symbol) -> Definition::accessibility?
101+
100102
def try_cache: (TypeName, cache: Hash[TypeName, Definition | false | nil]) { () -> Definition } -> Definition
101103

102104
def validate_params_with: (Array[AST::TypeParam], result: VarianceCalculator::Result) { (AST::TypeParam) -> void } -> void

test/rbs/definition_builder_test.rb

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,14 @@ def self.initialize_clone: (self) -> self
11561156
def self.initialize_dup: (self) -> self
11571157
def self.respond_to_missing?: () -> bool
11581158
end
1159+
1160+
class DirectPublic
1161+
public def initialize: () -> void
1162+
public def initialize_copy: (self) -> self
1163+
public def initialize_clone: (self) -> self
1164+
public def initialize_dup: (self) -> self
1165+
public def respond_to_missing?: () -> bool
1166+
end
11591167
EOF
11601168

11611169
manager.build do |env|
@@ -1179,6 +1187,15 @@ def self.respond_to_missing?: () -> bool
11791187
assert_method_definition definition.methods[:initialize_dup], ["(self) -> self"], accessibility: :public
11801188
assert_method_definition definition.methods[:respond_to_missing?], ["() -> bool"], accessibility: :public
11811189
end
1190+
1191+
builder.build_instance(type_name("::DirectPublic")).tap do |definition|
1192+
assert_instance_of Definition, definition
1193+
assert_method_definition definition.methods[:initialize], ["() -> void"], accessibility: :public
1194+
assert_method_definition definition.methods[:initialize_copy], ["(self) -> self"], accessibility: :public
1195+
assert_method_definition definition.methods[:initialize_clone], ["(self) -> self"], accessibility: :public
1196+
assert_method_definition definition.methods[:initialize_dup], ["(self) -> self"], accessibility: :public
1197+
assert_method_definition definition.methods[:respond_to_missing?], ["() -> bool"], accessibility: :public
1198+
end
11821199
end
11831200
end
11841201
end
@@ -2215,6 +2232,60 @@ def self?.a: () -> void
22152232
end
22162233
end
22172234

2235+
def test_alias_visibility_with_special_method
2236+
SignatureManager.new do |manager|
2237+
manager.files.merge!(Pathname("foo.rbs") => <<-EOF)
2238+
class C
2239+
def original: () -> void
2240+
alias initialize original
2241+
2242+
def self.original: () -> void
2243+
alias self.initialize self.original
2244+
end
2245+
EOF
2246+
manager.build do |env|
2247+
builder = DefinitionBuilder.new(env: env)
2248+
2249+
builder.build_instance(type_name("::C")).tap do |definition|
2250+
assert_predicate definition.methods[:original], :public?
2251+
assert_predicate definition.methods[:initialize], :private?
2252+
end
2253+
2254+
builder.build_singleton(type_name("::C")).tap do |definition|
2255+
assert_predicate definition.methods[:original], :public?
2256+
assert_predicate definition.methods[:initialize], :public?
2257+
end
2258+
end
2259+
end
2260+
end
2261+
2262+
def test_attribute_visibility_with_special_method
2263+
SignatureManager.new do |manager|
2264+
manager.files.merge!(Pathname("foo.rbs") => <<-EOF)
2265+
class C
2266+
attr_reader initialize: String
2267+
public attr_reader initialize_copy: String
2268+
2269+
attr_reader self.initialize: String
2270+
public attr_reader self.initialize_copy: String
2271+
end
2272+
EOF
2273+
manager.build do |env|
2274+
builder = DefinitionBuilder.new(env: env)
2275+
2276+
builder.build_instance(type_name("::C")).tap do |definition|
2277+
assert_predicate definition.methods[:initialize], :private?
2278+
assert_predicate definition.methods[:initialize_copy], :public?
2279+
end
2280+
2281+
builder.build_singleton(type_name("::C")).tap do |definition|
2282+
assert_predicate definition.methods[:initialize], :public?
2283+
assert_predicate definition.methods[:initialize_copy], :public?
2284+
end
2285+
end
2286+
end
2287+
end
2288+
22182289
def test_def_with_visibility_modifier
22192290
SignatureManager.new do |manager|
22202291
manager.files.merge!(Pathname("foo.rbs") => <<-EOF)

0 commit comments

Comments
 (0)