diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc3a17418b..7d71c1a0d1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ### Bug fixes +* [#7644](https://github.com/rubocop-hq/rubocop/issues/7644): Fix patterns with named wildcards in unions. ([@marcandre][]) * [#7639](https://github.com/rubocop-hq/rubocop/pull/7639): Fix logical operator edge case in `omit_parentheses` style of `Style/MethodCallWithArgsParentheses`. ([@gsamokovarov][]) * [#7661](https://github.com/rubocop-hq/rubocop/pull/7661): Fix to return correct info from multi-line regexp. ([@Tietew][]) * [#7655](https://github.com/rubocop-hq/rubocop/issues/7655): Fix an error when processing a regexp with a line break at the start of capture parenthesis. ([@koic][]) diff --git a/lib/rubocop/node_pattern.rb b/lib/rubocop/node_pattern.rb index 14e58a05ccb..fd663fbb681 100644 --- a/lib/rubocop/node_pattern.rb +++ b/lib/rubocop/node_pattern.rb @@ -188,7 +188,7 @@ def initialize(str, node_var = 'node0') @temps = 0 # avoid name clashes between temp variables @captures = 0 # number of captures seen - @unify = {} # named wildcard -> temp variable number + @unify = {} # named wildcard -> temp variable @params = 0 # highest % (param) number seen run(node_var) end @@ -466,12 +466,67 @@ def insure_same_captures(enum, what) end end + def access_unify(name) + var = @unify[name] + + if var == :forbidden_unification + fail_due_to "Wildcard #{name} was first seen in a subset of a" \ + " union and can't be used outside that union" + end + var + end + + def forbid_unification(*names) + names.each do |name| + @unify[name] = :forbidden_unification + end + end + + # rubocop:disable Metrics/MethodLength, Metrics/AbcSize + def unify_in_union(enum) + # We need to reset @unify before each branch is processed. + # Moreover we need to keep track of newly encountered wildcards. + # Var `new_unify_intersection` will hold those that are encountered + # in all branches; these are not a problem. + # Var `partial_unify` will hold those encountered in only a subset + # of the branches; these can't be used outside of the union. + + return to_enum __method__, enum unless block_given? + + new_unify_intersection = nil + partial_unify = [] + unify_before = @unify.dup + + result = enum.each do |e| + @unify = unify_before.dup if new_unify_intersection + yield e + new_unify = @unify.keys - unify_before.keys + if new_unify_intersection.nil? + # First iteration + new_unify_intersection = new_unify + else + union = new_unify_intersection | new_unify + new_unify_intersection &= new_unify + partial_unify |= union - new_unify_intersection + end + end + + # At this point, all members of `new_unify_intersection` can be used + # for unification outside of the union, but partial_unify may not + + forbid_unification(*partial_unify) + + result + end + # rubocop:enable Metrics/MethodLength, Metrics/AbcSize + def compile_union # we need to ensure that each branch of the {} contains the same # number of captures (since only one branch of the {} can actually # match, the same variables are used to hold the captures for each # branch) enum = tokens_until('}', 'union') + enum = unify_in_union(enum) terms = insure_same_captures(enum, 'branch of {}') .map { compile_expr } @@ -503,12 +558,12 @@ def compile_wildcard(name) # we have already seen a wildcard with this name before # so the value it matched the first time will already be stored # in a temp. check if this value matches the one stored in the temp - "#{CUR_ELEMENT} == temp#{@unify[name]}" + "#{CUR_ELEMENT} == #{access_unify(name)}" else - n = @unify[name] = next_temp_value - # double assign to temp#{n} to avoid "assigned but unused variable" - "(temp#{n} = #{CUR_ELEMENT}; " \ - "temp#{n} = temp#{n}; true)" + n = @unify[name] = "unify_#{name.gsub('-', '__')}" + # double assign to avoid "assigned but unused variable" + "(#{n} = #{CUR_ELEMENT}; " \ + "#{n} = #{n}; true)" end end @@ -560,9 +615,8 @@ def compile_args(tokens) def compile_arg(token) case token when WILDCARD then - name = token[1..-1] - number = @unify[name] || fail_due_to('invalid in arglist: ' + token) - "temp#{number}" + name = token[1..-1] + access_unify(name) || fail_due_to('invalid in arglist: ' + token) when LITERAL then token when PARAM then get_param(token[1..-1]) when CLOSING then fail_due_to("#{token} in invalid position") diff --git a/spec/rubocop/node_pattern_spec.rb b/spec/rubocop/node_pattern_spec.rb index cd06147f8e3..8a09d769937 100644 --- a/spec/rubocop/node_pattern_spec.rb +++ b/spec/rubocop/node_pattern_spec.rb @@ -460,6 +460,110 @@ it_behaves_like 'matching' end + + context 'within a union' do + context 'confined to the union' do + context 'without unification' do + let(:pattern) { '{(array (int 1) _num) (array _num (int 1))}' } + let(:ruby) { '[2, 1]' } + + it_behaves_like 'matching' + end + + context 'with partial unification' do + let(:pattern) { '{(array _num _num) (array _num (int 1))}' } + + context 'matching the unified branch' do + let(:ruby) { '[5, 5]' } + + it_behaves_like 'matching' + end + + context 'matching the free branch' do + let(:ruby) { '[2, 1]' } + + it_behaves_like 'matching' + end + + context 'that can not be unified' do + let(:ruby) { '[3, 2]' } + + it_behaves_like 'nonmatching' + end + end + end + + context 'with a preceeding unifying constraint' do + let(:pattern) do + '(array _num {(array (int 1) _num) + send + (array _num (int 1))})' + end + + context 'matching a branch' do + let(:ruby) { '[2, [2, 1]]' } + + it_behaves_like 'matching' + end + + context 'that can not be unified' do + let(:ruby) { '[3, [2, 1]]' } + + it_behaves_like 'nonmatching' + end + end + + context 'with a succeeding unifying constraint' do + context 'with branches without the wildcard' do + context 'encountered first' do + let(:pattern) do + '(array {send + (array (int 1) _num) + } _num)' + end + + it_behaves_like 'invalid' + end + + context 'encountered after' do + let(:pattern) do + '(array {(array (int 1) _num) + (array _num (int 1)) + send + } _num)' + end + + it_behaves_like 'invalid' + end + end + + context 'with all branches with the wildcard' do + let(:pattern) do + '(array {(array (int 1) _num) + (array _num (int 1)) + } _num)' + end + + context 'matching the first branch' do + let(:ruby) { '[[1, 2], 2]' } + + it_behaves_like 'matching' + end + + context 'matching another branch' do + let(:ruby) { '[[2, 1], 2]' } + + it_behaves_like 'matching' + end + + context 'that can not be unified' do + let(:ruby) { '[[2, 1], 1]' } + + it_behaves_like 'nonmatching' + end + end + end + end end end