From: merch-redmine@...
Date: 2019-09-10T16:37:00+00:00
Subject: [ruby-core:94889] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?

Issue #16157 has been updated by jeremyevans0 (Jeremy Evans).


Note that the `RUBY_VERSION` check is only needed in a subset of the cases.  In cases where the target method does not accept keyword arguments, no changes are needed (no need to introduce keyword arguments at all).  In cases where the last positional argument is not a hash (and even in many of those cases), the simpler code that doesn't require `RUBY_VERSION` will work.  The only time you really need the `RUBY_VERSION` check is for complete argument delegation to arbitrary methods with arbitrary arguments.

I took a brief look at some things caught by your methodology.  I decided to look at ActiveSupport since that is one of the more popular gems.  Here are a few examples of delegation and a discussion of whether changes are needed.

```ruby
# lib/active_support/values/time_zone.rb
    def local(*args)
      time = Time.utc(*args)
      ActiveSupport::TimeWithZone.new(nil, self, time)
    end

    def at(*args)
      Time.at(*args).utc.in_time_zone(self)
    end
```

Most methods written in C do not care if they are called with keyword arguments or a positional hash argument and will work with either.  So these cases probably do not need to be updated.

```ruby
# lib/active_support/time_with_zone.rb
    def method_missing(sym, *args, &block)
      wrap_with_time_zone time.__send__(sym, *args, &block)
    rescue NoMethodError => e
      raise e, e.message.sub(time.inspect, inspect), e.backtrace
    end
```

This is pure delegation code to arbitrary methods.  However, I think `time` is an instance of `Time`, this will not require changes.  Now, users can define their own methods on Time in Ruby that accept keyword arguments, and if you want to handle that, you may want to update the code to handle keyword arguments.

```ruby
#lib/active_support/testing/setup_and_teardown.rb
        # Add a callback, which runs before <tt>TestCase#setup</tt>.
        def setup(*args, &block)
          set_callback(:setup, :before, *args, &block)
        end

        # Add a callback, which runs after <tt>TestCase#teardown</tt>.
        def teardown(*args, &block)
          set_callback(:teardown, :after, *args, &block)
        end
```

`set_callback` doesn't accept keyword arguments, it treats the positional arguments as an array.  So no changes are needed here.

```ruby
#lib/active_support/tagged_logging.rb
      def tagged(*tags)
        new_tags = push_tags(*tags)
        yield self
      ensure
        pop_tags(new_tags.size)
      end

    def tagged(*tags)
      formatter.tagged(*tags) { yield self }
    end
```

Here `tagged` at the bottom delegates all arguments to `tagged` at the top, which delegates all arguments to `push_tags`.  However, as `push_tags` does not accept keyword arguments, no changes are needed.

I tried your scanner using Sequel (in the 500 top gems).  143 matches in 63 files, none of which actually required changes.  There were a few changes needed in Sequel which the scanner didn't catch, involving passing option hashes to CSV using `**` instead of as a positional argument.

In general, I would not recommend using a lexical scanner to try to find cases to fix. Run the tests for the program/library, and fix any warnings.  Even for decent sized programs/libraries, it does not take very long to fix the related issues.  For many cases where you would have to fix issues, there are already other existing issues due to the implicit conversion between keyword and positional arguments.

----------------------------------------
Misc #16157: What is the correct and *portable* way to do generic delegation?
https://bugs.ruby-lang.org/issues/16157#change-81503

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
----------------------------------------
With the keyword argument changes in 2.7 we must now specify keyword arguments explicitly when doing generic delegation. But this change is not compatible with 2.6, where it adds an empty hash to the argument list of methods that do not need/accept keyword arguments.

To illustrate the problem:

```ruby
class ProxyWithoutKW < BasicObject
  def initialize(target)
    @target = target
  end
  def method_missing(*a, &b)
    @target.send(*a, &b)
  end
end

class ProxyWithKW < BasicObject
  def initialize(target)
    @target = target
  end
  def method_missing(*a, **o, &b)
    @target.send(*a, **o, &b)
  end
end

class Test
  def args(*a)   a  end
  def arg(a)     a  end
  def opts(**o)  o  end
end
                                          # 2.6        2.7              3.0
ProxyWithoutKW.new(Test.new).args(42)     # [42]       [42]             [42]        ok
ProxyWithoutKW.new(Test.new).arg(42)      # 42         42               42          ok
ProxyWithoutKW.new(Test.new).opts(k: 42)  # {:k=>42}   {:k=>42} +warn   [{:k=>42}]  incompatible with >= 2.7
ProxyWithKW.new(Test.new).args(42)        # [42, {}]   [42]             [42]        incompatible with <= 2.6
ProxyWithKW.new(Test.new).arg(42)         # error      42               42          incompatible with <= 2.6
ProxyWithKW.new(Test.new).opts(k: 42)     # {:k=>42}   {:k=>42} +warn   {:k=>42}    must ignore warning? cannot use pass_positional_hash in 2.6
```

I don't know how to solve this, so I'm asking for the **official** correct way to write portable delegation code. And by **portable** I mean code that can be used in gems that target ruby 2.6 and above.




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>