Skip to content

Commit fd036db

Browse files
jeremyevansk0kubun
authored andcommitted
Do not respect ruby2_keywords on method/proc with post arguments
Previously, ruby2_keywords could be used on a method or proc with post arguments, but I don't think the behavior is desired: ```ruby def a(*c, **kw) [c, kw] end def b(*a, b) a(*a, b) end ruby2_keywords(:b) b({foo: 1}, bar: 1) ``` This changes ruby2_keywords to emit a warning and not set the flag on a method/proc with post arguments. While here, fix the ruby2_keywords specs for warnings, since they weren't testing what they should be testing. They all warned because the method didn't accept a rest argument, not because it accepted a keyword or keyword rest argument. [Backport #21402]
1 parent d49c7d0 commit fd036db

File tree

5 files changed

+62
-10
lines changed

5 files changed

+62
-10
lines changed

proc.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3958,12 +3958,13 @@ proc_ruby2_keywords(VALUE procval)
39583958
switch (proc->block.type) {
39593959
case block_type_iseq:
39603960
if (ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_rest &&
3961+
!ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_post &&
39613962
!ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_kw &&
39623963
!ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_kwrest) {
39633964
ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.ruby2_keywords = 1;
39643965
}
39653966
else {
3966-
rb_warn("Skipping set of ruby2_keywords flag for proc (proc accepts keywords or proc does not accept argument splat)");
3967+
rb_warn("Skipping set of ruby2_keywords flag for proc (proc accepts keywords or post arguments or proc does not accept argument splat)");
39673968
}
39683969
break;
39693970
default:

spec/ruby/core/module/ruby2_keywords_spec.rb

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ def obj.foo(a, b, c) end
275275

276276
it "prints warning when a method accepts keywords" do
277277
obj = Object.new
278-
def obj.foo(a:, b:) end
278+
def obj.foo(*a, b:) end
279279

280280
-> {
281281
obj.singleton_class.class_exec do
@@ -286,12 +286,25 @@ def obj.foo(a:, b:) end
286286

287287
it "prints warning when a method accepts keyword splat" do
288288
obj = Object.new
289-
def obj.foo(**a) end
289+
def obj.foo(*a, **b) end
290290

291291
-> {
292292
obj.singleton_class.class_exec do
293293
ruby2_keywords :foo
294294
end
295295
}.should complain(/Skipping set of ruby2_keywords flag for/)
296296
end
297+
298+
ruby_version_is "3.5" do
299+
it "prints warning when a method accepts post arguments" do
300+
obj = Object.new
301+
def obj.foo(*a, b) end
302+
303+
-> {
304+
obj.singleton_class.class_exec do
305+
ruby2_keywords :foo
306+
end
307+
}.should complain(/Skipping set of ruby2_keywords flag for/)
308+
end
309+
end
297310
end

spec/ruby/core/proc/ruby2_keywords_spec.rb

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,28 @@
3939
end
4040

4141
it "prints warning when a proc accepts keywords" do
42-
f = -> a:, b: { }
42+
f = -> *a, b: { }
4343

4444
-> {
4545
f.ruby2_keywords
4646
}.should complain(/Skipping set of ruby2_keywords flag for/)
4747
end
4848

4949
it "prints warning when a proc accepts keyword splat" do
50-
f = -> **a { }
50+
f = -> *a, **b { }
5151

5252
-> {
5353
f.ruby2_keywords
5454
}.should complain(/Skipping set of ruby2_keywords flag for/)
5555
end
56+
57+
ruby_version_is "3.5" do
58+
it "prints warning when a proc accepts post arguments" do
59+
f = -> *a, b { }
60+
61+
-> {
62+
f.ruby2_keywords
63+
}.should complain(/Skipping set of ruby2_keywords flag for/)
64+
end
65+
end
5666
end

test/ruby/test_keyword.rb

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2424,6 +2424,21 @@ class << c
24242424
assert_raise(ArgumentError) { m.call(42, a: 1, **h2) }
24252425
end
24262426

2427+
def test_ruby2_keywords_post_arg
2428+
def self.a(*c, **kw) [c, kw] end
2429+
def self.b(*a, b) a(*a, b) end
2430+
assert_warn(/Skipping set of ruby2_keywords flag for b \(method accepts keywords or post arguments or method does not accept argument splat\)/) do
2431+
assert_nil(singleton_class.send(:ruby2_keywords, :b))
2432+
end
2433+
assert_equal([[{foo: 1}, {bar: 1}], {}], b({foo: 1}, bar: 1))
2434+
2435+
b = ->(*a, b){a(*a, b)}
2436+
assert_warn(/Skipping set of ruby2_keywords flag for proc \(proc accepts keywords or post arguments or proc does not accept argument splat\)/) do
2437+
b.ruby2_keywords
2438+
end
2439+
assert_equal([[{foo: 1}, {bar: 1}], {}], b.({foo: 1}, bar: 1))
2440+
end
2441+
24272442
def test_proc_ruby2_keywords
24282443
h1 = {:a=>1}
24292444
foo = ->(*args, &block){block.call(*args)}
@@ -2436,8 +2451,8 @@ def test_proc_ruby2_keywords
24362451
assert_raise(ArgumentError) { foo.call(:a=>1, &->(arg, **kw){[arg, kw]}) }
24372452
assert_equal(h1, foo.call(:a=>1, &->(arg){arg}))
24382453

2439-
[->(){}, ->(arg){}, ->(*args, **kw){}, ->(*args, k: 1){}, ->(*args, k: ){}].each do |pr|
2440-
assert_warn(/Skipping set of ruby2_keywords flag for proc \(proc accepts keywords or proc does not accept argument splat\)/) do
2454+
[->(){}, ->(arg){}, ->(*args, x){}, ->(*args, **kw){}, ->(*args, k: 1){}, ->(*args, k: ){}].each do |pr|
2455+
assert_warn(/Skipping set of ruby2_keywords flag for proc \(proc accepts keywords or post arguments or proc does not accept argument splat\)/) do
24412456
pr.ruby2_keywords
24422457
end
24432458
end
@@ -2790,10 +2805,21 @@ def method_missing(*args)
27902805
assert_equal(:opt, o.clear_last_opt(a: 1))
27912806
assert_nothing_raised(ArgumentError) { o.clear_last_empty_method(a: 1) }
27922807

2793-
assert_warn(/Skipping set of ruby2_keywords flag for bar \(method accepts keywords or method does not accept argument splat\)/) do
2808+
assert_warn(/Skipping set of ruby2_keywords flag for bar \(method accepts keywords or post arguments or method does not accept argument splat\)/) do
27942809
assert_nil(c.send(:ruby2_keywords, :bar))
27952810
end
27962811

2812+
c.class_eval do
2813+
def bar_post(*a, x) = nil
2814+
define_method(:bar_post_bmethod) { |*a, x| }
2815+
end
2816+
assert_warn(/Skipping set of ruby2_keywords flag for bar_post \(method accepts keywords or post arguments or method does not accept argument splat\)/) do
2817+
assert_nil(c.send(:ruby2_keywords, :bar_post))
2818+
end
2819+
assert_warn(/Skipping set of ruby2_keywords flag for bar_post_bmethod \(method accepts keywords or post arguments or method does not accept argument splat\)/) do
2820+
assert_nil(c.send(:ruby2_keywords, :bar_post_bmethod))
2821+
end
2822+
27972823
utf16_sym = "abcdef".encode("UTF-16LE").to_sym
27982824
c.send(:define_method, utf16_sym, c.instance_method(:itself))
27992825
assert_warn(/abcdef/) do

vm_method.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2599,13 +2599,14 @@ rb_mod_ruby2_keywords(int argc, VALUE *argv, VALUE module)
25992599
switch (me->def->type) {
26002600
case VM_METHOD_TYPE_ISEQ:
26012601
if (ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.has_rest &&
2602+
!ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.has_post &&
26022603
!ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.has_kw &&
26032604
!ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.has_kwrest) {
26042605
ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.ruby2_keywords = 1;
26052606
rb_clear_method_cache(module, name);
26062607
}
26072608
else {
2608-
rb_warn("Skipping set of ruby2_keywords flag for %"PRIsVALUE" (method accepts keywords or method does not accept argument splat)", QUOTE_ID(name));
2609+
rb_warn("Skipping set of ruby2_keywords flag for %"PRIsVALUE" (method accepts keywords or post arguments or method does not accept argument splat)", QUOTE_ID(name));
26092610
}
26102611
break;
26112612
case VM_METHOD_TYPE_BMETHOD: {
@@ -2618,13 +2619,14 @@ rb_mod_ruby2_keywords(int argc, VALUE *argv, VALUE module)
26182619
const struct rb_captured_block *captured = VM_BH_TO_ISEQ_BLOCK(procval);
26192620
const rb_iseq_t *iseq = rb_iseq_check(captured->code.iseq);
26202621
if (ISEQ_BODY(iseq)->param.flags.has_rest &&
2622+
!ISEQ_BODY(iseq)->param.flags.has_post &&
26212623
!ISEQ_BODY(iseq)->param.flags.has_kw &&
26222624
!ISEQ_BODY(iseq)->param.flags.has_kwrest) {
26232625
ISEQ_BODY(iseq)->param.flags.ruby2_keywords = 1;
26242626
rb_clear_method_cache(module, name);
26252627
}
26262628
else {
2627-
rb_warn("Skipping set of ruby2_keywords flag for %"PRIsVALUE" (method accepts keywords or method does not accept argument splat)", QUOTE_ID(name));
2629+
rb_warn("Skipping set of ruby2_keywords flag for %"PRIsVALUE" (method accepts keywords or post arguments or method does not accept argument splat)", QUOTE_ID(name));
26282630
}
26292631
break;
26302632
}

0 commit comments

Comments
 (0)