Skip to content

Commit 4dc66fc

Browse files
authored
Fix aref_field bracket consumption causing invalid source_range (#1642)
# 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 `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. # Completed Tasks - [x] I have read the [Contributing Guide][contrib]. - [x] The pull request is complete (implemented / written). - [x] Git commits have been cleaned up (squash WIP / revert commits). - [x] I wrote tests and ran `bundle exec rake` locally (if code is attached to PR). [contrib]: https://github.com/lsegal/yard/blob/main/CONTRIBUTING.md
2 parents 34796c5 + 8898f69 commit 4dc66fc

File tree

2 files changed

+54
-2
lines changed

2 files changed

+54
-2
lines changed

lib/yard/parser/ruby/ruby_parser.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,10 @@ def on_aref(*args)
398398

399399
def on_aref_field(*args)
400400
@map[:lbracket].pop
401-
AstNode.new(:aref_field, args,
402-
:listline => lineno..lineno, :listchar => charno...charno)
401+
ll, lc = *@map[:aref].shift
402+
sr = args.first.source_range.begin..lc
403+
lr = args.first.line_range.begin..ll
404+
AstNode.new(:aref_field, args, :char => sr, :line => lr)
403405
end
404406

405407
def on_array(other)

spec/parser/ruby/ruby_parser_spec.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,5 +702,55 @@ def add(x) = x + 1
702702
expect(next_node.line_range).to eq(3..3)
703703
expect(code[next_node.source_range]).to eq('next')
704704
end
705+
706+
# @bug gh-1420
707+
it "parses constant assignment with aref after aref assignment correctly" do
708+
code = <<-RUBY
709+
module Foo
710+
a = {}
711+
712+
# Earlier assignment
713+
a[:b] = 1
714+
715+
# This constant fails to parse correctly
716+
X = a[:b]
717+
end
718+
RUBY
719+
720+
parser = YARD::Parser::Ruby::RubyParser.new(code, nil)
721+
ast = parser.parse.root
722+
723+
# Find the constant assignment node (X = a[:b])
724+
assign_node = nil
725+
ast.traverse do |node|
726+
if node.type == :assign
727+
# Check if it's a constant assignment (X = a[:b])
728+
# Structure: assign[0] = var_field containing const "X"
729+
# assign[1] = aref node (a[:b])
730+
lhs = node[0]
731+
rhs = node[1]
732+
if lhs && lhs.type == :var_field && lhs[0] && lhs[0].type == :const &&
733+
(lhs[0][0] == "X" || lhs[0][0] == :X) && rhs && rhs.type == :aref
734+
assign_node = node
735+
break
736+
end
737+
end
738+
end
739+
740+
expect(assign_node).not_to be_nil, "Should find constant assignment X = a[:b]"
741+
742+
# Check that the aref node (a[:b]) has a valid source_range
743+
# The bug causes the ending position to be less than the beginning position
744+
aref_node = assign_node[1]
745+
expect(aref_node.type).to eq :aref
746+
expect(aref_node.source_range).not_to be_nil
747+
expect(aref_node.source_range.end).to be >= aref_node.source_range.begin,
748+
"aref node source_range should be valid (ending >= beginning), got #{aref_node.source_range.inspect}. " \
749+
"This indicates the parser got confused by the previous aref assignment."
750+
751+
# Verify the source can be extracted correctly
752+
extracted_source = code[aref_node.source_range]
753+
expect(extracted_source).to eq "a[:b]"
754+
end
705755
end
706756
end if HAVE_RIPPER

0 commit comments

Comments
 (0)