[GR-45621] Fix {Method,Proc}#parameters method

PullRequest: truffleruby/4161

CHANGELOG.md

@@ -54,6 +54,7 @@ Compatibility:
 * Limit maximum encoding set size by 256 (#3039, @thomasmarshall, @goyox86).
 * Remove deprecated methods `Dir.exists?`, `File.exists?`, and `Kernel#=~` (#3039, @patricklinpl, @nirvdrum).
 * Remove deprecated `FileTest.exists?` method (#3039, @andrykonchin).
+* Fix {Method,Proc}#parameters and return `*`, `**` and `&` names for anonymous parameters (@andrykonchin).
 
 Performance:
 

spec/ruby/core/method/parameters_spec.rb

@@ -22,6 +22,8 @@ def one_splat_one_block(*args, &block)
       local_is_not_parameter = {}
     end
 
+    def forward_parameters(...) end
+
     def underscore_parameters(_, _, _ = 1, *_, _:, _: 2, **_, &_); end
 
     define_method(:one_optional_defined_method) {|x = 1|}
@@ -267,6 +269,20 @@ def object.foo(&)
     end
   end
 
+  ruby_version_is ""..."3.1" do
+    it "returns [:rest, :*], [:block, :&] for forward parameters operator" do
+      m = MethodSpecs::Methods.new
+      m.method(:forward_parameters).parameters.should == [[:rest, :*], [:block, :&]]
+    end
+  end
+
+  ruby_version_is "3.1" do
+    it "returns [:rest, :*], [:keyrest, :**], [:block, :&] for forward parameters operator" do
+      m = MethodSpecs::Methods.new
+      m.method(:forward_parameters).parameters.should == [[:rest, :*], [:keyrest, :**], [:block, :&]]
+    end
+  end
+
   it "returns the args and block for a splat and block argument" do
     m = MethodSpecs::Methods.new
     m.method(:one_splat_one_block).parameters.should == [[:rest, :args], [:block, :block]]

spec/ruby/core/proc/parameters_spec.rb

@@ -170,4 +170,8 @@
       [:block, :_]
     ]
   end
+
+  it "returns :nokey for **nil parameter" do
+    proc { |**nil| }.parameters.should == [[:nokey]]
+  end
 end

spec/tags/library/socket/tcpsocket/initialize_tags.txt

@@ -5,3 +5,4 @@ fails:TCPSocket#initialize using IPv6 when a server is listening on the given ad
 fails:TCPSocket#initialize using IPv6 when a server is listening on the given address creates a socket which is set to close on exec
 slow:TCPSocket#initialize raises IO::TimeoutError with :connect_timeout when no server is listening on the given address
 slow:TCPSocket#initialize with a running server connects to a server when passed connect_timeout argument
+fails(transient):TCPSocket#initialize raises IO::TimeoutError with :connect_timeout when no server is listening on the given address

src/main/java/org/truffleruby/core/proc/RubyProc.java

@@ -123,7 +123,8 @@ public int getArityNumber() {
     }
 
     public ArgumentDescriptor[] getArgumentDescriptors() {
-        return argumentDescriptors == null ? arity.toAnonymousArgumentDescriptors() : argumentDescriptors;
+        // parameters can be unnamed in a Proc created using Symbol#to_proc
+        return argumentDescriptors == null ? arity.toUnnamedArgumentDescriptors() : argumentDescriptors;
     }
 
     // region SourceLocation

src/main/java/org/truffleruby/core/symbol/SymbolNodes.java

@@ -226,7 +226,7 @@ public static RootCallTarget createCallTarget(RubyLanguage language, RubySymbol
                     0,
                     "&:" + symbol.getString(),
                     "Symbol#to_proc",
-                    ArgumentDescriptor.AT_LEAST_ONE);
+                    ArgumentDescriptor.AT_LEAST_ONE_UNNAMED);
 
             // ModuleNodes.DefineMethodNode relies on the lambda CallTarget to always use a RubyLambdaRootNode,
             // and we want to use a single CallTarget for both proc and lambda.

src/main/java/org/truffleruby/language/arguments/ArgumentDescriptorUtils.java

@@ -15,11 +15,15 @@
 import org.truffleruby.RubyLanguage;
 import org.truffleruby.core.array.ArrayHelpers;
 import org.truffleruby.core.array.RubyArray;
+import org.truffleruby.core.symbol.RubySymbol;
 import org.truffleruby.parser.ArgumentDescriptor;
 import org.truffleruby.parser.ArgumentType;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
-import org.truffleruby.parser.TranslatorEnvironment;
+
+import static org.truffleruby.parser.TranslatorEnvironment.FORWARDED_BLOCK_NAME;
+import static org.truffleruby.parser.TranslatorEnvironment.FORWARDED_KEYWORD_REST_NAME;
+import static org.truffleruby.parser.TranslatorEnvironment.FORWARDED_REST_NAME;
 
 public final class ArgumentDescriptorUtils {
 
@@ -44,16 +48,22 @@ private static RubyArray toArray(RubyLanguage language, RubyContext context, Arg
 
     private static RubyArray toArray(RubyLanguage language, RubyContext context, ArgumentType argType, String name) {
         final Object[] store;
+        final RubySymbol typeSymbol = language.getSymbol(argType.symbolicName);
 
-        if (argType.anonymous || name == null) {
-            store = new Object[]{ language.getSymbol(argType.symbolicName) };
+        if (argType == ArgumentType.anonrest) {
+            store = new Object[]{ typeSymbol, language.coreSymbols.MULTIPLY };
+        } else if (argType == ArgumentType.anonkeyrest) {
+            store = new Object[]{ typeSymbol, language.coreSymbols.POW };
+        } else if (argType == ArgumentType.rest && name.equals(FORWARDED_REST_NAME)) {
+            store = new Object[]{ typeSymbol, language.coreSymbols.MULTIPLY };
+        } else if (argType == ArgumentType.keyrest && name.equals(FORWARDED_KEYWORD_REST_NAME)) {
+            store = new Object[]{ typeSymbol, language.coreSymbols.POW };
+        } else if (argType == ArgumentType.block && name.equals(FORWARDED_BLOCK_NAME)) {
+            store = new Object[]{ typeSymbol, language.coreSymbols.AMPERSAND };
+        } else if (argType.anonymous || name == null) {
+            store = new Object[]{ typeSymbol };
         } else {
-            // make sure to normalize parameter names to "_" if they start with "_$"
-            if (name.startsWith(TranslatorEnvironment.UNDERSCORE_PREFIX)) {
-                name = "_";
-            }
-
-            store = new Object[]{ language.getSymbol(argType.symbolicName), language.getSymbol(name) };
+            store = new Object[]{ typeSymbol, language.getSymbol(name) };
         }
 
         return ArrayHelpers.createArray(context, language, store);

src/main/java/org/truffleruby/language/methods/Arity.java

@@ -164,31 +164,33 @@ public String[] getRequiredKeywordArguments() {
         return requiredKeywords;
     }
 
-    public ArgumentDescriptor[] toAnonymousArgumentDescriptors() {
+    /** Generate argument descriptors for a method/proc that doesn't provide parameter names, e.g a core method
+     * implemented in Java. */
+    public ArgumentDescriptor[] toUnnamedArgumentDescriptors() {
         List<ArgumentDescriptor> descs = new ArrayList<>();
 
         for (int i = 0; i < preRequired; i++) {
-            descs.add(new ArgumentDescriptor(ArgumentType.anonreq));
+            descs.add(new ArgumentDescriptor(ArgumentType.unnamedreq));
         }
 
         for (int i = 0; i < optional; i++) {
-            descs.add(new ArgumentDescriptor(ArgumentType.anonopt));
+            descs.add(new ArgumentDescriptor(ArgumentType.unnamedopt));
         }
 
         if (hasRest) {
-            descs.add(new ArgumentDescriptor(ArgumentType.anonrest));
+            descs.add(new ArgumentDescriptor(ArgumentType.unnamedrest));
         }
 
         for (int i = 0; i < postRequired; i++) {
-            descs.add(new ArgumentDescriptor(ArgumentType.anonreq));
+            descs.add(new ArgumentDescriptor(ArgumentType.unnamedreq));
         }
 
         for (String keyword : keywordArguments) {
             descs.add(new ArgumentDescriptor(ArgumentType.key, keyword));
         }
 
         if (hasKeywordsRest) {
-            descs.add(new ArgumentDescriptor(ArgumentType.anonkeyrest));
+            descs.add(new ArgumentDescriptor(ArgumentType.unnamedkeyrest));
         }
 
         return descs.toArray(ArgumentDescriptor.EMPTY_ARRAY);

src/main/java/org/truffleruby/language/methods/SharedMethodInfo.java

@@ -91,7 +91,7 @@ public SharedMethodInfo convertMethodMissingToMethod(RubyModule declaringModule,
                 blockDepth,
                 moduleAndMethodName(declaringModule, methodName),
                 notes,
-                ArgumentDescriptor.ANY);
+                ArgumentDescriptor.ANY_UNNAMED);
     }
 
     public SourceSection getSourceSection() {
@@ -116,7 +116,7 @@ public ArgumentDescriptor[] getRawArgumentDescriptors() {
     }
 
     public ArgumentDescriptor[] getArgumentDescriptors() {
-        return argumentDescriptors == null ? arity.toAnonymousArgumentDescriptors() : argumentDescriptors;
+        return argumentDescriptors == null ? arity.toUnnamedArgumentDescriptors() : argumentDescriptors;
     }
 
     public boolean isBlock() {

src/main/java/org/truffleruby/parser/ArgumentDescriptor.java

@@ -36,10 +36,10 @@ public final class ArgumentDescriptor {
     /** The name of the argument */
     public final String name;
 
-    public static final ArgumentDescriptor[] ANY = { new ArgumentDescriptor(ArgumentType.anonrest) };
-    public static final ArgumentDescriptor[] AT_LEAST_ONE = {
-            new ArgumentDescriptor(ArgumentType.anonreq),
-            new ArgumentDescriptor(ArgumentType.anonrest) };
+    public static final ArgumentDescriptor[] ANY_UNNAMED = { new ArgumentDescriptor(ArgumentType.unnamedrest) };
+    public static final ArgumentDescriptor[] AT_LEAST_ONE_UNNAMED = {
+            new ArgumentDescriptor(ArgumentType.unnamedreq),
+            new ArgumentDescriptor(ArgumentType.unnamedrest) };
     public static final ArgumentDescriptor[] EMPTY_ARRAY = new ArgumentDescriptor[0];
 
     public ArgumentDescriptor(ArgumentType type, String name) {

src/main/java/org/truffleruby/parser/ArgumentType.java

@@ -26,20 +26,44 @@
  ***** END LICENSE BLOCK *****/
 package org.truffleruby.parser;
 
+/** Reflects semantic of different method/proc declared parameters types.
+ *
+ * Symbolic names match type names returned by the {Method, Proc}#parameters methods. */
 public enum ArgumentType {
 
+    /** optional keyword argument */
     key("key", false),
+    /** required keyword argument */
     keyreq("keyreq", false),
+    /** keyword rest parameter */
     keyrest("keyrest", false),
+    /** block parameter */
     block("block", false),
+    /** optional positional parameter */
     opt("opt", false),
+    /** rest parameter */
     rest("rest", false),
+    /** required positional parameter */
     req("req", false),
+
+    /* Parameters declared in a method/proc explicitly without name, e.g. anonymous *, ** , and &. Required parameter
+     * can be anonymous only in case of parameters nesting (e.g. `def foo(a, (b, c)) end`) */
     anonreq("req", true),
-    anonopt("opt", true),
     anonrest("rest", true),
     anonkeyrest("keyrest", true),
-    nokey("nokey", true); // **nil
+
+    /* Parameters in a method that doesn't provide parameter names, e.g. implemented using #method_missing or a core
+     * method implemented in Java.
+     * 
+     * A tiny difference between unnamed and anonymous parameters is that anonymous are reported by the #parameters
+     * method with names (*, ** or &). But unnamed are reported without names. */
+    unnamedreq("req", true),
+    unnamedopt("opt", true),
+    unnamedrest("rest", true),
+    unnamedkeyrest("keyrest", true),
+
+    /** no-keyword-arguments parameter (**nil) */
+    nokey("nokey", true);
 
     ArgumentType(String symbolicName, boolean anonymous) {
         this.symbolicName = symbolicName;

src/main/java/org/truffleruby/parser/TranslatorEnvironment.java

@@ -56,11 +56,11 @@ public final class TranslatorEnvironment {
     static final String DEFAULT_KEYWORD_REST_NAME = Layouts.TEMP_PREFIX + "kwrest";
 
     /** local variable name for * parameter caused by desugaring ... parameter (forward-everything) */
-    static final String FORWARDED_REST_NAME = Layouts.TEMP_PREFIX + "forward_rest";
+    public static final String FORWARDED_REST_NAME = Layouts.TEMP_PREFIX + "forward_rest";
     /** local variable name for ** parameter caused by desugaring ... parameter (forward-everything) */
-    static final String FORWARDED_KEYWORD_REST_NAME = Layouts.TEMP_PREFIX + "forward_kwrest";
+    public static final String FORWARDED_KEYWORD_REST_NAME = Layouts.TEMP_PREFIX + "forward_kwrest";
     /** local variable name for & parameter caused by desugaring ... parameter (forward-everything) */
-    static final String FORWARDED_BLOCK_NAME = Layouts.TEMP_PREFIX + "forward_block";
+    public static final String FORWARDED_BLOCK_NAME = Layouts.TEMP_PREFIX + "forward_block";
 
     /** A prefix for duplicated '_' local variables to build unique names */
     public static final String UNDERSCORE_PREFIX = "_$";