Fixes #4186. JRuby accepts wrong method arguments when mixing positional

with defaults and keywords.

StaticScopes now keep track of where keyword arguments start by index.
This is missing a regression test which I will add in next commit.

core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java

@@ -597,13 +597,9 @@ public static void checkForExtraUnwantedKeywordArgs(ThreadContext context, final
     private static final RubyHash.VisitorWithState<StaticScope> CheckUnwantedKeywordsVisitor = new RubyHash.VisitorWithState<StaticScope>() {
         @Override
         public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, StaticScope scope) {
-            String keyAsString = key.asJavaString();
-            int slot = scope.isDefined((keyAsString));
-
-            // Found name in higher variable scope.  Therefore non for this block/method def.
-            if ((slot >> 16) > 0) throw context.runtime.newArgumentError("unknown keyword: " + keyAsString);
-            // Could not find it anywhere.
-            if (((short) (slot & 0xffff)) < 0) throw context.runtime.newArgumentError("unknown keyword: " + keyAsString);
+            if (!scope.keywordExists(key.asJavaString())) {
+                throw context.runtime.newArgumentError("unknown keyword: " + key.asJavaString());
+            }
         }
     };
 

core/src/main/java/org/jruby/parser/ParserSupport.java

@@ -167,9 +167,14 @@ public AssignableNode assignableLabelOrIdentifier(String name, Node value) {
         return currentScope.assign(lexer.getPosition(), name.intern(), makeNullNil(value));
     }
 
+    // We know it has to be tLABEL or tIDENTIFIER so none of the other assignable logic is needed
+    public AssignableNode assignableKeyword(String name, Node value) {
+        return currentScope.assignKeyword(lexer.getPosition(), name.intern(), makeNullNil(value));
+    }
+
     // Only calls via f_kw so we know it has to be tLABEL
     public AssignableNode assignableLabel(String name, Node value) {
-        return currentScope.assign(lexer.getPosition(), name, makeNullNil(value));
+        return currentScope.assignKeyword(lexer.getPosition(), name, makeNullNil(value));
     }
 
     protected void getterIdentifierError(ISourcePosition position, String identifier) {

core/src/main/java/org/jruby/parser/RubyParser.java

@@ -5070,26 +5070,26 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
 states[575] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
                     lexer.setCurrentArg(null);
-                    yyVal = support.keyword_arg(((Node)yyVals[0+yyTop]).getPosition(), support.assignableLabelOrIdentifier(((String)yyVals[-1+yyTop]), ((Node)yyVals[0+yyTop])));
+                    yyVal = support.keyword_arg(((Node)yyVals[0+yyTop]).getPosition(), support.assignableKeyword(((String)yyVals[-1+yyTop]), ((Node)yyVals[0+yyTop])));
     return yyVal;
   }
 };
 states[576] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
                     lexer.setCurrentArg(null);
-                    yyVal = support.keyword_arg(lexer.getPosition(), support.assignableLabelOrIdentifier(((String)yyVals[0+yyTop]), new RequiredKeywordArgumentValueNode()));
+                    yyVal = support.keyword_arg(lexer.getPosition(), support.assignableKeyword(((String)yyVals[0+yyTop]), new RequiredKeywordArgumentValueNode()));
     return yyVal;
   }
 };
 states[577] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
-                    yyVal = support.keyword_arg(support.getPosition(((Node)yyVals[0+yyTop])), support.assignableLabelOrIdentifier(((String)yyVals[-1+yyTop]), ((Node)yyVals[0+yyTop])));
+                    yyVal = support.keyword_arg(support.getPosition(((Node)yyVals[0+yyTop])), support.assignableKeyword(((String)yyVals[-1+yyTop]), ((Node)yyVals[0+yyTop])));
     return yyVal;
   }
 };
 states[578] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
-                    yyVal = support.keyword_arg(lexer.getPosition(), support.assignableLabelOrIdentifier(((String)yyVals[0+yyTop]), new RequiredKeywordArgumentValueNode()));
+                    yyVal = support.keyword_arg(lexer.getPosition(), support.assignableKeyword(((String)yyVals[0+yyTop]), new RequiredKeywordArgumentValueNode()));
     return yyVal;
   }
 };

core/src/main/java/org/jruby/parser/RubyParser.y

@@ -2370,18 +2370,18 @@ f_label 	: tLABEL {
 
 f_kw            : f_label arg_value {
                     lexer.setCurrentArg(null);
-                    $$ = support.keyword_arg($2.getPosition(), support.assignableLabelOrIdentifier($1, $2));
+                    $$ = support.keyword_arg($2.getPosition(), support.assignableKeyword($1, $2));
                 }
                 | f_label {
                     lexer.setCurrentArg(null);
-                    $$ = support.keyword_arg(lexer.getPosition(), support.assignableLabelOrIdentifier($1, new RequiredKeywordArgumentValueNode()));
+                    $$ = support.keyword_arg(lexer.getPosition(), support.assignableKeyword($1, new RequiredKeywordArgumentValueNode()));
                 }
 
 f_block_kw      : f_label primary_value {
-                    $$ = support.keyword_arg(support.getPosition($2), support.assignableLabelOrIdentifier($1, $2));
+                    $$ = support.keyword_arg(support.getPosition($2), support.assignableKeyword($1, $2));
                 }
                 | f_label {
-                    $$ = support.keyword_arg(lexer.getPosition(), support.assignableLabelOrIdentifier($1, new RequiredKeywordArgumentValueNode()));
+                    $$ = support.keyword_arg(lexer.getPosition(), support.assignableKeyword($1, new RequiredKeywordArgumentValueNode()));
                 }
 
 

core/src/main/java/org/jruby/parser/StaticScope.java

@@ -36,6 +36,7 @@
 import org.jruby.ast.AssignableNode;
 import org.jruby.ast.DAsgnNode;
 import org.jruby.ast.DVarNode;
+import org.jruby.ast.IScopedNode;
 import org.jruby.ast.LocalAsgnNode;
 import org.jruby.ast.LocalVarNode;
 import org.jruby.ast.Node;
@@ -97,6 +98,8 @@ public class StaticScope implements Serializable {
 
     private long commandArgumentStack;
 
+    private int firstKeywordIndex = -1;
+
     // Method/Closure that this static scope corresponds to.  This is used to tell whether this
     // scope refers to a method scope or to determined IRScope of the parent of a compiling eval.
     private IRScope irScope;
@@ -353,6 +356,30 @@ public AssignableNode assign(ISourcePosition position, String name, Node value)
         return assign(position, name, value, this, 0);
     }
 
+    /**
+     * Register a keyword argument with this staticScope.  It additionally will track
+     * where the first keyword argument started so we can test and tell whether we have
+     * a kwarg or an ordinary variable during live execution (See keywordExists).
+     * @param position
+     * @param name
+     * @param value
+     * @return
+     */
+    public AssignableNode assignKeyword(ISourcePosition position, String name, Node value) {
+        AssignableNode assignment = assign(position, name, value, this, 0);
+
+        // register first keyword index encountered
+        if (firstKeywordIndex == -1) firstKeywordIndex = ((IScopedNode) assignment).getIndex();
+
+        return assignment;
+    }
+
+    public boolean keywordExists(String name) {
+        int slot = exists(name);
+
+        return slot >= 0 && firstKeywordIndex != -1 && slot >= firstKeywordIndex;
+    }
+
     /**
      * Get all visible variables that we can see from this scope that have been assigned
      * (e.g. seen so far)