Fix ruby 3.0 keyword arguments in translator #128
Conversation
|
Pretty sure we need mustermann to work in Ruby < 2.7 |
|
Passing locally in 2.6.10 through 3.1.2. |
|
This PR seems to lower the test coverage from 100% to 99.71%, see https://github.com/dentarg/mustermann/runs/7406755584?check_suite_focus=true#step:4:28 |
|
@dentarg without merging the coverage files, not sure the best way to cover the differing branches. I added |
|
True |
|
The change here get rids of the warnings ( Is this change correct though? @eregon @jeremyevans do you care to weigh in? |
|
Another way to fix (the warnings) would be something like this def self.translate(*types, &block)
Class.new(const_get(:NodeTranslator)) do
register(*types)
define_method(:translate, &block)
if block.parameters.flatten.any?(:args)
ruby2_keywords :translate
end
end
endbut I think this is not really a good solution as it is tied directly to the arg naming coming in with the block. |
|
This PR/change is incorrect at least on 2.7: https://eregon.me/blog/2021/02/13/correct-delegation-in-ruby-2-27-3.html What are the warnings, for which |
| @@ -73,7 +82,6 @@ def self.translate(*types, &block) | |||
| Class.new(const_get(:NodeTranslator)) do | |||
| register(*types) | |||
| define_method(:translate, &block) | |||
| ruby2_keywords :translate | |||
There was a problem hiding this comment.
My guess is this one is producing the warning, and it should probably be define_method(:translate, &block.ruby2_keywords) instead.
There was a problem hiding this comment.
Still seeing the warnings with define_method(:translate, &block.ruby2_keywords)
There was a problem hiding this comment.
Could you post the full warning output (it should show where it happens), and how to reproduce it?
There was a problem hiding this comment.
.../mustermann/mustermann/lib/mustermann/ast/translator.rb:75: warning: Skipping set of ruby2_keywords flag for proc (proc accepts keywords or proc does not accept argument splat)
You can see the full gamut in the test suite, as the class loading dumps a bunch of these.
| translate Array do |*args, **kwargs| | ||
| inject(t.pattern) do |pattern, element| | ||
| t.add_to(pattern, t(element, *args, **kwargs)) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Another possibility would be to call Proc#ruby2_keywords on this block which delegates *args (and other blocks doing the same).
That seems the cleanest solution.
|
I created #130 which I believe is the proper fix. |
|
@tconst I appreciate you helping work through this issue! |
|
@jkowens When can we expect patch on rubygems? Thanks in advance. |
|
Released v2.0.1 |
Removes
ruby2_keywordsadded in #126 and clears up the related console warnings in AST::Translator.This may not work in ruby < 2.7, untested at time of PR open.