Fix Prism crash with missing class/module name #757
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sorbet was crashing in Prism parser mode was when encountering malformed class or module definitions where the constant path is missing or invalid, such as:
or
or
For the first example, Prism generates a ClassNode with constant_path set to a MissingNode and a null body.
For the second example, Prism generates a ClassNode with the constant_path set as the first DefNode and a null body.
For the third example, Prism generates a ClassNode with the constant_path set as the first DefNode and the body is a StatementsNode containing the second DefNode.
The original parser throws away the class/module node and only includes the body node (if present) in the tree. So in the third example it would just be the
barDefNode. With the Prism parser it's fairly straightforward (easier?) for us to always return the class/module node, filling in a missing constant for the name, and the same body (e.g. thebarDefNode), so only the invalid constant path node is discarded.We considered changing how Prism parses these examples to always return a class/module node with a MissingNode for the constant_path and all children in the body, but it's not always obvious that that's the right way to parse it. For example:
In these examples it wouldn't make sense to parse them as class nodes with a body containing
(return)::Aor0.X—it's more likely the intention was that these are constant paths, not body nodes. As such, I think it makes sense to just work with the existing Prism behavior and do the best we can with it.Another option was to copy the mimic parser behavior exactly (throwing away the outer class/module) but since it's easier for us to keep it, I went with the approach in this commit.