Skip to content

Conversation

jscheid
Copy link

@jscheid jscheid commented Aug 25, 2025

Description

This fixes two unrelated issues that both previously presented with a similar symptom, namely the following error:

RangeError: cannot get the first element of beginless range

The first issue is that some constructs, such as {} => {} and foo in bar get parsed by Ripper as a case AST node. However, without a matching case 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 of line_range and source_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 in visit_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

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I wrote tests and ran bundle exec rake locally (if code is attached to PR).

@jscheid jscheid marked this pull request as draft August 25, 2025 18:27
@jscheid jscheid force-pushed the first-el-of-beginless-range branch 2 times, most recently from ecd8785 to 537ae6c Compare August 25, 2025 19:31
This fixes two unrelated issues that both previously presented with a
similar symptom, namely the following error:

RangeError: cannot get the first element of beginless range

The first issue is that some constructs, such as `{} => {}` and `foo
in bar` get parsed by Ripper as a `case` AST node. However, without a
matching `case` 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 of `line_range` and `source_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 in `visit_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 lsegal#1603.

Co-Authored-By: Claude <[email protected]>
@jscheid jscheid force-pushed the first-el-of-beginless-range branch from 537ae6c to c6b0b16 Compare August 25, 2025 19:41
@jscheid jscheid marked this pull request as ready for review August 25, 2025 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

YARD rake task crash
1 participant