Skip to content

Commit

Permalink
[Fix #7644] NodePattern: Fix unification within unions
Browse files Browse the repository at this point in the history
  • Loading branch information
marcandre authored and bbatsov committed Feb 10, 2020
1 parent ebe2121 commit 956b401
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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][])
Expand Down
72 changes: 63 additions & 9 deletions lib/rubocop/node_pattern.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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 }

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand Down
104 changes: 104 additions & 0 deletions spec/rubocop/node_pattern_spec.rb
Expand Up @@ -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

Expand Down

0 comments on commit 956b401

Please sign in to comment.