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

Fix a false negative for Performance/RegexpMatch #73

Merged

Conversation

koic
Copy link
Member

@koic koic commented Jul 28, 2019

Follow up rubocop/rubocop#5569.

This PR fixes a false negative for Performance/RegexpMatch when MatchData is not detected in if branch of guard condition.

% cat example.rb
# frozen_string_literal: true

raise unless ex.message =~ /can't modify frozen IOError/

% bundle exec rubocop --only Performance/RegexpMatch
Inspecting 1 file
.

1 file inspected, no offenses detected

This issue was detected by the following code.
https://github.com/rails/rails/blob/v6.0.0.rc2/actioncable/test/client_test.rb#L94

That is a feedback that @kamipo gave. Thank you.

And follow up rubocop/rubocop#5569 (comment).

This PR prevents a false positive for Performance/RegexpMatch when MatchData exists after guard condition.

The following is an example code with false positive.

def foo
  return if /re/ =~ str

  # `Regexp.last_match[1]` cannot be referenced if changed from `=~` to `match?`.
  do_something(Regexp.last_match[1])
end

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

koic added 2 commits July 27, 2019 17:58
Follow up rubocop/rubocop#5569.

This PR fixes a false negative for `Performance/RegexpMatch`
when `MatchData` is not detected in `if` branch of guard condition.

```console
% cat example.rb
# frozen_string_literal: true

raise unless ex.message =~ /can't modify frozen IOError/

% bundle exec rubocop --only Performance/RegexpMatch
Inspecting 1 file
.

1 file inspected, no offenses detected
```

This issue was detected by the following code.
https://github.com/rails/rails/blob/v6.0.0.rc2/actioncable/test/client_test.rb#L94
Follow up rubocop/rubocop#5569 (comment).

This commit fixes a false positive for `Performance/RegexpMatch`
when `MatchData` exists after guard condition.

The following is an example code with false positive.

```ruby
def foo
  return if /re/ =~ str

  # `Regexp.last_match[1]` cannot be referenced if changed from `=~` to `match?`.
  do_something(Regexp.last_match[1])
end
```
@koic
Copy link
Member Author

koic commented Jul 28, 2019

This PR still has the following problem. I will work on this.

% cd path/to/rails/rails # https://github.com/rails/rails
% bundle exec rubocop activerecord/lib/arel/visitors/oracle.rb --only Performance/RegexpMatch -d
For /Users/koic/src/github.com/rails/rails: configuration from /Users/koic/src/github.com/rails/rails/.rubocop.yml
configuration from /Users/koic/src/github.com/rubocop-hq/rubocop-performance/config/default.yml
configuration from /Users/koic/src/github.com/rubocop-hq/rubocop-performance/config/default.yml
Default configuration from /Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-0.71.0/config/default.yml
configuration from /Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.0.0/config/default.yml
configuration from /Users/koic/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rubocop-rails-2.0.0/config/default.yml
Inspecting 1 file
Scanning /Users/koic/src/github.com/rails/rails/activerecord/lib/arel/visitors/oracle.rb
An error occurred while Performance/RegexpMatch cop was inspecting /Users/koic/src/github.com/rails/rails/activerecord/lib/arel/visitors/oracle.rb:125:51.
undefined method `if_branch' for #<RuboCop::AST::BlockNode:0x00007fedd7303d48>
/Users/koic/src/github.com/rubocop-hq/rubocop-performance/lib/rubocop/cop/performance/regexp_match.rb:207:in `block in next_match_pos'
/Users/koic/src/github.com/rubocop-hq/rubocop-performance/lib/rubocop/cop/performance/regexp_match.rb:127:in `block in search_match_nodes'

koic added a commit to koic/rubocop-performance that referenced this pull request Jul 28, 2019
Follow up rubocop#73 (comment).

This commit fixes an error for `Performance/RegexpMatch` when
a regexp used independently with a regexp used in `if` are mixed.
@koic
Copy link
Member Author

koic commented Jul 28, 2019

I fixed the above error.

Follow up rubocop#73 (comment).

This commit fixes an error for `Performance/RegexpMatch` when
a regexp used independently with a regexp used in `if` are mixed.
@koic koic force-pushed the fix_false_negative_for_performance_regexp_match branch from 7a41f99 to 12fc7fb Compare July 29, 2019 04:21
@koic koic merged commit d7f063e into rubocop:master Jul 29, 2019
@koic koic deleted the fix_false_negative_for_performance_regexp_match branch July 29, 2019 04:27
yahonda added a commit to yahonda/oracle-enhanced that referenced this pull request Sep 9, 2019
* Use RuboCop Performance 1.4.1
* Refer rubocop/rubocop-performance#61 and/or rubocop/rubocop-performance#73

```ruby
$ bundle exec rubocop -a
Inspecting 66 files
..........C..C..C......C.................C........................

Offenses:

lib/active_record/connection_adapters/oracle_enhanced/database_statements.rb:68:21: C: [Corrected] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
          return if sql =~ /FROM all_/
                    ^^^^^^^^^^^^^^^^^^
lib/active_record/connection_adapters/oracle_enhanced/jdbc_connection.rb:241:26: C: [Corrected] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
            raise unless e.message =~ /^(Closed Connection|Io exception:|No more data to read from socket|IO Error:)/
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb:321:35: C: [Corrected] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
            host = "[#{host}]" if host =~ /^[^\[].*:/  # IPv6
                                  ^^^^^^^^^^^^^^^^^^^
lib/active_record/connection_adapters/oracle_enhanced/oci_connection.rb:323:46: C: [Corrected] Performance/RegexpMatch: Use match? instead of match when MatchData is not used.
            database = "/#{database}" unless database.match(/^\//)
                                             ^^^^^^^^^^^^^^^^^^^^^
lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb:650:46: C: [Corrected] Performance/RegexpMatch: Use match? instead of =~ when MatchData is not used.
              field["data_default"] = nil if field["data_default"] =~ /^(null|empty_[bc]lob\(\))$/i
                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/active_record/connection_adapters/oracle_enhanced/connection_spec.rb:183:58: C: [Corrected] Performance/RegexpMatch: Use match? instead of match when MatchData is not used.
      params[:database] = "/#{params[:database]}" unless params[:database].match(/^\//)
                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

66 files inspected, 6 offenses detected, 6 offenses corrected
$
```
patrickm53 pushed a commit to patrickm53/performance-develop-rubyonrails that referenced this pull request Sep 23, 2022
Follow up rubocop/rubocop-performance#73 (comment).

This commit fixes an error for `Performance/RegexpMatch` when
a regexp used independently with a regexp used in `if` are mixed.
richardstewart0213 added a commit to richardstewart0213/performance-build-Performance-optimization-analysis- that referenced this pull request Nov 4, 2022
Follow up rubocop/rubocop-performance#73 (comment).

This commit fixes an error for `Performance/RegexpMatch` when
a regexp used independently with a regexp used in `if` are mixed.
Cute0110 added a commit to Cute0110/Rubocop-Performance that referenced this pull request Sep 28, 2023
Follow up rubocop/rubocop-performance#73 (comment).

This commit fixes an error for `Performance/RegexpMatch` when
a regexp used independently with a regexp used in `if` are mixed.
SerhiiMisiura added a commit to SerhiiMisiura/Rubocop-Performance that referenced this pull request Oct 5, 2023
Follow up rubocop/rubocop-performance#73 (comment).

This commit fixes an error for `Performance/RegexpMatch` when
a regexp used independently with a regexp used in `if` are mixed.
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.

1 participant