Add support for ruby 2.7 **nil syntax

[Morriar]

Motivation

Ruby 2.7 introduces the no-keyword-arguments syntax **nil.

You can use **nil in a method definition to explicitly mark that the method accepts no keyword arguments. Calling such methods with keyword arguments will result in an ArgumentError.

def foo(*args, **nil)
end


foo(k: 1)
  #=> Ruby 2.7 or later: no keywords accepted (ArgumentError)

This pull request makes Sorbet support the **nil syntax and raise errors when keyword arguments are passed to methods that use it. See the many examples in test/testdata/infer/kwnil.rb.

Approach

I wanted to avoid touching the way arguments were handled just for **nil, so I figured the easiest way was to just remove it completely from the args list.

1. During parsing, a node is created for **nil and added to the arguments list
2. During desugar, the node is removed from the args list and the method definition is tagged as using **nil
3. During namer, we move that information from the method definition to the symbol so it can be accessed during typing
4. Finally, during typing, that information is used to raise an error for any call with keyword arguments to a method symbol tagged **nil (I reused the MethodArgumentCountMismatch error)

**nil in procs and lambda

With Ruby 2.7, you can also use **nil in procs and lambdas:

m = ->(a, **nil) {}
m.call(10, b: 20) # ArgumentError (no keywords accepted)

While this PR changes the parser so it does not create a syntax error, the **nil is still not checked in procs and lambdas.
We can store the information in the block definition but then the call is a special case with Proc.call and I'm not sure how to get back the **nil tag from the block at this point.

Test plan

See included automated tests.

This PR can be read commit by commit.

[jez]

It looks to me like this test case is allowed in Ruby 2.7:

def foo(x, **nil)
  p x
end

x = {hello: :world}
foo(x)

but I think after the changes in this PR Sorbet will reject this code, right?

[jez]

@Morriar Are you down to land the changes to expand the parser and the changes to make use of the new **nil syntax downstream in infer separately? I have a couple of reasons:

• We don't like adding more bits to ast nodes (like the isKwnil bit that was added here)
• We don't like adding new flags to the symbol table (like the isMethodWitKwnil flag here)
• I don't think the change as implemented current models Ruby particularly well (it emits too many errors than it should)

The **nil seems to be me to be a property of a method's arity / arguments, but we're modeling it as a bit on a method symbol, not stored anywhere near the arguments. It might be that's the best place for it, but I'm not sure at a glance, and I think the parser changes should be enough to be permissive with this new syntax, and we can take more time to think about what the semantics should be.

[Morriar]

@Morriar Are you down to land the changes to expand the parser and the changes to make use of the new **nil syntax downstream in infer separately?

Sure, I opened #3412 with only the parser bits.

To be honest, Sorbet already handles those errors through the signature validation. The infer part of this PR is mostly to catch those errors earlier and display a different error message.

The **nil seems to be me to be a property of a method's arity / arguments, but we're modeling it as a bit on a method symbol, not stored anywhere near the arguments.

For me it felt more like a property of the method symbol itself than something related to a particular argument. The syntax ties it to an arg but it if we had to write a Sorbet signature builder for it we would most likely have added it the same way we did for abstract or override.

[Morriar]

Keeping this PR open until we figure out what to do with the infer part.

[jez]

Keeping this PR open until we figure out what to do with the infer part.

Sounds good to me. Just chatted with @elliottt about this, and it's starting to sound like we'll have to significantly revamp how Sorbet treats keyword args, at which point supporting **nil would get a lot easier.

So I think we're good to keep this open until then.

Another thing so I don't forget it:

Trevor and I were discussing an idea where instead of having the isKwnil flag on the ast::MethodDef, we desugar a parser::Kwnilarg node to ast::RestArg { ast::KeywordArg { ast::Local { name = <nil> } } }, i.e., use a special variable name, and keep it as a parameter in the tree. (just for future reference)

Thanks again for getting the parser support broken out into a separate PR.