Better cooperation between public/protected/private with attr* and alias_method

13 files changed, 256 insertions, 42 deletions

diff --git a/NEWS.md b/NEWS.md
index 2e20e8dd8c..613976bfd6 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -249,6 +249,16 @@ Outstanding ones only.
         p C.ancestors #=> [C, M1, M2, Object, Kernel, BasicObject]
         ```
 
+    * Module#public, Module#protected and Module#private methods now accept single
+      array argument with a list of method names. [[Feature #17314]]
+
+    * Module#attr_accessor, Module#attr_reader, Module#attr_writer and Module#attr
+      methods now return array of defined methods names as symbols.
+      [[Feature #17314]]
+
+    * Module#alias_method now returns the defined alias as symbol.
+      [[Feature #17314]]
+
 * Mutex
 
     * `Mutex` is now acquired per-`Fiber` instead of per-`Thread`. This change
@@ -691,3 +701,4 @@ end
 [Feature #17371]: https://bugs.ruby-lang.org/issues/17371
 [GH-2991]:        https://github.com/ruby/ruby/pull/2991
 [Bug #17030]:     https://bugs.ruby-lang.org/issues/17030
+[Feature #17314]: https://bugs.ruby-lang.org/issues/17314
diff --git a/object.c b/object.c
index 552b10ba49..4fe6b02e03 100644
--- a/object.c
+++ b/object.c
@@ -2255,37 +2255,42 @@ id_for_attr(VALUE obj, VALUE name)
 
 /*
  *  call-seq:
- *     attr_reader(symbol, ...)  -> nil
- *     attr(symbol, ...)         -> nil
- *     attr_reader(string, ...)  -> nil
- *     attr(string, ...)         -> nil
+ *     attr_reader(symbol, ...)  -> array
+ *     attr(symbol, ...)         -> array
+ *     attr_reader(string, ...)  -> array
+ *     attr(string, ...)         -> array
  *
  *  Creates instance variables and corresponding methods that return the
  *  value of each instance variable. Equivalent to calling
  *  ``<code>attr</code><i>:name</i>'' on each name in turn.
  *  String arguments are converted to symbols.
+ *  Returns an array of defined methods names as symbols.
  */
 
 static VALUE
 rb_mod_attr_reader(int argc, VALUE *argv, VALUE klass)
 {
     int i;
+    VALUE names = rb_ary_new2(argc);
 
     for (i=0; i<argc; i++) {
-	rb_attr(klass, id_for_attr(klass, argv[i]), TRUE, FALSE, TRUE);
+	ID id = id_for_attr(klass, argv[i]);
+	rb_attr(klass, id, TRUE, FALSE, TRUE);
+	rb_ary_push(names, ID2SYM(id));
     }
-    return Qnil;
+    return names;
 }
 
 /**
  *  call-seq:
- *    attr(name, ...) -> nil
- *    attr(name, true) -> nil
- *    attr(name, false) -> nil
+ *    attr(name, ...) -> array
+ *    attr(name, true) -> array
+ *    attr(name, false) -> array
  *
  *  The first form is equivalent to #attr_reader.
  *  The second form is equivalent to <code>attr_accessor(name)</code> but deprecated.
  *  The last form is equivalent to <code>attr_reader(name)</code> but deprecated.
+ *  Returns an array of defined methods names as symbols.
  *--
  * \private
  * \todo can be static?
@@ -2295,47 +2300,57 @@ VALUE
 rb_mod_attr(int argc, VALUE *argv, VALUE klass)
 {
     if (argc == 2 && (argv[1] == Qtrue || argv[1] == Qfalse)) {
+	ID id = id_for_attr(klass, argv[0]);
+	VALUE names = rb_ary_new();
+
 	rb_warning("optional boolean argument is obsoleted");
-	rb_attr(klass, id_for_attr(klass, argv[0]), 1, RTEST(argv[1]), TRUE);
-	return Qnil;
+	rb_attr(klass, id, 1, RTEST(argv[1]), TRUE);
+	rb_ary_push(names, ID2SYM(id));
+	if (argv[1] == Qtrue) rb_ary_push(names, rb_str_intern(rb_sprintf("%"PRIsVALUE"=", ID2SYM(id))));
+	return names;
     }
     return rb_mod_attr_reader(argc, argv, klass);
 }
 
 /*
  *  call-seq:
- *      attr_writer(symbol, ...)    -> nil
- *      attr_writer(string, ...)    -> nil
+ *      attr_writer(symbol, ...)    -> array
+ *      attr_writer(string, ...)    -> array
  *
  *  Creates an accessor method to allow assignment to the attribute
  *  <i>symbol</i><code>.id2name</code>.
  *  String arguments are converted to symbols.
+ *  Returns an array of defined methods names as symbols.
  */
 
 static VALUE
 rb_mod_attr_writer(int argc, VALUE *argv, VALUE klass)
 {
     int i;
+    VALUE names = rb_ary_new2(argc);
 
     for (i=0; i<argc; i++) {
-	rb_attr(klass, id_for_attr(klass, argv[i]), FALSE, TRUE, TRUE);
+	ID id = id_for_attr(klass, argv[i]);
+	rb_attr(klass, id, FALSE, TRUE, TRUE);
+	rb_ary_push(names, rb_str_intern(rb_sprintf("%"PRIsVALUE"=", ID2SYM(id))));
     }
-    return Qnil;
+    return names;
 }
 
 /*
  *  call-seq:
- *     attr_accessor(symbol, ...)    -> nil
- *     attr_accessor(string, ...)    -> nil
+ *     attr_accessor(symbol, ...)    -> array
+ *     attr_accessor(string, ...)    -> array
  *
  *  Defines a named attribute for this module, where the name is
  *  <i>symbol.</i><code>id2name</code>, creating an instance variable
  *  (<code>@name</code>) and a corresponding access method to read it.
  *  Also creates a method called <code>name=</code> to set the attribute.
  *  String arguments are converted to symbols.
+ *  Returns an array of defined methods names as symbols.
  *
  *     module Mod
- *       attr_accessor(:one, :two)
+ *       attr_accessor(:one, :two) #=> [:one, :one=, :two, :two=]
  *     end
  *     Mod.instance_methods.sort   #=> [:one, :one=, :two, :two=]
  */
@@ -2344,11 +2359,17 @@ static VALUE
 rb_mod_attr_accessor(int argc, VALUE *argv, VALUE klass)
 {
     int i;
+    VALUE names = rb_ary_new2(argc * 2);
 
     for (i=0; i<argc; i++) {
-	rb_attr(klass, id_for_attr(klass, argv[i]), TRUE, TRUE, TRUE);
+	ID id = id_for_attr(klass, argv[i]);
+	VALUE idv = ID2SYM(id);
+
+	rb_attr(klass, id, TRUE, TRUE, TRUE);
+	rb_ary_push(names, idv);
+	rb_ary_push(names, rb_str_intern(rb_sprintf("%"PRIsVALUE"=", idv)));
     }
-    return Qnil;
+    return names;
 }
 
 /*
diff --git a/process.c b/process.c
index dee565ede4..0b2fdeb840 100644
--- a/process.c
+++ b/process.c
@@ -2075,12 +2075,11 @@ check_exec_redirect1(VALUE ary, VALUE key, VALUE param)
         rb_ary_push(ary, hide_obj(rb_assoc_new(fd, param)));
     }
     else {
-        int i, n=0;
+        int i;
         for (i = 0 ; i < RARRAY_LEN(key); i++) {
             VALUE v = RARRAY_AREF(key, i);
             VALUE fd = check_exec_redirect_fd(v, !NIL_P(param));
             rb_ary_push(ary, hide_obj(rb_assoc_new(fd, param)));
-            n++;
         }
     }
     return ary;
diff --git a/spec/ruby/core/main/fixtures/classes.rb b/spec/ruby/core/main/fixtures/classes.rb
index 6aba948ce0..757cee4e4a 100644
--- a/spec/ruby/core/main/fixtures/classes.rb
+++ b/spec/ruby/core/main/fixtures/classes.rb
@@ -13,6 +13,14 @@ def main_public_method
 end
 public :main_public_method
 
+def main_public_method2
+end
+public :main_public_method2
+
 def main_private_method
 end
 private :main_private_method
+
+def main_private_method2
+end
+private :main_private_method2
diff --git a/spec/ruby/core/main/private_spec.rb b/spec/ruby/core/main/private_spec.rb
index e34e0c7b7b..78c5d287d4 100644
--- a/spec/ruby/core/main/private_spec.rb
+++ b/spec/ruby/core/main/private_spec.rb
@@ -4,20 +4,41 @@ require_relative 'fixtures/classes'
 describe "main#private" do
   after :each do
     Object.send(:public, :main_public_method)
+    Object.send(:public, :main_public_method2)
   end
 
-  it "sets the visibility of the given method to private" do
-    eval "private :main_public_method", TOPLEVEL_BINDING
-    Object.should have_private_method(:main_public_method)
+  context "when single argument is passed and it is not an array" do
+    it "sets the visibility of the given methods to private" do
+      eval "private :main_public_method", TOPLEVEL_BINDING
+      Object.should have_private_method(:main_public_method)
+    end
+  end
+
+  context "when multiple arguments are passed" do
+    it "sets the visibility of the given methods to private" do
+      eval "private :main_public_method, :main_public_method2", TOPLEVEL_BINDING
+      Object.should have_private_method(:main_public_method)
+      Object.should have_private_method(:main_public_method2)
+    end
+  end
+
+  ruby_version_is "3.0" do
+    context "when single argument is passed and is an array" do
+      it "sets the visibility of the given methods to private" do
+        eval "private [:main_public_method, :main_public_method2]", TOPLEVEL_BINDING
+        Object.should have_private_method(:main_public_method)
+        Object.should have_private_method(:main_public_method2)
+      end
+    end
   end
 
   it "returns Object" do
     eval("private :main_public_method", TOPLEVEL_BINDING).should equal(Object)
   end
 
-  it "raises a NameError when given an undefined name" do
+  it "raises a NameError when at least one of given method names is undefined" do
     -> do
-      eval "private :main_undefined_method", TOPLEVEL_BINDING
+      eval "private :main_public_method, :main_undefined_method", TOPLEVEL_BINDING
     end.should raise_error(NameError)
   end
 end
diff --git a/spec/ruby/core/main/public_spec.rb b/spec/ruby/core/main/public_spec.rb
index afe25c705a..bfc27a9e80 100644
--- a/spec/ruby/core/main/public_spec.rb
+++ b/spec/ruby/core/main/public_spec.rb
@@ -4,11 +4,32 @@ require_relative 'fixtures/classes'
 describe "main#public" do
   after :each do
     Object.send(:private, :main_private_method)
+    Object.send(:private, :main_private_method2)
   end
 
-  it "sets the visibility of the given method to public" do
-    eval "public :main_private_method", TOPLEVEL_BINDING
-    Object.should_not have_private_method(:main_private_method)
+  context "when single argument is passed and it is not an array" do
+    it "sets the visibility of the given methods to public" do
+      eval "public :main_private_method", TOPLEVEL_BINDING
+      Object.should_not have_private_method(:main_private_method)
+    end
+  end
+
+  context "when multiple arguments are passed" do
+    it "sets the visibility of the given methods to public" do
+      eval "public :main_private_method, :main_private_method2", TOPLEVEL_BINDING
+      Object.should_not have_private_method(:main_private_method)
+      Object.should_not have_private_method(:main_private_method2)
+    end
+  end
+
+  ruby_version_is "3.0" do
+    context "when single argument is passed and is an array" do
+      it "sets the visibility of the given methods to public" do
+        eval "public [:main_private_method, :main_private_method2]", TOPLEVEL_BINDING
+        Object.should_not have_private_method(:main_private_method)
+        Object.should_not have_private_method(:main_private_method2)
+      end
+    end
   end
 
   it "returns Object" do
diff --git a/spec/ruby/core/module/alias_method_spec.rb b/spec/ruby/core/module/alias_method_spec.rb
index 742e289a3f..5d3d0c23d9 100644
--- a/spec/ruby/core/module/alias_method_spec.rb
+++ b/spec/ruby/core/module/alias_method_spec.rb
@@ -85,8 +85,19 @@ describe "Module#alias_method" do
     Module.should have_public_instance_method(:alias_method, false)
   end
 
-  it "returns self" do
-    @class.send(:alias_method, :checking_return_value, :public_one).should equal(@class)
+  describe "returned value" do
+    ruby_version_is ""..."3.0" do
+      it "returns self" do
+        @class.send(:alias_method, :checking_return_value, :public_one).should equal(@class)
+      end
+    end
+
+    ruby_version_is "3.0" do
+      it "returns symbol of the defined method name" do
+        @class.send(:alias_method, :checking_return_value, :public_one).should equal(:checking_return_value)
+        @class.send(:alias_method, 'checking_return_value', :public_one).should equal(:checking_return_value)
+      end
+    end
   end
 
   it "works in module" do
diff --git a/spec/ruby/core/module/attr_accessor_spec.rb b/spec/ruby/core/module/attr_accessor_spec.rb
index 6a749341be..665a9346bd 100644
--- a/spec/ruby/core/module/attr_accessor_spec.rb
+++ b/spec/ruby/core/module/attr_accessor_spec.rb
@@ -67,6 +67,22 @@ describe "Module#attr_accessor" do
     Module.should have_public_instance_method(:attr_accessor, false)
   end
 
+  ruby_version_is ""..."3.0" do
+    it "returns nil" do
+      Class.new do
+        (attr_accessor :foo, 'bar').should == nil
+      end
+    end
+  end
+
+  ruby_version_is "3.0" do
+    it "returns an array of defined methods names as symbols" do
+      Class.new do
+        (attr_accessor :foo, 'bar').should == [:foo, :foo=, :bar, :bar=]
+      end
+    end
+  end
+
   describe "on immediates" do
     before :each do
       class Fixnum
diff --git a/spec/ruby/core/module/attr_reader_spec.rb b/spec/ruby/core/module/attr_reader_spec.rb
index 238e3db9ea..e16b7ba2e3 100644
--- a/spec/ruby/core/module/attr_reader_spec.rb
+++ b/spec/ruby/core/module/attr_reader_spec.rb
@@ -61,4 +61,20 @@ describe "Module#attr_reader" do
   it "is a public method" do
     Module.should have_public_instance_method(:attr_reader, false)
   end
+
+  ruby_version_is ""..."3.0" do
+    it "returns nil" do
+      Class.new do
+        (attr_reader :foo, 'bar').should == nil
+      end
+    end
+  end
+
+  ruby_version_is "3.0" do
+    it "returns an array of defined methods names as symbols" do
+      Class.new do
+        (attr_reader :foo, 'bar').should == [:foo, :bar]
+      end
+    end
+  end
 end
diff --git a/spec/ruby/core/module/attr_spec.rb b/spec/ruby/core/module/attr_spec.rb
index 8b91e77658..060d072f27 100644
--- a/spec/ruby/core/module/attr_spec.rb
+++ b/spec/ruby/core/module/attr_spec.rb
@@ -145,4 +145,24 @@ describe "Module#attr" do
   it "is a public method" do
     Module.should have_public_instance_method(:attr, false)
   end
+
+  ruby_version_is ""..."3.0" do
+    it "returns nil" do
+      Class.new do
+        (attr :foo, 'bar').should == nil
+        (attr :baz, false).should == nil
+        (attr :qux, true).should == nil
+      end
+    end
+  end
+
+  ruby_version_is "3.0" do
+    it "returns an array of defined methods names as symbols" do
+      Class.new do
+        (attr :foo, 'bar').should == [:foo, :bar]
+        (attr :baz, false).should == [:baz]
+        (attr :qux, true).should == [:qux, :qux=]
+      end
+    end
+  end
 end
diff --git a/spec/ruby/core/module/attr_writer_spec.rb b/spec/ruby/core/module/attr_writer_spec.rb
index e4b193a9d8..59d8b7bf53 100644
--- a/spec/ruby/core/module/attr_writer_spec.rb
+++ b/spec/ruby/core/module/attr_writer_spec.rb
@@ -61,4 +61,20 @@ describe "Module#attr_writer" do
   it "is a public method" do
     Module.should have_public_instance_method(:attr_writer, false)
   end
+
+  ruby_version_is ""..."3.0" do
+    it "returns nil" do
+      Class.new do
+        (attr_writer :foo, 'bar').should == nil
+      end
+    end
+  end
+
+  ruby_version_is "3.0" do
+    it "returns an array of defined methods names as symbols" do
+      Class.new do
+        (attr_writer :foo, 'bar').should == [:foo=, :bar=]
+      end
+    end
+  end
 end
diff --git a/spec/ruby/core/module/shared/set_visibility.rb b/spec/ruby/core/module/shared/set_visibility.rb
index a04b1a54a0..9f31e230ca 100644
--- a/spec/ruby/core/module/shared/set_visibility.rb
+++ b/spec/ruby/core/module/shared/set_visibility.rb
@@ -6,6 +6,40 @@ describe :set_visibility, shared: true do
   end
 
   describe "with argument" do
+    describe "one or more arguments" do
+      it "sets visibility of given method names" do
+        visibility = @method
+        old_visibility = [:protected, :private].find {|vis| vis != visibility }
+
+        mod = Module.new {
+          send old_visibility
+          def test1() end
+          def test2() end
+          send visibility, :test1, :test2
+        }
+        mod.should send(:"have_#{visibility}_instance_method", :test1, false)
+        mod.should send(:"have_#{visibility}_instance_method", :test2, false)
+      end
+    end
+
+    ruby_version_is "3.0" do
+      describe "array as a single argument" do
+        it "sets visibility of given method names" do
+          visibility = @method
+          old_visibility = [:protected, :private].find {|vis| vis != visibility }
+
+          mod = Module.new {
+            send old_visibility
+            def test1() end
+            def test2() end
+            send visibility, [:test1, :test2]
+          }
+          mod.should send(:"have_#{visibility}_instance_method", :test1, false)
+          mod.should send(:"have_#{visibility}_instance_method", :test2, false)
+        end
+      end
+    end
+
     it "does not clone method from the ancestor when setting to the same visibility in a child" do
       visibility = @method
       parent = Module.new {
diff --git a/vm_method.c b/vm_method.c
index e526ee0130..e2c5bfb064 100644
--- a/vm_method.c
+++ b/vm_method.c
@@ -1941,13 +1941,13 @@ rb_alias(VALUE klass, ID alias_name, ID original_name)
 
 /*
  *  call-seq:
- *     alias_method(new_name, old_name)   -> self
+ *     alias_method(new_name, old_name)   -> symbol
  *
  *  Makes <i>new_name</i> a new copy of the method <i>old_name</i>. This can
  *  be used to retain access to methods that are overridden.
  *
  *     module Mod
- *       alias_method :orig_exit, :exit
+ *       alias_method :orig_exit, :exit #=> :orig_exit
  *       def exit(code=0)
  *         puts "Exiting with code #{code}"
  *         orig_exit(code)
@@ -1968,8 +1968,19 @@ rb_mod_alias_method(VALUE mod, VALUE newname, VALUE oldname)
     if (!oldid) {
 	rb_print_undef_str(mod, oldname);
     }
-    rb_alias(mod, rb_to_id(newname), oldid);
-    return mod;
+    VALUE id = rb_to_id(newname);
+    rb_alias(mod, id, oldid);
+    return ID2SYM(id);
+}
+
+static void
+check_and_export_method(VALUE self, VALUE name, rb_method_visibility_t visi)
+{
+    ID id = rb_check_id(&name);
+    if (!id) {
+	rb_print_undef_str(self, name);
+    }
+    rb_export_method(self, id, visi);
 }
 
 static void
@@ -1984,13 +1995,19 @@ set_method_visibility(VALUE self, int argc, const VALUE *argv, rb_method_visibil
 	return;
     }
 
-    for (i = 0; i < argc; i++) {
-	VALUE v = argv[i];
-	ID id = rb_check_id(&v);
-	if (!id) {
-	    rb_print_undef_str(self, v);
+
+    VALUE v;
+
+    if (argc == 1 && (v = rb_check_array_type(argv[0])) != Qnil) {
+        long j;
+
+	for (j = 0; j < RARRAY_LEN(v); j++) {
+	    check_and_export_method(self, RARRAY_AREF(v, j), visi);
 	}
-	rb_export_method(self, id, visi);
+    } else {
+        for (i = 0; i < argc; i++) {
+            check_and_export_method(self, argv[i], visi);
+        }
     }
 }
 
@@ -2012,6 +2029,7 @@ set_visibility(int argc, const VALUE *argv, VALUE module, rb_method_visibility_t
  *     public                 -> self
  *     public(symbol, ...)    -> self
  *     public(string, ...)    -> self
+ *     public(array)          -> self
  *
  *  With no arguments, sets the default visibility for subsequently
  *  defined methods to public. With arguments, sets the named methods to
@@ -2030,6 +2048,7 @@ rb_mod_public(int argc, VALUE *argv, VALUE module)
  *     protected                -> self
  *     protected(symbol, ...)   -> self
  *     protected(string, ...)   -> self
+ *     protected(array)         -> self
  *
  *  With no arguments, sets the default visibility for subsequently
  *  defined methods to protected. With arguments, sets the named methods
@@ -2057,6 +2076,7 @@ rb_mod_protected(int argc, VALUE *argv, VALUE module)
  *     private                 -> self
  *     private(symbol, ...)    -> self
  *     private(string, ...)    -> self
+ *     private(array)          -> self
  *
  *  With no arguments, sets the default visibility for subsequently
  *  defined methods to private. With arguments, sets the named methods