Rubocop: TargetRubyVersion is inferred (2.1) from rainbow gem on Travis CI

[dgmstuart]

On 0.59.2, Ruby 2.5.1 rubocop is running fine locally, but fails with the following message on Travis:

Error: Unsupported Ruby version 2.1 found in `TargetRubyVersion` parameter (in vendor/bundle/ruby/2.5.0/gems/rainbow-3.0.0/.rubocop.yml). 2.1-compatible analysis was dropped after version 0.58.
Supported versions: 2.2, 2.3, 2.4, 2.5

This seems odd: I wouldn't expect the .rubocop.yml from the rainbow gem to be involved in determining how rubocop runs for my repo.

I've tried specifying the TargetRubyVersion explicitly, as well as allowing it to be inferred from the Gemfile.lock.

I suppose this might be an issue with Travis?

Branch: https://github.com/dgmstuart/swing-out-london/tree/travis-fail
Build: https://travis-ci.com/dgmstuart/swing-out-london/builds/88642470

---

Expected behavior

Builds on CI should infer the TargetRubyVersion from the Gemfile.lock

Actual behavior

Builds on Travis CI seems to be inferring the TargetRubyVersion from the .rubocop.yml of the rainbow gem, which is a runtime dependency of rubocop.

Steps to reproduce the problem

The following branch has the issue:
https://github.com/dgmstuart/swing-out-london/tree/travis-fail

Running Rake locally runs rubocop correctly, but running on Travis fails.

Here is an example of a failing build: https://travis-ci.com/dgmstuart/swing-out-london/builds/88642470

RuboCop version

$ bundle exec rubocop -V
0.59.2 (using Parser 2.5.1.2, running on ruby 2.5.1 x86_64-darwin17)

[bquorning]

The travis-ci.com link requires authentication. Could you download the log file (ensure that it doesn’t contain any private information) and attach it to this issue please?

[deivid-rodriguez]

I think the solution is going to be to add something like

AllCops:
  Exclude:
   - vendor/bundle/**/*

to your rubocop configuration, but I think this issue shows room for improvement in some places:

• Rainbow should not ship its rubocop configuration file. This should be fixed at their gemspec by replacing git ls-files with a more specific list of files. By the way, the fact that git ls-files is the default for newly generated gems is a common discussion in bundler.
• Rubocop should maybe add any folders named vendor/ to its default exclusion list?

[dgmstuart]

@bquorning Oops - sorry, I've fixed the link - it should work now

[dgmstuart]

@deivid-rodriguez My understanding is that the reason for gems shipping their rubocop configuration is so that for example if somebody submits a patch then they can do so with code that matches the style of that project?

[dgmstuart]

@deivid-rodriguez I've pushed a commit with that .rubocop.yml change and it does indeed seem to fix it:

https://travis-ci.com/dgmstuart/swing-out-london/jobs/153228212

[deivid-rodriguez]

I'm glad that worked!

@deivid-rodriguez My understanding is that the reason for gems shipping their rubocop configuration is so that for example if somebody submits a patch then they can do so with code that matches the style of that project?

I think it's just unintentional (due to the fact that the default generated gemspec generated by bundler uses git ls-files). What you say makes sense as a reason to include a rubocop config in the source code repository, but unless I'm missing something I don't think it ever makes sense to actually ship that config in a released gem.

[dgmstuart]

@deivid-rodriguez Ah OK, I wasn't familiar with that distinction between what is committed and what gets "shipped" but I think I understand it - thank you for explaining.

I also now think I understand why this only showed up on CI:

Locally gems are bundled to a system location, but on travis the command is

$ bundle install --jobs=3 --retry=3 --deployment --path=${BUNDLE_PATH:-vendor/bundle}

That means that the gems are inside the application root, and so rubocop will be run over them. Rubocop allows nesting different .rubocop.yml files in subdirectories, so inside the Rainbow gem, it would just consider that to be the config that should be used to run checks inside that gem directory.

I guess this means that even if Rainbow wasn't shipping their .rubocop.yml file I'd still probably get failures since rubocop would be running with my configuration on other people's code.

[dgmstuart]

Closing:

• The suggestion of adding vendor/ to the default exclude is more of a feature request
• The issue with Rainbow is something for that project to deal with, but since realistically no project is going to want to run rubocop on third-party code I'm not sure it's actually an issue.

[dgmstuart]

@deivid-rodriguez Ah, it looks like vendor is excluded by default:
https://github.com/rubocop-hq/rubocop/blob/master/config/default.yml#L62
Added in this PR: https://github.com/rubocop-hq/rubocop/pull/806

My guess is that (like with other parts of the rubocop config) excluding _any_ file from all cops overrides _all_ the defaults.

[deivid-rodriguez]

The issue with Rainbow is something for that project to deal with, but since realistically no project is going to want to run rubocop on third-party code I'm not sure it's actually an issue.

Yup. It's not a big deal, as you said you would've got different error if they didn't ship their config. But it's something they can improve.

My guess is that (like with other parts of the rubocop config) excluding any file from all cops overrides all the defaults.

Yeah, :+1:. You could also try to change that behavior with

inherit_mode:
  merge:
    - Exclude

[dgmstuart]

Yeah, 👍. You could also try to change that behavior with

inherit_mode:
  merge:
    - Exclude

🤩 My friend, you have just improved my life. Thank you!

https://github.com/rubocop-hq/rubocop/blob/master/manual/configuration.md#merging-arrays-using-inherit_mode