Update rubocop to latest with Ruby 2.2 support #233

cupakromer · 2015-08-02T04:42:05Z

This updates Rubocop from 0.23.0 to 0.32.1 (most recent).

Unfortunately, in order to support this new version, which includes several bug fixes and support for Ruby 2.2, we need to drop support on Ruby 1.9.2. In fact Rubocop dropped 1.9.2 support as of version 0.25.0.

Some additional new cops are now available:

Metrics/AbcSize (Assignment Branch Condition)
Metrics/PerceivedComplexity

There are three additional new cops but they have been disabled (these settings should be ported to rspec-dev as part of the project defaults):

Lint/NonLocalExitFromIterator

This has been disabled due it's inability to distinguish between when / where return is sent from within a block. For example, a DSL block which has an explicit return in a method definition triggers this cop:
```
some_method do
  def a_local_method
    return if true # triggers cop even though `return` will not exit from the block
  end
end
```
Style/ParallelAssignment

Rubocop suggested this because it believed that parallel assignment was slower than sequential assignment. Unfortunately the benchmark they reference is flawed due to how Ruby handles implicit returns. It turns out that parallel assignment is faster.
Style/StringLiteralsInInterpolation

We previously disabled the related cop Style/StringLiterals. We simply don't care about single vs double qoutes.

See also: rspec/rspec-dev#134

cupakromer · 2015-08-02T06:20:45Z

😦 way too many builds. A bit tired so was thrashing trying to get the travis config working. Now that things are green, I'm grabbing a few hours of sleep and will get this cleaned up in the AM.

Also, I'm temporarily dumping the parallel-vs-sequential assignment and symbol-to-proc-vs-block benchmarks to stdout in the script/run_build. You can see how all of the different Ruby versions shape up in the last travis build: https://travis-ci.org/rspec/rspec-support/builds/73746126

I skimmed the benchmarks and it looks like the general trends are:

sequential assignment & blocks are faster on 1.8.7, REE, JRubys
parallel assignment & symbol-to-proc are faster on 1.9.2 through 2.2

There were a lot of numbers to skim over tonight, but I think the benchmarks show that sequential vs parallel assignment really doesn't matter much for us. So we can disable that cop and use what seems best for the situation.

As for the block vs symbol-to-proc, we just need to decide if / when we are going to start following the optimizations for the newer Rubies. Thoughts?

/cc @rspec/rspec

This also updates the travis setup to use the latest bundler. Travis currently defaults to Bundler version 1.7.4, however, this bundler version does not support the `platform: :ruby_22` configuration setting. So we need to tell travis to install a more recent bundler. Also, `platform: :ruby_19` applies to **both** 1.9.2 and 1.9.3. But Rubocop stopped supporting 1.9.2. Thus we need to check the `RUBY_VERSION` in our `Gemfile` and not attempt to install Rubocop specifically for 1.9.2. Unfortunately, this bundler does not work with JRuby (2.0 mode), but does work with JRuby 1.8 mode. So we need to update the config to ignore JRuby 2.0. However, the current config incorrectly assumes we can do that by checking the `JRUBY_OPTS` environment variable. Travis actually sets this be default on _all_ systems: http://docs.travis-ci.com/user/environment-variables/#Default-Environment-Variables However, we can check the Ruby version we pass into the travis matrix by using the `TRAVIS_RUBY_VERSION` and seeing if it equals "jruby".

lib/rspec/support/ruby_features.rb:9:7: C: Outdent access modifiers like module_function. module_function ^^^^^^^^^^^^^^^ lib/rspec/support/ruby_features.rb:24:7: C: Outdent access modifiers like module_function. module_function ^^^^^^^^^^^^^^^ lib/rspec/support/ruby_features.rb:48:7: C: Outdent access modifiers like module_function. module_function ^^^^^^^^^^^^^^^

lib/rspec/support/spec/string_matcher.rb:7:1: C: Extra empty line detected at block body beginning.

The check name is a little misleading. The check really is more: "`returned` called from inside a block". Given RSpec's DSL syntax and our heavy block usage, from which calling `return` is valid, this is being turned off.

lib/rspec/support/caller_filter.rb:72:23: C: Operator *= should be surrounded with a single space. increment *= 2 # The choice of two here is arbitrary. ^^ lib/rspec/support/method_signature_verifier.rb:98:48: C: Operator + should be surrounded with a single space. @max_non_kw_args = @min_non_kw_args + optional_non_kw_args ^

This raises the code metric thresholds for perceived complexity and assignment branch conditionals. These are only meant to be temporary settings to ignore the existing offenses. We fully intend to reduce these over time.

We have not decided how we are going to handle the symbol to proc vs block implementations. This is because the block is faster on Ruby 1.8.7, REE, and JRuby but slower on Ruby 1.9.2 through 2.2.2.

lib/rspec/support/version_checker.rb:8:9: C: Do not use parallel assignment. @library_name, @library_version = library_name, library_version ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

cupakromer · 2015-08-02T19:04:36Z

This is good to go.

JonRowe · 2015-08-02T23:50:08Z

.travis.yml

  # Note this doesn't work on JRUBY 2.0.0 mode so we don't do it
-  - if [ -z "$JRUBY_OPTS" ]; then gem install bundler -v=1.5.3 && alias bundle="bundle _1.5.3_"; fi
+  - if [ "jruby" != "$TRAVIS_RUBY_VERSION" ]; then gem install bundler; fi


This doesn't do the same thing, can we now safely use the later version of bundler on JRuby?

