Skip to content

Commit 82cf606

Browse files
committed
Fix error message when there are extra keyword arguments
List all the extra keyword argument instead of the first one.
1 parent c5b753a commit 82cf606

File tree

3 files changed

+54
-12
lines changed

3 files changed

+54
-12
lines changed

spec/ruby/language/keyword_arguments_spec.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ def m(*a, kw:)
5858
m(kw: 1).should == []
5959
-> { m(kw: 1, kw2: 2) }.should raise_error(ArgumentError, 'unknown keyword: :kw2')
6060
-> { m(kw: 1, true => false) }.should raise_error(ArgumentError, 'unknown keyword: true')
61+
-> { m(kw: 1, a: 1, b: 2, c: 3) }.should raise_error(ArgumentError, 'unknown keywords: :a, :b, :c')
6162
end
6263

6364
it "handle * and ** at the same call site" do

src/main/java/org/truffleruby/core/exception/CoreExceptions.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,18 @@ public RubyException argumentErrorCantOmitPrecision(Node currentNode) {
181181
}
182182

183183
@TruffleBoundary
184-
public RubyException argumentErrorUnknownKeyword(Object key, Node currentNode) {
185-
return argumentError("unknown keyword: " + inspect(key), currentNode);
184+
public RubyException argumentErrorUnknownKeywords(Object[] keys, Node currentNode) {
185+
if (keys.length == 1) {
186+
return argumentError("unknown keyword: " + inspect(keys[0]), currentNode);
187+
}
188+
189+
final String[] names = new String[keys.length];
190+
191+
for (int i = 0; i < keys.length; i++) {
192+
names[i] = inspect(keys[i]);
193+
}
194+
195+
return argumentError("unknown keywords: " + String.join(", ", names), currentNode);
186196
}
187197

188198
@TruffleBoundary

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

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import com.oracle.truffle.api.CompilerDirectives;
1313
import org.truffleruby.RubyLanguage;
14+
import org.truffleruby.core.exception.RubyException;
1415
import org.truffleruby.core.hash.RubyHash;
1516
import org.truffleruby.core.hash.library.HashStoreLibrary;
1617
import org.truffleruby.core.hash.library.HashStoreLibrary.EachEntryCallback;
@@ -20,16 +21,22 @@
2021
import org.truffleruby.language.methods.Arity;
2122

2223
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
24+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
2325
import com.oracle.truffle.api.frame.VirtualFrame;
2426
import com.oracle.truffle.api.nodes.ExplodeLoop;
2527
import com.oracle.truffle.api.profiles.BranchProfile;
2628

29+
import java.util.ArrayList;
30+
import java.util.Arrays;
31+
import java.util.List;
32+
import java.util.stream.Collectors;
33+
2734
/** Check that no extra keyword arguments are given, when there is no **kwrest */
2835
public class CheckKeywordArityNode extends RubyBaseNode {
2936

3037
public final Arity arity;
3138
@Child private ReadUserKeywordsHashNode readUserKeywordsHashNode;
32-
@Child private CheckKeywordArgumentsNode checkKeywordArgumentsNode;
39+
@Child private CheckExtraKeywordArgumentsNode checkExtraKeywordArgumentsNode;
3340

3441
public CheckKeywordArityNode(Arity arity) {
3542
assert !arity.hasKeywordsRest() : "no need to create this node";
@@ -45,37 +52,37 @@ public void checkArity(VirtualFrame frame) {
4552
}
4653

4754
private void checkKeywordArguments(RubyHash keywordArguments) {
48-
if (checkKeywordArgumentsNode == null) {
55+
if (checkExtraKeywordArgumentsNode == null) {
4956
CompilerDirectives.transferToInterpreterAndInvalidate();
50-
checkKeywordArgumentsNode = insert(new CheckKeywordArgumentsNode(getLanguage(), arity));
57+
checkExtraKeywordArgumentsNode = insert(new CheckExtraKeywordArgumentsNode(getLanguage(), arity));
5158
}
52-
checkKeywordArgumentsNode.check(keywordArguments);
59+
60+
checkExtraKeywordArgumentsNode.check(keywordArguments);
5361
}
5462

55-
private static class CheckKeywordArgumentsNode extends RubyBaseNode implements EachEntryCallback {
63+
private static class CheckExtraKeywordArgumentsNode extends RubyBaseNode implements EachEntryCallback {
5664

5765
@CompilationFinal(dimensions = 1) private final RubySymbol[] allowedKeywords;
5866

5967
private final BranchProfile unknownKeywordProfile = BranchProfile.create();
6068
@Child private HashStoreLibrary hashes;
6169

62-
public CheckKeywordArgumentsNode(RubyLanguage language, Arity arity) {
70+
public CheckExtraKeywordArgumentsNode(RubyLanguage language, Arity arity) {
6371
assert !arity.hasKeywordsRest();
6472
hashes = HashStoreLibrary.createDispatched();
6573
allowedKeywords = keywordsAsSymbols(language, arity);
6674
}
6775

6876
public void check(RubyHash keywordArguments) {
69-
hashes.eachEntry(keywordArguments.store, keywordArguments, this, null);
77+
hashes.eachEntry(keywordArguments.store, keywordArguments, this, keywordArguments);
7078
}
7179

7280
@Override
7381
public void accept(int index, Object key, Object value, Object state) {
7482
if (!keywordAllowed(key)) {
7583
unknownKeywordProfile.enter();
76-
throw new RaiseException(
77-
getContext(),
78-
coreExceptions().argumentErrorUnknownKeyword(key, this));
84+
RubyHash keywordArguments = (RubyHash) state;
85+
throw new RaiseException(getContext(), unknownKeywordsError(keywordArguments));
7986
}
8087
}
8188

@@ -89,6 +96,30 @@ private boolean keywordAllowed(Object keyword) {
8996

9097
return false;
9198
}
99+
100+
@TruffleBoundary
101+
private RubyException unknownKeywordsError(RubyHash keywordArguments) {
102+
Object[] keys = findExtraKeywordArguments(keywordArguments);
103+
return coreExceptions().argumentErrorUnknownKeywords(keys, this);
104+
}
105+
106+
@TruffleBoundary
107+
@SuppressWarnings("unchecked")
108+
private Object[] findExtraKeywordArguments(RubyHash keywordArguments) {
109+
final ArrayList<Object> actualKeywordsAsList = new ArrayList<>();
110+
hashes.eachEntry(
111+
keywordArguments.store,
112+
keywordArguments,
113+
(index, key, value, state) -> ((ArrayList<Object>) state).add(key),
114+
actualKeywordsAsList);
115+
116+
final List<RubySymbol> allowedKeywordsAsList = Arrays.asList(allowedKeywords);
117+
final List<Object> extraKeywords = actualKeywordsAsList.stream()
118+
.filter(k -> !allowedKeywordsAsList.contains(k))
119+
.collect(Collectors.toList());
120+
121+
return extraKeywords.toArray();
122+
}
92123
}
93124

94125
static RubySymbol[] keywordsAsSymbols(RubyLanguage language, Arity arity) {

0 commit comments

Comments
 (0)