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

NodePattern: Add <> for matching children in any order. #6965

Merged
merged 1 commit into from Apr 25, 2019

Conversation

marcandre
Copy link
Contributor

@marcandre marcandre commented Apr 24, 2019

These can be used multiple times in the same sequence as well as
nested.

Limitations: A sequence can only contain a single ellipsis (contained
or not within a <> sequence).

Other implementation changes:

  • Captures are now an array (i.e. capture0 is now captures[0])
  • Temporary variables like temp1 were renamed to node1.
  • Reading tokens until a delimiter was factorized into tokens_until

Example of produced code:

# Pattern: (hash <(pair (sym $_)) $...>)

(captures = Array.new(2)) && node0.is_a?(RuboCop::AST::Node) &&
node0.children.size >= 0 &&
node0.is_a?(RuboCop::AST::Node) && node0.hash_type? &&
(captures[1] = []) && node0.children[0..-1].each_with_object({}) { |child1, matched2|
  case
    when !matched2[0] && child1.is_a?(RuboCop::AST::Node) &&
child1.children.size == 1 &&
child1.is_a?(RuboCop::AST::Node) && child1.pair_type? &&
(node3 = child1.children[0]).is_a?(RuboCop::AST::Node) &&
  node3.children.size == 1 &&
  node3.is_a?(RuboCop::AST::Node) && node3.sym_type? &&
  (captures[0] = node3.children[0]; true) then matched2[0] = true
      else captures[1] << child1
    end
}.size == 1

Fixes #6841

…rder.

These can be used multiple times in the same sequence as well as
nested.

Limitations: A sequence can only contain a single ellipsis (contained
or not within a <> sequence).

Other implementation changes:
- Captures are now an array (e.g. `capture0` is now `captures[0]`)
- Temporary variables like `temp1` were renamed to `node1`.
- Reading tokens until a delimiter was factorized into `tokens_until`
@marcandre
Copy link
Contributor Author

Fixed a few small things and even found a strange JRuby bug, all green now.

@marcandre marcandre changed the title [Fix #6841] NodePattern: Add <> for matching children in any order. NodePattern: Add <> for matching children in any order. Apr 25, 2019
@bbatsov bbatsov merged commit 971328e into rubocop:master Apr 25, 2019
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 25, 2019

Fantastic work, @marcandre!

koic added a commit to koic/rubocop-performance that referenced this pull request May 2, 2019
Follow up rubocop/rubocop#6965.

This PR fixes a false negartive for `Performance/RegexpMatch`
when using RuboCop 0.68 or higher.

```ruby
# example.rb
def foo
  if /re/.match(foo, 1)
    do_something
  end
end
```

## RuboCop 0.67

RuboCop 0.67 warns.

```console
% rubocop _0.67.2_ --only Performance/RegexpMatch
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

## RuboCop 0.68 (Before)

RuboCop 0.68 does not warn. This is a false negative.

```console
% rubocop _0.68.1_ --only Performance/RegexpMatch --require
rubocop-performance
Inspecting 1 file
.

1 file inspected, no offenses detected
```

## RuboCop 0.68 (After)

This PR fixes a false negative.

```console
% rubocop --only Performance/RegexpMatch --require rubocop-performance
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

This fix uses the following `<>` node pattern notation introduced in
RuboCop 0.68, so it supports Ruby version 0.68 or higher.
rubocop/rubocop#6965
koic added a commit to koic/rubocop-performance that referenced this pull request May 2, 2019
Follow up rubocop/rubocop#6965.

This PR fixes a false negartive for `Performance/RegexpMatch`
when using RuboCop 0.68 or higher.

```ruby
# example.rb
def foo
  if /re/.match(foo, 1)
    do_something
  end
end
```

## RuboCop 0.67

RuboCop 0.67 warns.

```console
% rubocop _0.67.2_ --only Performance/RegexpMatch
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

## RuboCop 0.68 (Before)

RuboCop 0.68 does not warn. This is a false negative.

```console
% rubocop _0.68.1_ --only Performance/RegexpMatch --require
rubocop-performance
Inspecting 1 file
.

1 file inspected, no offenses detected
```

## RuboCop 0.68 (After)

This PR fixes a false negative.

```console
% rubocop --only Performance/RegexpMatch --require rubocop-performance
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

This fix uses the following `<>` node pattern notation introduced in
RuboCop 0.68, so it supports Ruby version 0.68 or higher.
rubocop/rubocop#6965
koic added a commit to koic/rubocop-performance that referenced this pull request May 3, 2019
Follow up rubocop/rubocop#6965.

This PR fixes a false negartive for `Performance/RegexpMatch`
when using RuboCop 0.68 or higher.

```ruby
# example.rb
def foo
  if /re/.match(foo, 1)
    do_something
  end
end
```

## RuboCop 0.67

RuboCop 0.67 warns.

```console
% rubocop _0.67.2_ --only Performance/RegexpMatch
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

## RuboCop 0.68 (Before)

RuboCop 0.68 does not warn. This is a false negative.

```console
% rubocop _0.68.1_ --only Performance/RegexpMatch --require
rubocop-performance
Inspecting 1 file
.

1 file inspected, no offenses detected
```

## RuboCop 0.68 (After)

This PR fixes a false negative.

```console
% rubocop --only Performance/RegexpMatch --require rubocop-performance
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

This fix uses the following `<>` node pattern notation introduced in
RuboCop 0.68, so it supports Ruby version 0.68 or higher.
rubocop/rubocop#6965
@pirj
Copy link
Member

pirj commented May 22, 2019

Please correct me if I'm mistaken, but it seems there's a minor mistake in the description:

# Pattern: (hash <(pair (:sym $_)) $...>)

should probably be:

# Pattern: (hash <(pair (sym $_)) $...>)

@marcandre
Copy link
Contributor Author

marcandre commented May 22, 2019

@pirj Both are valid and should give you similar results. Using :sym will generate node.type == :sym (note: when it is at the beginning of a sequence!), while sym will generate node.sym_type? (note: in any position), and unless it is redefined by the user, sym_type? returns type == :sym :-)
HTH

