Fix crash when module/class `constant_path` is invalid node

[thomasmarshall]

module
end

Prism parses this as a ModuleNode where constant_path is a MissingNode and body is null:

Prism.parse "module\nend"
=>
#<Prism::ParseResult:0x000000012756abf8
 @comments=[],
 @data_loc=nil,
 @errors=
  [#<Prism::ParseError @type=:module_name @message="unexpected constant path after `module`; class/module name must be CONSTANT" @location=#<Prism::Location @start_offset=0 @length=6 start_line=1> @level=:syntax>,
   #<Prism::ParseError @type=:expect_eol_after_statement @message="unexpected 'end', expecting end-of-input" @location=#<Prism::Location @start_offset=7 @length=3 start_line=2> @level=:syntax>,
   #<Prism::ParseError @type=:unexpected_token_ignore @message="unexpected 'end', ignoring it" @location=#<Prism::Location @start_offset=7 @length=3 start_line=2> @level=:syntax>],
 @magic_comments=[],
 @source=#<Prism::ASCIISource:0x000000012754a088 @offsets=[0, 7], @source="module\nend", @start_line=1>,
 @value=
  @ ProgramNode (location: (1,0)-(1,6))
  ├── flags: ∅
  ├── locals: []
  └── statements:
      @ StatementsNode (location: (1,0)-(1,6))
      ├── flags: ∅
      └── body: (length: 2)
          ├── @ ModuleNode (location: (1,0)-(1,6))
          │   ├── flags: newline
          │   ├── locals: []
          │   ├── module_keyword_loc: (1,0)-(1,6) = "module"
          │   ├── constant_path:
          │   │   @ MissingNode (location: (1,0)-(1,6))
          │   │   └── flags: ∅
          │   ├── body: ∅
          │   ├── end_keyword_loc: (1,6)-(1,6) = ""
          │   └── name: :""
          └── @ MissingNode (location: (1,6)-(1,6))
              └── flags: newline,

The original parser throws it away.

module
  def foo; end
end

Prism parses this as a ModuleNode where constant_path is a DefNode and body is null. The original parser throws it away.

module
  def foo; end
  def bar; end
end

Prism parses this as a ModuleNode where constant_path is the first DefNode and body is a StatementsNode containing the second DefNode. The original parser throws the module node and first def node away, returning only the second def node.

How should we handle this in Sorbet? I came up with a few options:

1. Match the original parser, throwing away the class/module and first child node, returning just the body (bar in this case)
2. Return a class/module node with constant missing name and reconstruct a body containing all children
3. Return an empty class/module node, throwing away the constant_path and body

The first option is tempting because it matches the original behavior, but can we do better since Prism error recovery is better?

The second option seems like we're working around how Prism chose to interpret this invalid code. Perhaps in our example it feels obvious that this should be a module node with a missing constant path node and two def nodes in the body, but in other cases it might not be quite so straightforward?

The third option to me seems like the most sensible—throwing away everything except the module shell node. However, this is different behavior from the current parser.

[thomasmarshall]

WIP branch sorbet/sorbet@master...Shopify:sorbet:tm-fix-prism-module-class-constant-crash

[thomasmarshall]

We'll consider fixing this upstream in ruby/prism.

[thomasmarshall]

@amomchilov these are the examples Prism has:

class (return)::A; end
       ^~~~~~ unexpected void value expression

class 0.X end
      ^~~ unexpected constant path after `class`; class/module name must be CONSTANT

module Parent module end
              ^~~~~~ unexpected constant path after `module`; class/module name must be CONSTANT
                     ^~~ unexpected 'end', assuming it is closing the parent module definition

I think in these cases it's not quite so obvious how the parser should handle them. For examples 1 and 2, they both look like they are intended to be constant paths, rather than intended to be part of the class body. In that case, it wouldn't make sense to set constant_path as a MissingNode and put (return)::A in the class body, for example.

I suppose Prism could do something different depending on whether it looks like the node is intended to be a constant path (but invalid) vs intended to be part of the body, but maybe that's a bit fuzzy.

Might it be preferable for us to stick with the current parser behavior and handle it in Sorbet the same way the original parser does (throw away everything except the body)? It's not ideal, but matching the existing behavior would allow us to move forward with something we know is at least acceptable.