Allow ArgsNode to iterate over its arguments

[dvandersluis]

While working on rubocop/rubocop#9071, I needed a way to evaluate block arguments (and get their names) that are inside destructuring, but BlockNode#arguments returns the mlhs node instead of the actual arguments. This change adds each_arg to ArgsNode so that only argument types can be iterated, and created a Node type (ArgNode) for the 7 arg types, which exposes name, default_value, etc. I added some predicates that I think are useful too.

I overrode ArgsNode#to_a so that arguments.size will return the actual number of arguments when they are not all at the same node depth.

Finally, this adds BlockNode#argument_names which is convenient for me in rubocop/rubocop#9071. It gives a way to collect all the argument names even when there are numbered params (for which BlockNode#arguments just returns a bare array).

[marcandre]

Thank you for the great PR. I meant to do something like that for a while too. I'm definitely 👍 for the creation of ArgNode.

One thing to keep in mind is numblock. There's a distinction between the number of children in the AST node that isn't always clean. I had started a PR but then realized that my implementation was not a good idea. Anyways, it is not necessary for this PR though, but we must keep that in mind.

As far as the interface goes, I'd prefer to keep to_a intact and provide a new method to access the information.
Instead of each_arg, and names how about only argument_list? We could also make it accessible from BlockNode + NumblockNode.

[dvandersluis]

@marcandre sounds good about argument_list, I'll make that change. The only issue there IMO is that for foo { |x, (y, z)| }, arguments.size will still be 2, but arguments_list.size will be 3 so that's okay.

[marcandre]

@marcandre sounds good about argument_list, I'll make that change. The only issue there IMO is that for foo { |x, (y, z)| }, arguments.size will still be 2, but arguments_list.size will be 3 so that's okay.

Exactly. And I'm thinking { _1 + _2 } should have arguments.size be 0, but maybe arguments? should be true and argument_list.size be 2.

[dvandersluis]

@marcandre updated to use argument_list instead of the previous solution.

I'm not really sure I understand what a procarg0 is, but it seems a block with 1 argument is parsed to it with modernize mode, so I added a class for it as well. It couldn't just reuse ArgNode because the arg node is its child, so name is overridden. I think we probably can do something similar for ForwardArgsNode, but since it's a collection I didn't want to change it in case I was overlooking something. Instead, I made the specs that may parse to a forward_args conditional on RuboCop::AST::Builder.emit_forward_arg.

And I'm thinking { _1 + _2 } should have arguments.size be 0, but maybe arguments? should be true and argument_list.size be 2.

I didn't do this for this PR but I'll try to get this working next.

[marcandre]

I'm not really sure I understand what a procarg0

It is to distinguish foo {|x| ... } and foo {|x,| ... } which behave a bit differently and were parsed the same way. That distinction rarely affects rubocop so we still use the legacy format by default.

so I added a class for it as well.

Perfect. It's a valid question as to why the format is like this though, I'll ask.

[marcandre]

I like where this is going!

[dvandersluis]

@marcandre I wasn't sure if procarg0 should be added to node_types.adoc? I don't think any of the "modern" node types are documented there, but now we have a class for it. (I did add shadowarg which was previously missing)

[marcandre]

Looks pretty much ready to merge.
One last small detail, could we return frozen arrays? We might want to cache this in the future, and there shouldn't be a need to modify this result.

[marcandre]

Great work, thanks!