Skip to content

Commit

Permalink
[Fix #323] Fix node captures inside of ?, +, and * repetition
Browse files Browse the repository at this point in the history
The calculation for how many captures a pattern has was not correct in this case.
A pattern like `(send _ _ (int {$_ | $_ | $_})?)` would end up with 3 captures, even though
the union would in total only contribute one
  • Loading branch information
Earlopain authored and marcandre committed Jan 22, 2025
1 parent 757c3cc commit a9f7be9
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/fix_captures_in_repetition.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#323](https://github.com/rubocop/rubocop-ast/issues/323): Fix node captures inside of `?`, `+`, and `*` repetition. ([@earlopain][])
8 changes: 7 additions & 1 deletion lib/rubocop/ast/node_pattern/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def child
children[0]
end

# @return [Integer] nb of captures of that node and its descendants
# @return [Integer] nb of captures that this node will emit
def nb_captures
children_nodes.sum(&:nb_captures)
end
Expand Down Expand Up @@ -243,6 +243,12 @@ def in_sequence_head

[with(children: new_children)]
end

# Each child in a union must contain the same number
# of captures. Only one branch ends up capturing.
def nb_captures
child.nb_captures
end
end

# Registry
Expand Down
162 changes: 162 additions & 0 deletions spec/rubocop/ast/node_pattern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,168 @@ def not_match_codes(*codes)

it_behaves_like 'invalid'
end

context 'using repetition' do
shared_examples 'repetition' do |behavior|
context 'with one capture' do
let(:pattern) { pattern_placeholder.sub('{}', '{$_}') }

it_behaves_like behavior
end

context 'with two captures' do
let(:pattern) { pattern_placeholder.sub('{}', '{$_ | $_}') }

it_behaves_like behavior
end

context 'with three captures' do
let(:pattern) { pattern_placeholder.sub('{}', '{$_ | $_ | $_}') }

it_behaves_like behavior
end
end

context 'with ?' do
context 'one capture' do
let(:pattern_placeholder) { '(send _ _ (int {})?)' }

context 'one match' do
it_behaves_like 'repetition', 'single capture' do
let(:ruby) { 'foo(1)' }
let(:captured_val) { [1] }
end
end

context 'no match' do
it_behaves_like 'repetition', 'single capture' do
let(:ruby) { 'foo' }
let(:captured_val) { [] }
end
end
end

context 'two captures' do
let(:pattern_placeholder) { '(send _ $_ (int {})?)' }

context 'one match' do
it_behaves_like 'repetition', 'multiple capture' do
let(:ruby) { 'foo(1)' }
let(:captured_vals) { [:foo, [1]] }
end
end

context 'no match' do
it_behaves_like 'repetition', 'multiple capture' do
let(:ruby) { 'foo' }
let(:captured_vals) { [:foo, []] }
end
end
end
end

context 'with +' do
context 'one capture' do
let(:pattern_placeholder) { '(send _ _ (int {})+)' }

context 'one match' do
it_behaves_like 'repetition', 'single capture' do
let(:ruby) { 'foo(1)' }
let(:captured_val) { [1] }
end
end

context 'two matches' do
it_behaves_like 'repetition', 'single capture' do
let(:ruby) { 'foo(1, 2)' }
let(:captured_val) { [1, 2] }
end
end

context 'no match' do
it_behaves_like 'repetition', 'nonmatching' do
let(:ruby) { 'foo' }
end
end
end

context 'two captures' do
let(:pattern_placeholder) { '(send _ $_ (int {})+)' }

context 'one match' do
it_behaves_like 'repetition', 'multiple capture' do
let(:ruby) { 'foo(1)' }
let(:captured_vals) { [:foo, [1]] }
end
end

context 'two matches' do
it_behaves_like 'repetition', 'multiple capture' do
let(:ruby) { 'foo(1, 2)' }
let(:captured_vals) { [:foo, [1, 2]] }
end
end

context 'no match' do
it_behaves_like 'repetition', 'nonmatching' do
let(:ruby) { 'foo' }
end
end
end
end

context 'with *' do
context 'one capture' do
let(:pattern_placeholder) { '(send _ _ (int {})*)' }

context 'one match' do
it_behaves_like 'repetition', 'single capture' do
let(:ruby) { 'foo(1)' }
let(:captured_val) { [1] }
end
end

context 'two matches' do
it_behaves_like 'repetition', 'single capture' do
let(:ruby) { 'foo(1, 2)' }
let(:captured_val) { [1, 2] }
end
end

context 'no match' do
it_behaves_like 'repetition', 'single capture' do
let(:ruby) { 'foo' }
let(:captured_val) { [] }
end
end
end

context 'two captures' do
let(:pattern_placeholder) { '(send _ $_ (int {})*)' }

context 'one match' do
it_behaves_like 'repetition', 'multiple capture' do
let(:ruby) { 'foo(1)' }
let(:captured_vals) { [:foo, [1]] }
end
end

context 'two matches' do
it_behaves_like 'repetition', 'multiple capture' do
let(:ruby) { 'foo(1, 2)' }
let(:captured_vals) { [:foo, [1, 2]] }
end
end

context 'no match' do
it_behaves_like 'repetition', 'multiple capture' do
let(:ruby) { 'foo' }
let(:captured_vals) { [:foo, []] }
end
end
end
end
end
end

describe 'negation' do
Expand Down

0 comments on commit a9f7be9

Please sign in to comment.