Reduce unnecessary external state assignment (singleton) (#1312)

It really bugs me that RDoc has a tendency of assigning a state through
a setter immediately after the object's initialization. This makes the
dependencies unclear and code unnecessarily verbose.

I'm taking a first step to fix the pattern by passing `singleton` as a
constructor argument whenever possible.

---------

Co-authored-by: Vinicius Stock <[email protected]>

Author: st0012

lib/rdoc/code_object/alias.rb

@@ -23,7 +23,7 @@ class RDoc::Alias < RDoc::CodeObject
   ##
   # Is this an alias declared in a singleton context?
 
-  attr_accessor :singleton
+  attr_reader :singleton
 
   ##
   # Source file token stream
@@ -34,7 +34,7 @@ class RDoc::Alias < RDoc::CodeObject
   # Creates a new Alias with a token stream of +text+ that aliases +old_name+
   # to +new_name+, has +comment+ and is a +singleton+ context.
 
-  def initialize(text, old_name, new_name, comment, singleton = false)
+  def initialize(text, old_name, new_name, comment, singleton: false)
     super()
 
     @text = text

lib/rdoc/code_object/any_method.rb

@@ -39,8 +39,8 @@ class RDoc::AnyMethod < RDoc::MethodAttr
   ##
   # Creates a new AnyMethod with a token stream +text+ and +name+
 
-  def initialize text, name
-    super
+  def initialize(text, name, singleton: false)
+    super(text, name, singleton: singleton)
 
     @c_function = nil
     @dont_rename_initialize = false
@@ -53,10 +53,9 @@ def initialize text, name
   # Adds +an_alias+ as an alias for this method in +context+.
 
   def add_alias an_alias, context = nil
-    method = self.class.new an_alias.text, an_alias.new_name
+    method = self.class.new an_alias.text, an_alias.new_name, singleton: singleton
 
     method.record_location an_alias.file
-    method.singleton = self.singleton
     method.params = self.params
     method.visibility = self.visibility
     method.comment = an_alias.comment
@@ -207,7 +206,7 @@ def marshal_load array
     @is_alias_for  = array[15]
 
     array[8].each do |new_name, document|
-      add_alias RDoc::Alias.new(nil, @name, new_name, RDoc::Comment.from_document(document), @singleton)
+      add_alias RDoc::Alias.new(nil, @name, new_name, RDoc::Comment.from_document(document), singleton: @singleton)
     end
 
     @parent_name ||= if @full_name =~ /#/ then

lib/rdoc/code_object/attr.rb

@@ -22,11 +22,10 @@ class RDoc::Attr < RDoc::MethodAttr
   # Creates a new Attr with body +text+, +name+, read/write status +rw+ and
   # +comment+.  +singleton+ marks this as a class attribute.
 
-  def initialize(text, name, rw, comment, singleton = false)
-    super text, name
+  def initialize(text, name, rw, comment, singleton: false)
+    super(text, name, singleton: singleton)
 
     @rw = rw
-    @singleton = singleton
     self.comment = comment
   end
 
@@ -44,9 +43,7 @@ def == other
   # Add +an_alias+ as an attribute in +context+.
 
   def add_alias(an_alias, context)
-    new_attr = self.class.new(self.text, an_alias.new_name, self.rw,
-                              self.comment, self.singleton)
-
+    new_attr = self.class.new(text, an_alias.new_name, rw, comment, singleton: singleton)
     new_attr.record_location an_alias.file
     new_attr.visibility = self.visibility
     new_attr.is_alias_for = self

lib/rdoc/code_object/class_module.rb

@@ -392,7 +392,7 @@ def marshal_load array # :nodoc:
       singleton  ||= false
       visibility ||= :public
 
-      attr = RDoc::Attr.new nil, name, rw, nil, singleton
+      attr = RDoc::Attr.new nil, name, rw, nil, singleton: singleton
 
       add_attribute attr
       attr.visibility = visibility
@@ -419,8 +419,7 @@ def marshal_load array # :nodoc:
         @visibility = visibility
 
         methods.each do |name, file|
-          method = RDoc::AnyMethod.new nil, name
-          method.singleton = true if type == 'class'
+          method = RDoc::AnyMethod.new nil, name, singleton: type == 'class'
           method.record_location RDoc::TopLevel.new file
           add_method method
         end

lib/rdoc/code_object/method_attr.rb

@@ -69,7 +69,7 @@ class RDoc::MethodAttr < RDoc::CodeObject
   #
   # Usually this is called by super from a subclass.
 
-  def initialize text, name
+  def initialize(text, name, singleton: false)
     super()
 
     @text = text
@@ -78,7 +78,7 @@ def initialize text, name
     @aliases      = []
     @is_alias_for = nil
     @parent_name  = nil
-    @singleton    = nil
+    @singleton    = singleton
     @visibility   = :public
     @see = false
 

lib/rdoc/parser/c.rb

@@ -248,9 +248,7 @@ def do_aliases
   # method that reference the same function.
 
   def add_alias(var_name, class_obj, old_name, new_name, comment)
-    al = RDoc::Alias.new '', old_name, new_name, ''
-    al.singleton = @singleton_classes.key? var_name
-    al.comment = comment
+    al = RDoc::Alias.new '', old_name, new_name, comment, singleton: @singleton_classes.key?(var_name)
     al.record_location @top_level
     class_obj.add_alias al
     @stats.add_alias al
@@ -1015,10 +1013,9 @@ def handle_method(type, var_name, meth_name, function, param_count,
         type = 'method' # force public
       end
 
-      meth_obj = RDoc::AnyMethod.new '', meth_name
+      singleton = singleton || %w[singleton_method module_function].include?(type)
+      meth_obj = RDoc::AnyMethod.new '', meth_name, singleton: singleton
       meth_obj.c_function = function
-      meth_obj.singleton =
-        singleton || %w[singleton_method module_function].include?(type)
 
       p_count = Integer(param_count) rescue -1
 

lib/rdoc/parser/prism_ruby.rb

@@ -289,18 +289,16 @@ def handle_meta_method_comment(comment, node)
 
     if attributes
       attributes.each do |attr|
-        a = RDoc::Attr.new(@container, attr, rw, processed_comment)
+        a = RDoc::Attr.new(@container, attr, rw, processed_comment, singleton: @singleton)
         a.store = @store
         a.line = line_no
-        a.singleton = @singleton
         record_location(a)
         @container.add_attribute(a)
         a.visibility = visibility
       end
     elsif line_no || node
       method_name ||= call_node_name_arguments(node).first if is_call_node
-      meth = RDoc::AnyMethod.new(@container, method_name)
-      meth.singleton = @singleton || singleton_method
+      meth = RDoc::AnyMethod.new(@container, method_name, singleton: @singleton || singleton_method)
       handle_consecutive_comment_directive(meth, comment)
       comment.normalize
       meth.call_seq = comment.extract_call_seq
@@ -316,7 +314,6 @@ def handle_meta_method_comment(comment, node)
         meth,
         line_no: line_no,
         visibility: visibility,
-        singleton: @singleton || singleton_method,
         params: '()',
         calls_super: false,
         block_params: nil,
@@ -452,8 +449,7 @@ def add_alias_method(old_name, new_name, line_no)
     comment = consecutive_comment(line_no)
     handle_consecutive_comment_directive(@container, comment)
     visibility = @container.find_method(old_name, @singleton)&.visibility || :public
-    a = RDoc::Alias.new(nil, old_name, new_name, comment, @singleton)
-    a.comment = comment
+    a = RDoc::Alias.new(nil, old_name, new_name, comment, singleton: @singleton)
     handle_modifier_directive(a, line_no)
     a.store = @store
     a.line = line_no
@@ -472,10 +468,9 @@ def add_attributes(names, rw, line_no)
     return unless @container.document_children
 
     names.each do |symbol|
-      a = RDoc::Attr.new(nil, symbol.to_s, rw, comment)
+      a = RDoc::Attr.new(nil, symbol.to_s, rw, comment, singleton: @singleton)
       a.store = @store
       a.line = line_no
-      a.singleton = @singleton
       record_location(a)
       handle_modifier_directive(a, line_no)
       @container.add_attribute(a) if should_document?(a)
@@ -514,7 +509,7 @@ def add_method(name, receiver_name:, receiver_fallback_type:, visibility:, singl
     return if @in_proc_block
 
     receiver = receiver_name ? find_or_create_module_path(receiver_name, receiver_fallback_type) : @container
-    meth = RDoc::AnyMethod.new(nil, name)
+    meth = RDoc::AnyMethod.new(nil, name, singleton: singleton)
     if (comment = consecutive_comment(start_line))
       handle_consecutive_comment_directive(@container, comment)
       handle_consecutive_comment_directive(meth, comment)
@@ -533,7 +528,6 @@ def add_method(name, receiver_name:, receiver_fallback_type:, visibility:, singl
       meth,
       line_no: start_line,
       visibility: visibility,
-      singleton: singleton,
       params: params,
       calls_super: calls_super,
       block_params: block_params,
@@ -553,12 +547,11 @@ def add_method(name, receiver_name:, receiver_fallback_type:, visibility:, singl
     end
   end
 
-  private def internal_add_method(container, meth, line_no:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:) # :nodoc:
+  private def internal_add_method(container, meth, line_no:, visibility:, params:, calls_super:, block_params:, tokens:) # :nodoc:
     meth.name ||= meth.call_seq[/\A[^()\s]+/] if meth.call_seq
     meth.name ||= 'unknown'
     meth.store = @store
     meth.line = line_no
-    meth.singleton = singleton
     container.add_method(meth) # should add after setting singleton and before setting visibility
     meth.visibility = visibility
     meth.params ||= params

lib/rdoc/parser/ruby.rb

@@ -293,7 +293,7 @@ def consume_trailing_spaces # :nodoc:
   # Creates a new attribute in +container+ with +name+.
 
   def create_attr container, single, name, rw, comment # :nodoc:
-    att = RDoc::Attr.new get_tkread, name, rw, comment, single == SINGLE
+    att = RDoc::Attr.new get_tkread, name, rw, comment, singleton: single == SINGLE
     record_location att
 
     container.add_attribute att
@@ -792,8 +792,7 @@ def parse_alias(context, single, tk, comment)
       return
     end
 
-    al = RDoc::Alias.new(get_tkread, old_name, new_name, comment,
-                         single == SINGLE)
+    al = RDoc::Alias.new(get_tkread, old_name, new_name, comment, singleton: single == SINGLE)
     record_location al
     al.line   = line_no
 
@@ -1358,10 +1357,9 @@ def parse_meta_method(container, single, tk, comment)
 
     return unless name
 
-    meth = RDoc::MetaMethod.new get_tkread, name
+    meth = RDoc::MetaMethod.new get_tkread, name, singleton: singleton
     record_location meth
     meth.line   = line_no
-    meth.singleton = singleton
 
     remove_token_listener self
 
@@ -1461,9 +1459,8 @@ def parse_method(container, single, tk, comment)
 
     return unless name
 
-    meth = RDoc::AnyMethod.new get_tkread, name
+    meth = RDoc::AnyMethod.new get_tkread, name, singleton: single == SINGLE ? true : singleton
     look_for_directives_in meth, comment
-    meth.singleton = single == SINGLE ? true : singleton
     if singleton
       # `current_line_visibility' is useless because it works against
       # the normal method named as same as the singleton method, after

test/rdoc/test_rdoc_any_method.rb

@@ -236,7 +236,7 @@ def test_marshal_dump
     assert_equal 'Klass#method', loaded.full_name
     assert_equal 'method',       loaded.name
     assert_equal 'param',        loaded.params
-    assert_nil                   loaded.singleton # defaults to nil
+    assert_equal false,          loaded.singleton
     assert_equal :public,        loaded.visibility
     assert_equal cm,             loaded.parent
     assert_equal section,        loaded.section

test/rdoc/test_rdoc_class_module.rb

@@ -171,7 +171,7 @@ def test_marshal_dump
 
     a1 = RDoc::Attr.new nil, 'a1', 'RW', ''
     a1.record_location tl
-    a2 = RDoc::Attr.new nil, 'a2', 'RW', '', true
+    a2 = RDoc::Attr.new nil, 'a2', 'RW', '', singleton: true
     a2.record_location tl
 
     m1 = RDoc::AnyMethod.new nil, 'm1'
@@ -358,7 +358,7 @@ def test_marshal_load_version_1
 
     a1 = RDoc::Attr.new nil, 'a1', 'RW', ''
     a1.record_location tl
-    a2 = RDoc::Attr.new nil, 'a2', 'RW', '', true
+    a2 = RDoc::Attr.new nil, 'a2', 'RW', '', singleton: true
     a2.record_location tl
 
     m1 = RDoc::AnyMethod.new nil, 'm1'
@@ -437,7 +437,7 @@ def test_marshal_load_version_2
 
     a1 = RDoc::Attr.new nil, 'a1', 'RW', ''
     a1.record_location tl
-    a2 = RDoc::Attr.new nil, 'a2', 'RW', '', true
+    a2 = RDoc::Attr.new nil, 'a2', 'RW', '', singleton: true
     a2.record_location tl
 
     m1 = RDoc::AnyMethod.new nil, 'm1'
@@ -522,7 +522,7 @@ def test_marshal_load_version_3
 
     a1 = RDoc::Attr.new nil, 'a1', 'RW', ''
     a1.record_location tl
-    a2 = RDoc::Attr.new nil, 'a2', 'RW', '', true
+    a2 = RDoc::Attr.new nil, 'a2', 'RW', '', singleton: true
     a2.record_location tl
 
     m1 = RDoc::AnyMethod.new nil, 'm1'
@@ -1583,14 +1583,14 @@ def setup
       extmod_method = @extmod.add_method(RDoc::AnyMethod.new(nil, "extmod_method"))
       extmod_method.section = @extmod.add_section("Extmod method section")
 
-      extmod_attr = @extmod.add_attribute(RDoc::Attr.new(nil, "extmod_attr_without_a_section", "RW", "", true))
-      extmod_attr = @extmod.add_attribute(RDoc::Attr.new(nil, "extmod_attr", "RW", "", true))
+      extmod_attr = @extmod.add_attribute(RDoc::Attr.new(nil, "extmod_attr_without_a_section", "RW", "", singleton: true))
+      extmod_attr = @extmod.add_attribute(RDoc::Attr.new(nil, "extmod_attr", "RW", "", singleton: true))
       extmod_attr.section = @extmod.add_section("Extmod attr section")
 
       extmod_private_method = @extmod.add_method(RDoc::AnyMethod.new(nil, "extmod_private_method"))
       extmod_private_method.visibility = :private
 
-      extmod_private_attr = @extmod.add_attribute(RDoc::Attr.new(nil, "extmod_private_attr", "RW", "", true))
+      extmod_private_attr = @extmod.add_attribute(RDoc::Attr.new(nil, "extmod_private_attr", "RW", "", singleton: true))
       extmod_private_attr.visibility = :private
 
       @klass.add_include(RDoc::Include.new("Incmod", nil))

test/rdoc/test_rdoc_context.rb

@@ -89,11 +89,9 @@ def test_add_alias_method
   end
 
   def test_add_alias_method_singleton
-    meth = RDoc::AnyMethod.new nil, 'old_name'
-    meth.singleton = true
+    meth = RDoc::AnyMethod.new nil, 'old_name', singleton: true
 
-    as = RDoc::Alias.new nil, 'old_name', 'new_name', 'comment'
-    as.singleton = true
+    as = RDoc::Alias.new nil, 'old_name', 'new_name', 'comment', singleton: true
 
     as.parent = @context
 

test/rdoc/test_rdoc_cross_reference.rb

@@ -176,11 +176,9 @@ def assert_resolve_method(x)
     @c1.methods_hash.clear
 
     i_op = RDoc::AnyMethod.new nil, x
-    i_op.singleton = false
     @c1.add_method i_op
 
-    c_op = RDoc::AnyMethod.new nil, x
-    c_op.singleton = true
+    c_op = RDoc::AnyMethod.new nil, x, singleton: true
     @c1.add_method c_op
 
     assert_ref i_op, x

test/rdoc/test_rdoc_markup_to_html_crossref.rb

@@ -122,7 +122,6 @@ def test_convert_RDOCLINK_rdoc_ref_method_label
 
   def test_convert_RDOCLINK_rdoc_ref_method_percent
     m = @c1.add_method RDoc::AnyMethod.new nil, '%'
-    m.singleton = false
 
     result = @to.convert 'rdoc-ref:C1#%'
 
@@ -137,7 +136,6 @@ def test_convert_RDOCLINK_rdoc_ref_method_percent
 
   def test_convert_RDOCLINK_rdoc_ref_method_escape_html
     m = @c1.add_method RDoc::AnyMethod.new nil, '<<'
-    m.singleton = false
 
     result = @to.convert 'rdoc-ref:C1#<<'
 
@@ -151,7 +149,6 @@ def test_convert_RDOCLINK_rdoc_ref_method_escape_html
 
   def test_convert_RDOCLINK_rdoc_ref_method_percent_label
     m = @c1.add_method RDoc::AnyMethod.new nil, '%'
-    m.singleton = false
 
     result = @to.convert 'rdoc-ref:C1#%@f'
 

test/rdoc/test_rdoc_ri_driver.rb

@@ -1214,8 +1214,7 @@ def test_list_methods_matching_regexp
     @cFoo.add_method index
     @store1.save_method @cFoo, index
 
-    c_index = RDoc::AnyMethod.new nil, '[]'
-    c_index.singleton = true
+    c_index = RDoc::AnyMethod.new nil, '[]', singleton: true
     c_index.record_location @top_level
     @cFoo.add_method c_index
     @store1.save_method @cFoo, c_index
@@ -1577,9 +1576,8 @@ def util_store
     @bother.params = "(things)"
     @bother.record_location @top_level
 
-    @new = @cFoo_Bar.add_method RDoc::AnyMethod.new nil, 'new'
+    @new = @cFoo_Bar.add_method RDoc::AnyMethod.new nil, 'new', singleton: true
     @new.record_location @top_level
-    @new.singleton = true
 
     @attr = @cFoo_Bar.add_attribute RDoc::Attr.new nil, 'attr', 'RW', ''
     @attr.record_location @top_level