Feature #16955
closedA new mode `Warning[:keyword_into_rest_arg] = true` for 2.7
Description
Problem¶
Please see #16954. This ticket is for discussing the second solution.
Proposal¶
The problem is caused by automatic conversion from keywords to positional arguments, especially, when the keyword is implicitly absorbed into rest arguments.
def app_proxy(*args)
# args.last is a normal (non-ruby2_keyword-flagged) Hash, { :k => 42 }, converted from the keyword "k: 42"
...
end
app_proxy(k: 42) # this call is troublesome
So I propose a new mode Warning[:keyword_into_rest_arg] = true
to notice such an event.
1: Warning[:keyword_into_rest_arg] = true
2:
3: def app_proxy(*args)
4: end
5:
6: app_proxy(k: 42)
# t.rb:6: warning: The keyword argument is absorbed into the rest parameter
# t.rb:3: warning: The called method `app_proxy' is defined here
An experimental patch is here. (The new mode is enabled by default in the patch just to make the experiment easy.)
Important note: false positives¶
Unfortunately, this brings many false positives. Typically, the following code is completely innocent and will work great in 3.0, but is warned:
1: def foo(*args)
2: opt = args.pop if args.last.is_a?(Hash)
3: ...
4: end
5:
6: foo(k: 42)
# t.rb:6: warning: The keyword argument is absorbed into the rest parameter
# t.rb:1: warning: The called method `foo' is defined here
By an experiment with make test-all
, tons of warnings are observed. Note that duplicated warings are already filtered out. (Some tests generates a code dynamically, which makes the duplication filter not work.)
Many warnings occur within standard libraries. I think that it is possible to fix these warnings, but we need to introduce tons of fixes in stdlib of 2.7.2. I'm afraid if it is acceptable.
However, @Eregon (Benoit Daloze) said that it might be still usable with combination of a dedicated warning filter mechanism (such as deprecation_toolkit).
Discussion points¶
- Is this really helpful?
- We need to add this change into Ruby 2.7.2. Is it acceptable? (Matz had already agreed with this change. But after that, it turned out that it brings so many false positives.)
- Is the name
:keyword_into_rest_arg
okay? - Should the commandline option
-W:keyword_into_rest_arg
be added?