@marcandre
Copy link
Contributor Author

PS: I believe you are right though that (sym ...) is probably more usual and better than (:sym ...). I modified my description above accordingly.

pirj added a commit to pirj/rubocop-rspec that referenced this pull request Aug 11, 2019
0.68.0 ships with hash children matching
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
pirj added a commit to pirj/rubocop-rspec that referenced this pull request Aug 11, 2019
0.68.0 ships with the new children matching syntax
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
@pirj
Copy link
Member

pirj commented Aug 11, 2019

What makes this change double as awesome is that it also works for matching arguments, not only in a hash. 💯

pirj added a commit to pirj/rubocop-rspec that referenced this pull request Aug 12, 2019
0.68.0 ships with the new children matching syntax
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
pirj added a commit to pirj/rubocop-rspec that referenced this pull request Aug 18, 2019
0.68.0 ships with the new children matching syntax
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
kellysutton pushed a commit to kellysutton/rubocop-rspec that referenced this pull request Oct 28, 2019
0.68.0 ships with the new children matching syntax
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
renawatson68 added a commit to renawatson68/performance-develop-rubyonrails that referenced this pull request Sep 23, 2022
Follow up rubocop/rubocop#6965.

This PR fixes a false negartive for `Performance/RegexpMatch`
when using RuboCop 0.68 or higher.

```ruby
# example.rb
def foo
  if /re/.match(foo, 1)
    do_something
  end
end
```

## RuboCop 0.67

RuboCop 0.67 warns.

```console
% rubocop _0.67.2_ --only Performance/RegexpMatch
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

## RuboCop 0.68 (Before)

RuboCop 0.68 does not warn. This is a false negative.

```console
% rubocop _0.68.1_ --only Performance/RegexpMatch --require
rubocop-performance
Inspecting 1 file
.

1 file inspected, no offenses detected
```

## RuboCop 0.68 (After)

This PR fixes a false negative.

```console
% rubocop --only Performance/RegexpMatch --require rubocop-performance
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

This fix uses the following `<>` node pattern notation introduced in
RuboCop 0.68, so it supports Ruby version 0.68 or higher.
rubocop/rubocop#6965
richardstewart0213 added a commit to richardstewart0213/performance-build-Performance-optimization-analysis- that referenced this pull request Nov 4, 2022
Follow up rubocop/rubocop#6965.

