Make TEnumBlock behave as a scope

[Morriar]

To represent T::Enum subclasses like this one:

class MyEnum < T::Enum
  enums do
    # Some comments
    A = new
    B = new
  end
end

We currently use a special node called TEnumBlock that only accepts an array of names representing the constants like A, B. On top of being unnecessarily restrictive and complex, it doesn't allow storing the comments attached to each enum constant.

In reality, enums do .. end is nothing more than a block that can accept constants but also other types of nodes. This PR changes the superclass of TEnumBlock to be a Tree that can contain any kind of nodes including Const to represent the A, B etc. This means we can now attach comments to the constants and parse the TEnum properly.

This is a breaking change as we change the API to build TEnum which may break some clients.

[paracycle]

The changes look fine to me, but I can't understand how one is supposed to extract all enum members from the resulting TEnumBlock object? Do you need to find all the constants defined in that scope with the value new?

[paracycle]

Shouldn't we be checking that the enums call has actually been supplied a block and no arguments?

[Morriar]

Good call, I added code to handle malformed enums calls as arbitrary sends. I also added a few tests around this case.

[Morriar]

The changes look fine to me, but I can't understand how one is supposed to extract all enum members from the resulting TEnumBlock object? Do you need to find all the constants defined in that scope with the value new?

Good point, I added a TEnum#members method to make gathering the enum constants super easily:

enum = TEnum.new("Enum") do |enum|
  enum << TEnumBlock.new do |block|
    block << Const.new("A", "new")
    block << Const.new("B", "new")
  end
end

assert_equal(["A", "B"], enum.members)

[egiurleo]

Just to make sure I'm understanding -- if arguments are passed in, we treat enums as a regular method call? (Which makes sense to me)

[paracycle]

Yes:

class Foo < T::Enum
  enums("A", "B", "C")
end

should just be a call to the enums class method on Foo (whatever that may be).

[paracycle]

What about cases where we might have:

class Foo < T::Enum
  enums do
    A = new("A")
  end
end

Is this not a concern for RBI files, I wonder?

[Morriar]

You're right, it should find these ones too.

I updated the implementation and test to accept any constant definition that matches /^new(\(.*\))?$/

[paracycle]

Sorry if this is pedantic, but I think we need to consider cases without parens as well:

A = new "String"

[Morriar]

Following our conversation earlier I removed the members helper as we don't have a clean way to ensure the RBI::Const is actually a valid T::Enum member.

Let's revisit this once we need it 👍