Skip to content

Commit acb29f7

Browse files
jeremyevansjeremyevans
committed
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.
1 parent b0c80c2 commit acb29f7

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
@@ -4012,12 +4012,13 @@ proc_ruby2_keywords(VALUE procval)
40124012
switch (proc->block.type) {
40134013
case block_type_iseq:
40144014
if (ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_rest &&
4015+
!ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_post &&
40154016
!ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_kw &&
40164017
!ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_kwrest) {
40174018
ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.ruby2_keywords = 1;
40184019
}
40194020
else {
4020-
rb_warn("Skipping set of ruby2_keywords flag for proc (proc accepts keywords or proc does not accept argument splat)");
4021+
rb_warn("Skipping set of ruby2_keywords flag for proc (proc accepts keywords or post arguments or proc does not accept argument splat)");
40214022
}
40224023
break;
40234024
default:

‎spec/ruby/core/module/ruby2_keywords_spec.rb‎

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

214214
it "prints warning when a method accepts keywords" do
215215
obj = Object.new
216-
def obj.foo(a:, b:) end
216+
def obj.foo(*a, b:) end
217217

218218
-> {
219219
obj.singleton_class.class_exec do
@@ -224,12 +224,25 @@ def obj.foo(a:, b:) end
224224

225225
it "prints warning when a method accepts keyword splat" do
226226
obj = Object.new
227-
def obj.foo(**a) end
227+
def obj.foo(*a, **b) end
228228

229229
-> {
230230
obj.singleton_class.class_exec do
231231
ruby2_keywords :foo
232232
end
233233
}.should complain(/Skipping set of ruby2_keywords flag for/)
234234
end
235+
236+
ruby_version_is "3.5" do
237+
it "prints warning when a method accepts post arguments" do
238+
obj = Object.new
239+
def obj.foo(*a, b) end
240+
241+
-> {
242+
obj.singleton_class.class_exec do
243+
ruby2_keywords :foo
244+
end
245+
}.should complain(/Skipping set of ruby2_keywords flag for/)
246+
end
247+
end
235248
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
@@ -2959,13 +2959,14 @@ rb_mod_ruby2_keywords(int argc, VALUE *argv, VALUE module)
29592959
switch (me->def->type) {
29602960
case VM_METHOD_TYPE_ISEQ:
29612961
if (ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.has_rest &&
2962+
!ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.has_post &&
29622963
!ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.has_kw &&
29632964
!ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.has_kwrest) {
29642965
ISEQ_BODY(me->def->body.iseq.iseqptr)->param.flags.ruby2_keywords = 1;
29652966
rb_clear_method_cache(module, name);
29662967
}
29672968
else {
2968-
rb_warn("Skipping set of ruby2_keywords flag for %"PRIsVALUE" (method accepts keywords or method does not accept argument splat)", QUOTE_ID(name));
2969+
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));
29692970
}
29702971
break;
29712972
case VM_METHOD_TYPE_BMETHOD: {
@@ -2978,13 +2979,14 @@ rb_mod_ruby2_keywords(int argc, VALUE *argv, VALUE module)
29782979
const struct rb_captured_block *captured = VM_BH_TO_ISEQ_BLOCK(procval);
29792980
const rb_iseq_t *iseq = rb_iseq_check(captured->code.iseq);
29802981
if (ISEQ_BODY(iseq)->param.flags.has_rest &&
2982+
!ISEQ_BODY(iseq)->param.flags.has_post &&
29812983
!ISEQ_BODY(iseq)->param.flags.has_kw &&
29822984
!ISEQ_BODY(iseq)->param.flags.has_kwrest) {
29832985
ISEQ_BODY(iseq)->param.flags.ruby2_keywords = 1;
29842986
rb_clear_method_cache(module, name);
29852987
}
29862988
else {
2987-
rb_warn("Skipping set of ruby2_keywords flag for %"PRIsVALUE" (method accepts keywords or method does not accept argument splat)", QUOTE_ID(name));
2989+
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));
29882990
}
29892991
break;
29902992
}

0 commit comments

Comments
 (0)