This PR fixes a false negartive for `Performance/RegexpMatch`
when using RuboCop 0.68 or higher.

```ruby
# example.rb
def foo
  if /re/.match(foo, 1)
    do_something
  end
end
```

## RuboCop 0.67

RuboCop 0.67 warns.

```console
% rubocop _0.67.2_ --only Performance/RegexpMatch
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

## RuboCop 0.68 (Before)

RuboCop 0.68 does not warn. This is a false negative.

```console
% rubocop _0.68.1_ --only Performance/RegexpMatch --require
rubocop-performance
Inspecting 1 file
.

1 file inspected, no offenses detected
```

## RuboCop 0.68 (After)

This PR fixes a false negative.

```console
% rubocop --only Performance/RegexpMatch --require rubocop-performance
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

This fix uses the following `<>` node pattern notation introduced in
RuboCop 0.68, so it supports Ruby version 0.68 or higher.
rubocop/rubocop#6965
pirj added a commit to rubocop/rubocop-capybara that referenced this pull request Dec 29, 2022
0.68.0 ships with the new children matching syntax
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
ydah pushed a commit to rubocop/rubocop-factory_bot that referenced this pull request Apr 13, 2023
0.68.0 ships with the new children matching syntax
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
MarttiCheng added a commit to MarttiCheng/Rubocop-Performance that referenced this pull request Sep 28, 2023
Follow up rubocop/rubocop#6965.

This PR fixes a false negartive for `Performance/RegexpMatch`
when using RuboCop 0.68 or higher.

```ruby
# example.rb
def foo
  if /re/.match(foo, 1)
    do_something
  end
end
```

## RuboCop 0.67

RuboCop 0.67 warns.

```console
% rubocop _0.67.2_ --only Performance/RegexpMatch
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

## RuboCop 0.68 (Before)

RuboCop 0.68 does not warn. This is a false negative.

```console
% rubocop _0.68.1_ --only Performance/RegexpMatch --require
rubocop-performance
Inspecting 1 file
.

1 file inspected, no offenses detected
```

## RuboCop 0.68 (After)

This PR fixes a false negative.

```console
% rubocop --only Performance/RegexpMatch --require rubocop-performance
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

This fix uses the following `<>` node pattern notation introduced in
RuboCop 0.68, so it supports Ruby version 0.68 or higher.
rubocop/rubocop#6965
SerhiiMisiura added a commit to SerhiiMisiura/Rubocop-Performance that referenced this pull request Oct 5, 2023
Follow up rubocop/rubocop#6965.

This PR fixes a false negartive for `Performance/RegexpMatch`
when using RuboCop 0.68 or higher.

```ruby
# example.rb
def foo
  if /re/.match(foo, 1)
    do_something
  end
end
```

## RuboCop 0.67

RuboCop 0.67 warns.

```console
% rubocop _0.67.2_ --only Performance/RegexpMatch
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

## RuboCop 0.68 (Before)

RuboCop 0.68 does not warn. This is a false negative.

```console
% rubocop _0.68.1_ --only Performance/RegexpMatch --require
rubocop-performance
Inspecting 1 file
.

1 file inspected, no offenses detected
```

## RuboCop 0.68 (After)

This PR fixes a false negative.

```console
% rubocop --only Performance/RegexpMatch --require rubocop-performance
Inspecting 1 file
C

Offenses:

example.rb:2:6: C: Performance/RegexpMatch: Use match? instead of match
when MatchData is not used.
  if /re/.match(foo, 1)
       ^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```

This fix uses the following `<>` node pattern notation introduced in
RuboCop 0.68, so it supports Ruby version 0.68 or higher.
rubocop/rubocop#6965
ydah pushed a commit to rubocop/rubocop-rspec_rails that referenced this pull request Mar 27, 2024
0.68.0 ships with the new children matching syntax
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
ydah pushed a commit to rubocop/rubocop-rspec_rails that referenced this pull request Mar 27, 2024
0.68.0 ships with the new children matching syntax
(rubocop/rubocop#6965) that might come handy
for some of RSpec cops.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Matching any child with NodePattern
3 participants