1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00

Warn for keyword to last hash parameter when method has no optional/rest parameters

Previously, there was no warning in this case, even though we will
be changing the behavior in Ruby 3.

Fixes [Bug #14130]
This commit is contained in:
Jeremy Evans 2019-08-30 19:23:10 -07:00
parent 6424d316b9
commit 3463e83192
3 changed files with 68 additions and 35 deletions

View file

@ -717,7 +717,9 @@ describe "A method" do
ruby ruby
m(1, b: 2).should == [1, 2] m(1, b: 2).should == [1, 2]
-> { m("a" => 1, b: 2) }.should raise_error(ArgumentError) suppress_keyword_warning.call do
-> { m("a" => 1, b: 2) }.should raise_error(ArgumentError)
end
end end
evaluate <<-ruby do evaluate <<-ruby do
@ -726,7 +728,9 @@ describe "A method" do
m(2).should == [2, 1] m(2).should == [2, 1]
m(1, b: 2).should == [1, 2] m(1, b: 2).should == [1, 2]
m("a" => 1, b: 2).should == [{"a" => 1, b: 2}, 1] suppress_keyword_warning.call do
m("a" => 1, b: 2).should == [{"a" => 1, b: 2}, 1]
end
end end
evaluate <<-ruby do evaluate <<-ruby do
@ -735,7 +739,9 @@ describe "A method" do
m(1).should == 1 m(1).should == 1
m(1, a: 2, b: 3).should == 1 m(1, a: 2, b: 3).should == 1
m("a" => 1, b: 2).should == {"a" => 1, b: 2} suppress_keyword_warning.call do
m("a" => 1, b: 2).should == {"a" => 1, b: 2}
end
end end
evaluate <<-ruby do evaluate <<-ruby do
@ -744,7 +750,9 @@ describe "A method" do
m(1).should == [1, {}] m(1).should == [1, {}]
m(1, a: 2, b: 3).should == [1, {a: 2, b: 3}] m(1, a: 2, b: 3).should == [1, {a: 2, b: 3}]
m("a" => 1, b: 2).should == [{"a" => 1, b: 2}, {}] suppress_keyword_warning.call do
m("a" => 1, b: 2).should == [{"a" => 1, b: 2}, {}]
end
end end
evaluate <<-ruby do evaluate <<-ruby do

View file

@ -22,7 +22,9 @@ class TestKeywordArguments < Test::Unit::TestCase
def test_f2 def test_f2
assert_equal([:xyz, "foo", 424242], f2(:xyz)) assert_equal([:xyz, "foo", 424242], f2(:xyz))
assert_equal([{"bar"=>42}, "foo", 424242], f2("bar"=>42)) assert_warn(/The keyword argument for `f2' .* is passed as the last hash parameter/) do
assert_equal([{"bar"=>42}, "foo", 424242], f2("bar"=>42))
end
end end
@ -322,6 +324,10 @@ class TestKeywordArguments < Test::Unit::TestCase
end end
end end
def req_plus_keyword(x, **h)
[x, h]
end
def opt_plus_keyword(x=1, **h) def opt_plus_keyword(x=1, **h)
[x, h] [x, h]
end end
@ -331,6 +337,19 @@ class TestKeywordArguments < Test::Unit::TestCase
end end
def test_keyword_split def test_keyword_split
assert_warn(/The keyword argument for `req_plus_keyword' .* is passed as the last hash parameter/) do
assert_equal([{:a=>1}, {}], req_plus_keyword(:a=>1))
end
assert_warn(/The keyword argument for `req_plus_keyword' .* is passed as the last hash parameter/) do
assert_equal([{"a"=>1}, {}], req_plus_keyword("a"=>1))
end
assert_warn(/The keyword argument for `req_plus_keyword' .* is passed as the last hash parameter/) do
assert_equal([{"a"=>1, :a=>1}, {}], req_plus_keyword("a"=>1, :a=>1))
end
assert_equal([{:a=>1}, {}], req_plus_keyword({:a=>1}))
assert_equal([{"a"=>1}, {}], req_plus_keyword({"a"=>1}))
assert_equal([{"a"=>1, :a=>1}, {}], req_plus_keyword({"a"=>1, :a=>1}))
assert_equal([1, {:a=>1}], opt_plus_keyword(:a=>1)) assert_equal([1, {:a=>1}], opt_plus_keyword(:a=>1))
assert_equal([1, {"a"=>1}], opt_plus_keyword("a"=>1)) assert_equal([1, {"a"=>1}], opt_plus_keyword("a"=>1))
assert_equal([1, {"a"=>1, :a=>1}], opt_plus_keyword("a"=>1, :a=>1)) assert_equal([1, {"a"=>1, :a=>1}], opt_plus_keyword("a"=>1, :a=>1))
@ -536,7 +555,9 @@ class TestKeywordArguments < Test::Unit::TestCase
[a, b, c, d, e, f, g] [a, b, c, d, e, f, g]
end end
end end
assert_equal([1, 2, 1, [], {:f=>5}, 2, {}], a.new.foo(1, 2, f:5), bug8993) assert_warn(/The keyword argument for `foo' .* is passed as the last hash parameter/) do
assert_equal([1, 2, 1, [], {:f=>5}, 2, {}], a.new.foo(1, 2, f:5), bug8993)
end
end end
def test_splat_keyword_nondestructive def test_splat_keyword_nondestructive

View file

@ -756,41 +756,45 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
if (kw_flag & VM_CALL_KW_SPLAT) { if (kw_flag & VM_CALL_KW_SPLAT) {
kw_splat = !iseq->body->param.flags.has_rest; kw_splat = !iseq->body->param.flags.has_rest;
} }
if (given_argc > min_argc && if ((iseq->body->param.flags.has_kw || iseq->body->param.flags.has_kwrest ||
(iseq->body->param.flags.has_kw || iseq->body->param.flags.has_kwrest ||
(kw_splat && given_argc > max_argc)) && (kw_splat && given_argc > max_argc)) &&
args->kw_argv == NULL) { args->kw_argv == NULL) {
if (((kw_flag & (VM_CALL_KWARG | VM_CALL_KW_SPLAT)) || !ec->cfp->iseq /* called from C */)) { if (given_argc > min_argc) {
int check_only_symbol = (kw_flag & VM_CALL_KW_SPLAT) && if (((kw_flag & (VM_CALL_KWARG | VM_CALL_KW_SPLAT)) || !ec->cfp->iseq /* called from C */)) {
iseq->body->param.flags.has_kw && int check_only_symbol = (kw_flag & VM_CALL_KW_SPLAT) &&
!iseq->body->param.flags.has_kwrest; iseq->body->param.flags.has_kw &&
!iseq->body->param.flags.has_kwrest;
if (args_pop_keyword_hash(args, &keyword_hash, check_only_symbol)) { if (args_pop_keyword_hash(args, &keyword_hash, check_only_symbol)) {
given_argc--; given_argc--;
}
else if (check_only_symbol) {
if (keyword_hash != Qnil) {
rb_warn_split_last_hash_to_keyword(calling, ci);
} }
else { else if (check_only_symbol) {
rb_warn_keyword_to_last_hash(calling, ci); if (keyword_hash != Qnil) {
rb_warn_split_last_hash_to_keyword(calling, ci);
}
else {
rb_warn_keyword_to_last_hash(calling, ci);
}
} }
} }
} else if (args_pop_keyword_hash(args, &keyword_hash, 1)) {
else if (args_pop_keyword_hash(args, &keyword_hash, 1)) { /* Warn the following:
/* Warn the following: * def foo(k:1) p [k]; end
* def foo(k:1) p [k]; end * foo({k:42}) #=> 42
* foo({k:42}) #=> 42 */
*/ if (ec->cfp->iseq) {
if (ec->cfp->iseq) { /* called from Ruby level */
/* called from Ruby level */ rb_warn_last_hash_to_keyword(calling, ci);
rb_warn_last_hash_to_keyword(calling, ci); }
} given_argc--;
given_argc--; }
} else if (keyword_hash != Qnil && ec->cfp->iseq) {
else if (keyword_hash != Qnil && ec->cfp->iseq) { rb_warn_split_last_hash_to_keyword(calling, ci);
rb_warn_split_last_hash_to_keyword(calling, ci); }
} }
else if (given_argc == min_argc && kw_flag) {
rb_warn_keyword_to_last_hash(calling, ci);
}
} }
if (given_argc > max_argc && max_argc != UNLIMITED_ARGUMENTS) { if (given_argc > max_argc && max_argc != UNLIMITED_ARGUMENTS) {