Fix #7231 unknown cop in config file will exit with code 2

[jethroo]

This PR addresses the issue raised in #7231. Loading the config file will now memorize the warning for unknown cops and will exit with status 1 if the run would otherwise exit with 0 (STATUS_SUCCESS).

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.

[bbatsov]

Shouldn't this return an error instead? I also think it's bad to reuse the same error code for unknown cop and reported offences.

[jethroo]

Shouldn't this return an error instead? I also think it's bad to reuse the same error code for unknown cop and reported offences.

yeah that was also confusing me because there is IncorrectCopNameError which seems already to be raised but for cops passed via commandline only, so I could also do:

runner_status = execute_runners
if runner_status == STATUS_SUCCESS &&
   @config_store.for(Dir.pwd).warnings.any?
  raise IncorrectCopNameError.new(@config_store.for(Dir.pwd).warnings.join(', '))
end

just let me know what you want to happen in the warning/error case ;)

[bbatsov]

I guess there's a difference between an incorrect cop name you've specified manually and something that's in your config that became obsolete, so I probably wouldn't reuse the same exception. Not sure what's the best way to approach this, as I'm not even sure that unknown cops should affect the return status unless this is an explicit error. @rubocop-hq/rubocop-core What do you think about this?

[jethroo]

as rubocop has been bumped to 0.77.0 I could see how renamed cops are handled, like in: https://travis-ci.org/jethroo/rack-mini-profiler/jobs/618208952#L275

$ bundle exec rake

Running RuboCop...

Error: The `Layout/AlignHash` cop has been renamed to `Layout/HashAlignment`.

(obsolete configuration found in .rubocop-https---raw-githubusercontent-com-discourse-discourse-master--rubocop-yml, please update it)

The `Layout/TrailingBlankLines` cop has been renamed to `Layout/TrailingEmptyLines`.

(obsolete configuration found in .rubocop-https---raw-githubusercontent-com-discourse-discourse-master--rubocop-yml, please update it)

RuboCop failed!

The command "bundle exec rake" exited with 1.

An error is logged and rubocops exits with status 2

jethroo-air ~/Workspace/rack-mini-profiler [master]$ bundle exec rubocop
Error: The `Layout/TrailingBlankLines` cop has been renamed to `Layout/TrailingEmptyLines`.
(obsolete configuration found in .rubocop-https---raw-githubusercontent-com-discourse-discourse-master--rubocop-yml, please update it)
jethroo-air ~/Workspace/rack-mini-profiler [master]$ echo $?
2

so maybe this should also happen for unknown cop since the effect on run is the same the specified cop can't be applied.

[bbatsov]

Yeah, that's exactly what I had in mind. I think it's fine to just raise an error for unknown cops.

[jethroo]

@bbatsov as discussed now a ValidationError is raised for unknown cops while loading the configuration and the run exits with code 2 now

[koic]

Thanks!