Fix unexpected empty source map stacks #1625
Open
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.
Description
This fixes two unrelated issues that both previously presented with a similar symptom, namely the following error:
The first issue is that some constructs, such as
{} => {}
andfoo in bar
get parsed by Ripper as acase
AST node. However, without a matchingcase
keyword, the YARD parser doesn't know where it starts.The second issue is that tokens following the
def
keyword are (correctly) never treated as keywords themselves, because they are used as the method name instead. However, this logic erroneously also applied to keywords following the:def
symbol - again making the YARD parser unable to determine where the keyword (following the:def
symbol) starts.In both cases, the problematic constructs by themselves did not cause a crash but merely provided wrong source ranges. However, in specific parser states, the corresponding
@map
entry is the empty array rather than nil (because a similar, valid, construct was encountered before, which initializes the map entry). This was causing crashes since the beginning ofline_range
andsource_range
becomes nil as a result.Here we're fixing the underlying issue and we're also adding specs to ensure that source ranges are correct, and that crashes in the presence of pre-initializing constructs are eliminated.
Note that another fix would have been to treat empty
@map
entries the same as nil@map
entries invisit_event
. Doing so would also prevent the crashes, and improve robustness more generally, however it would mask potential similar problems with other constructs. Therefore I have refrained from making that change -- a debatable decision that might be revisited in a separate change. However, I've added a better error message in this case.DISCLOSURE: Claude wrote large parts of the specs and helped me understand the problems and create the fixes, but I'm fully adopting its output as my own (after polishing it manually and reviewing it.)
Fixes #1603.
Completed Tasks
bundle exec rake
locally (if code is attached to PR).