Feature #16411: Safer keyword argument extension

• Status: Open
• Assignee: -
• Target version: -

[Dan0042 (Daniel DeLorme)]

The problem of safe keyword extension has been described like this:

#We have an existing method
def debug(*args)
  args.each {|arg| puts arg.inspect }
end

#It can be invoked using the "braceless hash style"
debug(key: 42) #=> {:key=>42}

#Then, consider we improve (extend) the method to accept the output IO as a keyword parameter "output":
def debug(*args, output: $stdout)
  args.each {|arg| output.puts arg.inspect }
end

#However, this change breaks the existing call.
debug(key: 42) #=> ArgumentError (unknown keyword: key)

The best solution to this is currently seen as

def debug(*args, output: $stdout, **hash)
  args << hash unless hash.empty?
  args.each {|arg| output.puts arg.inspect }
end
debug(key: 42) #=> {:key=>42}

With the caveat that it doesn't work if the braceless hash has a key in common with the new keyword argument(s):

debug(input: "a", output: "A") #=> NoMethodError (private method `puts' called for "A":String)

That's a reasonable compromise, but I think it would be very unusual to mix positional arguments and keyword arguments into a single braceless hash. So in the case above, if hash is non-empty, it's quite likely that the output key should be part of it, and not interpreted as a keyword argument. In short I'm saying that:

debug({input: "a"})                #is likely
debug(input: "a")                  #is likely
debug({input: "a"}, output:STDERR) #is likely
debug(input: "a", output:STDERR)   #is unlikely, or at least very bad form

So following from that, I believe the safest way to extend a method with keyword arguments is like this:

def debug(*args, **hash)
  defaults = {output: $stdout}
  kw = defaults.merge(hash)
  if kw.size > defaults.size #hash has some key not in defaults
    args << hash
    kw = defaults
  end
  output = kw[:output]
  args.each {|arg| output.puts arg.inspect }
end
debug(key: 42)                                   #=> {:key=>42}
debug({input: "a", output: "A"})                 #=> {:input=>"a", :output=>"A"}
debug(input: "a", output: "A")                   #=> {:input=>"a", :output=>"A"}
debug({input: "a", output: "A"}, output: STDERR) #=> {:input=>"a", :output=>"A"}

Of course it doesn't handle debug(output: "A") but I don't think there's any generic way to handle that.

The solution above is obviously way too verbose and way too much boilerplate to be used as-is, and that's why I'd like ruby to provide some kind of facility to perform this operation. The above could be somewhat simplified via some kind of utility function like

def debug(*args, **hash)
  kw = kw_or_hash(hash, output: $output){ |hash| args << hash }
  output = kw[:output]
  args.each {|arg| output.puts arg.inspect }
end

but ideally this safe keyword extension mechanism would allow to keep the keywords definition inside the method definition, unlike the above.
This could perhaps be done via some special syntax like

def debug(*args, output: $stdout, **!?)

but I don't think yet more special-case syntax is an ideal solution either.
Maybe the ideal would be via meta-programming like

extended_keywords def debug(*args, output: $stdout)

but I'm not sure how that would work internally. Is it possible?

[shevegen (Robert A. Heiler)]

I don't think adding **!? would be a good idea. I'd also ideally keep
the complexity in ruby less, when possible; it's (to me) interesting
to see that keywords add to the complexity. Ideally it should be so
simple for everyone to understand that there would never be any confusion
about it. IMO that was one rationale why it was changed for ruby 3.0.

[Dan0042 (Daniel DeLorme)]

Yeah, **!? in itself was a joke placeholder intended to mean "insert funky new syntax here but I'm not sure what exactly"

It's true that one rationale of keyword argument separation was to allow safe keyword extension. This would have been possible with strict lexical separation but that was just too much of a breaking change for backward compatibility. So instead we have to deal with some amount of complexity.

[jeremyevans0 (Jeremy Evans)]

Dan0042 (Daniel DeLorme) wrote:

Yeah, **!? in itself was a joke placeholder intended to mean "insert funky new syntax here but I'm not sure what exactly"

It's true that one rationale of keyword argument separation was to allow safe keyword extension. This would have been possible with strict lexical separation but that was just too much of a breaking change for backward compatibility. So instead we have to deal with some amount of complexity.

Actually, if you want safe keyword extension, you can do so for new methods if you do not need backwards compatibility:

def debug(*args, **nil)
  args.each {|arg| puts arg.inspect }
end

The **nil here indicates the method does not accept keyword arguments, and trying to pass keywords to the method will raise an ArgumentError. This allows you to later add keyword arguments to the method without risking breakage of existing calls.

Note that in general, adding **nil to an existing method is a breaking change, at least if the method could be called with keyword arguments.

[Dan0042 (Daniel DeLorme)]

I'm aware of **nil and it's true that it provides fully safe keyword extension. But you have to admit it's not likely to become common practice amongst rubyists. You'd have to add it to every regular method in advance, just in case it might eventually accept keyword arguments; that's just too much boilerplate.

---

Thinking about it some more I think I reached something interesting:

class Module
  def safe_keyword_extension(**defined)
    name = defined.delete(:for) or raise ArgumentError, "must specify method with 'for'"
    original = instance_method(name)
    define_method(name) do |*args, **keywords, &b|
      unless keywords.all?{ |k,v| t = defined[k] and t === v }
        args << keywords
        keywords = {}
      end
      original.bind(self).call(*args, **keywords, &b)
    end
  end
end

module Kernel
  safe_keyword_extension output: IO, for:
  def debug(*args, output: $stdout)
    args.each {|arg| output.puts arg.inspect }
  end
end

debug(key: 42)                                   #=> {:key=>42}
debug({key: 42}, output: STDOUT)                 #=> {:key=>42}
debug({input: "a", output: "A"})                 #=> {:input=>"a", :output=>"A"}
debug(input: "a", output: "A")                   #=> {:input=>"a", :output=>"A"}
debug({input: "a", output: "A"}, output: STDERR) #=> {:input=>"a", :output=>"A"}
debug(output: "A")                               #=> {:output=>"A"}

By validating that the keywords given to the debug method are all part of the defined keywords AND of the correct type, we can even make it work for debug(output: "A") which I previously thought impossible.