Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fixes #7644] NodePattern: Fix unification within unions #7697

Merged
merged 1 commit into from Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,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