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

Suppress Object#=~ deprecation warning #43

Merged

Conversation

koic
Copy link
Member

@koic koic commented Apr 13, 2019

This PR suppresses the following warning.

/Users/koic/src/github.com/rubocop-hq/rubocop-performance/lib/rubocop/cop/performance/string_replacement.rb:89:
warning: deprecated Object#=~ is called on RuboCop::AST::Node; it always
returns nil

Object#=~ has been deprecated from Ruby 2.6.
ruby/ruby@ebff9dc

Related commit: ruby/ruby@f7e94df


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.

This PR suppresses the following warning.

```console
/Users/koic/src/github.com/rubocop-hq/rubocop-performance/lib/rubocop/cop/performance/string_replacement.rb:89:
warning: deprecated Object#=~ is called on RuboCop::AST::Node; it always
returns nil
```

`Object#=~` has been deprecated from Ruby 2.6.
ruby/ruby@ebff9dc

Related commit: ruby/ruby@f7e94df
@@ -86,7 +86,8 @@ def accept_first_param?(first_param)

unless first_param.str_type?
return true if options
return true unless first_source =~ DETERMINISTIC_REGEX
return true unless first_source.is_a?(String) &&
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to use Regexp#match

DETERMINISTIC_REGEX.match(first_source)

Regexp#match? does not implemented on jruby

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, It looks like there is no advantage to replacing it with Regexp#match.

% git diff
diff --git a/lib/rubocop/cop/performance/string_replacement.rb b/lib/rubocop/cop/performance/string_replacement.rb
index 848180ec0..1ca8e9fe8 100644
--- a/lib/rubocop/cop/performance/string_replacement.rb
+++ b/lib/rubocop/cop/performance/string_replacement.rb
@@ -86,8 +86,7 @@ module RuboCop

           unless first_param.str_type?
             return true if options
-            return true unless first_source.is_a?(String) &&
-                               first_source =~ DETERMINISTIC_REGEX
+            return true unless DETERMINISTIC_REGEX.match(first_source)

             # This must be done after checking DETERMINISTIC_REGEX
             # Otherwise things like \s will trip us up

This change will cause the following TypeError.

% bundle exec rspec spec/rubocop/cop/performance/string_replacement_spec.rb

(snip)

Failures:

  1) RuboCop::Cop::Performance::StringReplacement non deterministic regex allows regex literal containing interpolations
     Failure/Error: return true unless DETERMINISTIC_REGEX.match(first_source)

     TypeError:
       no implicit conversion of RuboCop::AST::Node into String
     # ./lib/rubocop/cop/performance/string_replacement.rb:89:in `match'
     # ./lib/rubocop/cop/performance/string_replacement.rb:89:in `accept_first_param?'
     # ./lib/rubocop/cop/performance/string_replacement.rb:40:in `block in on_send'
     # ./lib/rubocop/cop/performance/string_replacement.rb:47:in `string_replacement?'
     # ./lib/rubocop/cop/performance/string_replacement.rb:38:in `on_send'
     # ./spec/rubocop/cop/performance/string_replacement_spec.rb:214:in `block (3 levels) in <top (required)>'

  2) RuboCop::Cop::Performance::StringReplacement non deterministic regex allows regex constructor containing regex with interpolations
     Failure/Error: return true unless DETERMINISTIC_REGEX.match(first_source)

     TypeError:
       no implicit conversion of RuboCop::AST::Node into String
     # ./lib/rubocop/cop/performance/string_replacement.rb:89:in `match'
     # ./lib/rubocop/cop/performance/string_replacement.rb:89:in `accept_first_param?'
     # ./lib/rubocop/cop/performance/string_replacement.rb:40:in `block in on_send'
     # ./lib/rubocop/cop/performance/string_replacement.rb:47:in `string_replacement?'
     # ./lib/rubocop/cop/performance/string_replacement.rb:38:in `on_send'
     # ./spec/rubocop/cop/performance/string_replacement_spec.rb:228:in `block (3 levels) in <top (required)>'

Finished in 0.14404 seconds (files took 1.53 seconds to load)
133 examples, 2 failures

I think it would be better to follow upstream changes.
ruby/ruby@f7e94df

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thanks to consider it.

@koic koic merged commit fe20562 into rubocop:master Apr 18, 2019
@koic koic deleted the suppress_object_matches_deprecation_method branch April 18, 2019 15:10
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.

2 participants