rubocop / rubocop-ast

RuboCop's AST extensions and NodePattern functionality
https://docs.rubocop.org/rubocop-ast
MIT License
112 stars 53 forks source link

Using captures in `?` with OR branches has an effect on captures outside the optional matching #323

Open Earlopain opened 1 month ago

Earlopain commented 1 month ago

It's a bit much, so I'll just start with the testcase:

require 'bundler/inline'

# Using captures in `?` with OR branches has an effect on captures outside the optional matching
gemfile do
  source 'https://rubygems.org'
  gem 'rubocop-ast', '1.32.3'
  gem 'minitest'
end

require 'minitest/autorun'

class Matcher
  class << self
    extend RuboCop::AST::NodePattern::Macros

    def_node_matcher :one_branch, <<~PATTERN
      (
        send _ $_method_name
        (
          sym
          {$_}
        )?
      )
    PATTERN

    def_node_matcher :two_branch, <<~PATTERN
      (
        send _ $_method_name
        (
          sym
          {$_ | $_}
        )?
      )
    PATTERN

    def_node_matcher :three_branch, <<~PATTERN
      (
        send _ $_method_name
        (
          sym
          {$_ | $_ | $_}
        )?
      )
    PATTERN
  end
end

class Test < Minitest::Test
  def assert_match(code, matcher, expected)
    ast = RuboCop::AST::ProcessedSource.new(code, 3.3).ast
    actual = Matcher.send(matcher, ast)
    assert_equal(expected, actual)
  end

  def test_one
    assert_match("foo", :one_branch, [:foo, []])
  end

  def test_one_optional_matches
    assert_match("foo(:bar)", :one_branch, [:foo, [:bar]])
  end

  def test_two
    assert_match("foo", :two_branch, [:foo, []])
  end

  def test_two_optional_matches
    assert_match("foo(:bar)", :two_branch, [:foo, [:bar]])
  end

  def test_three
    assert_match("foo", :three_branch, [:foo, []])
  end

  def test_three_optional_matches
    assert_match("foo(:bar)", :three_branch, [:foo, [:bar]])
  end
end

#   1) Failure:
# Test#test_two_optional_matches [test.rb:68]:
# Expected: [:foo, [:bar]]
#   Actual: [[:foo], [:bar]]
# 
#   2) Failure:
# Test#test_two [test.rb:64]:
# Expected: [:foo, []]
#   Actual: [[], []]
# 
#   3) Failure:
# Test#test_three [test.rb:72]:
# Expected: [:foo, []]
#   Actual: [:foo, [], [], []]
# 
# 6 runs, 6 assertions, 3 failures, 0 errors, 0 skips

There are three node patterns, all very similar except that an OR group has different amount of branches. I expect them to all return the same thing but what actually happens is that depending on the amount of branches the output changes:

There seems to be some state leak. While I'm getting better at writing node patterns, understanding what is going on here is far beyond me. I think this pattern should be valid but maybe I'm misunderstanding something?

marcandre commented 1 month ago

Thanks for opening the issue.

Yes, I agree that these patterns should all be valid and return the exact same results for any input.