Fix aref_field bracket consumption causing invalid source_range - #1642
Merged
Conversation
Fixes lsegal#1420 The issue occurred when parsing code with an aref assignment (a[:b] = 1) followed by a constant assignment using aref (X = a[:b]). The parser would create an invalid source_range for the constant's aref node, where the ending position was less than the beginning position. Root Cause: ----------- When Ripper encounters an aref on the left-hand side of an assignment (e.g., a[:b] = 1), it calls on_aref_field instead of on_aref. The on_aref_field method was not consuming the closing bracket position from @Map[:aref], which tracks bracket positions. This left the bracket position from the assignment in the map. When the next aref expression (X = a[:b]) was parsed, on_aref would shift the old bracket position from @Map[:aref], causing it to use the wrong ending position and create an invalid source_range where end < begin. The Fix: -------- Updated on_aref_field to match the behavior of on_aref: - Consume the closing bracket position from @Map[:aref] using shift - Use the bracket position to correctly calculate the source_range - Set both source_range and line_range based on the actual bracket position This ensures that when on_aref_field processes a[:b] = 1, it consumes the bracket, so when on_aref processes X = a[:b], it gets the correct bracket position and creates a valid source_range. Changes: -------- - Modified on_aref_field in lib/yard/parser/ruby/ruby_parser.rb to consume bracket positions from @Map[:aref] and calculate source_range correctly - Added spec to verify constant assignments with aref after aref assignments parse correctly with valid source_range The fix ensures both on_aref and on_aref_field handle bracket consumption consistently, preventing the parser from getting confused by previous aref assignments.
Owner
|
Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Fixes #1420
I caused the above regression four years ago and am proposing a fix here. This PR was heavily AI-assisted.
The issue occurred when parsing code with an
arefassignment (a[:b] = 1) followed by a constant assignment usingaref(X = a[:b]). The parser would create an invalidsource_rangefor the constant'sarefnode, where the ending position was less than the beginning position.Root Cause:
When Ripper encounters an aref on the left-hand side of an assignment (e.g.,
a[:b] = 1), it callson_aref_fieldinstead ofon_aref. Theon_aref_fieldmethod was not consuming the closing bracket position from@map[:aref], which tracks bracket positions. This left the bracket position from the assignment in the map.When the next aref expression (
X = a[:b]) was parsed,on_arefwould shift the old bracket position from@map[:aref], causing it to use the wrong ending position and create an invalidsource_rangewhereend<begin.The Fix:
Updated on_aref_field to match the behavior of
on_aref:@map[:aref]using shiftsource_rangesource_rangeandline_rangebased on the actual bracket positionThis ensures that when
on_aref_fieldprocessesa[:b] = 1, it consumes the bracket, so whenon_arefprocessesX = a[:b], it gets the correct bracket position and creates a valid source_range.Changes:
on_aref_fieldinlib/yard/parser/ruby/ruby_parser.rbto consume bracket positions from@map[:aref]and calculate source_range correctlyarefafterarefassignments parse correctly with validsource_rangeThe fix ensures both
on_arefandon_aref_fieldhandle bracket consumption consistently, preventing the parser from getting confused by previousarefassignments.Completed Tasks
bundle exec rakelocally (if code is attached to PR).