The comment above states that the older version of bundler is necessary due to an issue which is fixed. Though now I see that the default bundler on Travis, 1.7.4, doesn't contain the fix. So I can adjust this to support both.

Actually I'm not sure there is anything to change. The comment was stating that we weren't running the custom lower bundler version on JRuby, so it was supposed to be using the Travis default of 1.7.4. Am I missing something here?

JonRowe · 2015-08-02T23:53:18Z

Good work on the benchmarks, but shouldn't we be doing the config via rspec-dev as its centralised? Also I disagree with one of the style changes but I'm willing to go with the majority obviously :)

cupakromer · 2015-08-03T00:01:22Z

but shouldn't we be doing the config via rspec-dev as its centralised?

Bumping rubocop will cause all the builds to fail. We've opened up PRs on all repos that have these fixes as a stop-gap so the new cops can be configured and fixes made. I am going to have a PR open for rspec-mocks tonight, and a PR for rspec-dev with the "master" changes; which I plan to push out after the PRs get merged in so that the build stays happy.

JonRowe · 2015-08-03T00:17:21Z

Yeah but we can open the PRs from rspec-dev with the bump and then fix them in those PRS

benoittgt · 2018-03-15T18:21:45Z

@JonRowe do you think fix conflicts in a new PR (from this one) could be a good idea? I can work on it.

It seems lot's of things have change since. Maybe it can be close?

JonRowe · 2018-03-15T21:45:23Z

@benoittgt feel free to take a shot at rebasing this

benoittgt · 2019-06-12T06:50:50Z

Closing for now. In favor of #350

Updated rubocop rules see: #350

…pec/rspec-support#350) Updated rubocop rules see: rspec/rspec-support#350 --- This commit was imported from rspec/rspec-support@0953e54.

cupakromer added 3 commits August 2, 2015 11:22

Fix extra empty line at block body beginning.

9f5407a

lib/rspec/support/spec/string_matcher.rb:7:1: C: Extra empty line detected at block body beginning.

cupakromer force-pushed the update-rubocop branch from 02daf44 to 6761718 Compare August 2, 2015 15:23

cupakromer added 6 commits August 2, 2015 12:58

Disable non-local exist check.

528237e

The check name is a little misleading. The check really is more: "`returned` called from inside a block". Given RSpec's DSL syntax and our heavy block usage, from which calling `return` is valid, this is being turned off.

Disable another single vs double quote cop.

10e5a01

Increase code metric thresholds for now.

381c0be

This raises the code metric thresholds for perceived complexity and assignment branch conditionals. These are only meant to be temporary settings to ignore the existing offenses. We fully intend to reduce these over time.

Temporarily disable symbol to proc.

ce0fd7b

We have not decided how we are going to handle the symbol to proc vs block implementations. This is because the block is faster on Ruby 1.8.7, REE, and JRuby but slower on Ruby 1.9.2 through 2.2.2.

Fix parallel assignment offense.

d26a58b

lib/rspec/support/version_checker.rb:8:9: C: Do not use parallel assignment. @library_name, @library_version = library_name, library_version ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

cupakromer force-pushed the update-rubocop branch from 6761718 to d26a58b Compare August 2, 2015 18:22

cupakromer mentioned this pull request Aug 2, 2015

Updating Rubocop to 0.32.1 issues rspec/rspec-dev#134

Closed

JonRowe reviewed Aug 2, 2015
View reviewed changes

cupakromer mentioned this pull request Aug 3, 2015

Rubocop upgrade rspec/rspec-core#2051

Closed

cupakromer force-pushed the update-rubocop branch 3 times, most recently from bac55b1 to d26a58b Compare August 13, 2015 04:13

benoittgt mentioned this pull request Apr 5, 2018

Fix #233 conflicts -> Update rubocop #350

Merged

benoittgt closed this Jun 12, 2019

benoittgt deleted the update-rubocop branch June 12, 2019 06:51

JonRowe pushed a commit that referenced this pull request Jun 10, 2020

Fix #233 conflicts -> Update rubocop (#350)

c221faa

Updated rubocop rules see: #350

JonRowe pushed a commit that referenced this pull request Jun 10, 2020

Fix #233 conflicts -> Update rubocop (#350)

0953e54

Updated rubocop rules see: #350

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update rubocop to latest with Ruby 2.2 support #233

Update rubocop to latest with Ruby 2.2 support #233

cupakromer commented Aug 2, 2015

cupakromer commented Aug 2, 2015

cupakromer commented Aug 2, 2015

JonRowe Aug 2, 2015

cupakromer Aug 2, 2015

cupakromer Aug 3, 2015

JonRowe commented Aug 2, 2015

cupakromer commented Aug 3, 2015

JonRowe commented Aug 3, 2015

benoittgt commented Mar 15, 2018

JonRowe commented Mar 15, 2018

benoittgt commented Jun 12, 2019

Update rubocop to latest with Ruby 2.2 support #233

Update rubocop to latest with Ruby 2.2 support #233

Conversation

cupakromer commented Aug 2, 2015

cupakromer commented Aug 2, 2015

cupakromer commented Aug 2, 2015

JonRowe Aug 2, 2015

Choose a reason for hiding this comment

cupakromer Aug 2, 2015

Choose a reason for hiding this comment

cupakromer Aug 3, 2015

Choose a reason for hiding this comment

JonRowe commented Aug 2, 2015

cupakromer commented Aug 3, 2015

JonRowe commented Aug 3, 2015

benoittgt commented Mar 15, 2018

JonRowe commented Mar 15, 2018

benoittgt commented Jun 12, 2019