Skip to content

Commit

Permalink
Make it possible to set StyleGuideBaseURL per department
Browse files Browse the repository at this point in the history
### Summary

Resolves rubocop/rubocop-rails#107
and follow up rubocop/rubocop-rails#109 (comment).

This PR makes it possible to set a style guide URL (`StyleGuideBaseURL`)
for each department.

For example, RuboCop Rails gem (Rails department) supports
The Rails Style Guide. In that case, set .rubocop.yml as follows.

```yaml
# .rubocop.yml
require:
  - rubocop-rails

AllCops:
  StyleGuideBaseURL:  https://rubystyle.guide

Rails:
  StyleGuideBaseURL:  https://rails.rubystyle.guide
```

In practice `Rails: StyleGuideBaseURL: https://rails.rubystyle.guide`
will be set in config/default.yml of rubocop-rails.

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

time = Time.new
```

```console
% bundle exec rubocop --display-style-guide example.rb
Inspecting 1 file
W

Offenses:

example.rb:3:1: W: Lint/UselessAssignment: Useless assignment to
variable - time. (https://rubystyle.guide#underscore-unused-vars)
time = Time.new
^^^^
example.rb:3:13: C: Rails/TimeZone: Do not use Time.new without
zone. Use one of Time.zone.now, Time.current, Time.new.in_time_zone,
Time.new.utc, Time.new.getlocal, Time.new.xmlschema, Time.new.iso8601,
Time.new.jisx0301, Time.new.rfc3339, Time.new.httpdate, Time.new.to_i,
Time.new.to_f instead. (https://rails.rubystyle.guide#time,
http://danilenko.org/2012/7/6/rails_timezones)
time = Time.new
            ^^^

1 file inspected, 2 offenses detected
```

The style guide URLs set for each department is displayed.

- https://rubystyle.guide#underscore-unused-vars for `Lint/UselessAssignment` cop
- https://rails.rubystyle.guide#time for for `Rails/TimeZone` cop

So `AllCops` style guide URL is used when there is no style guide URL
for a specific department.

### Other Information

Note that tasks/cops_documentation.rake will require the following patch:

```diff
diff --git a/tasks/cops_documentation.rake
b/tasks/cops_documentation.rake
index bd93ffe31..751222fcb 100644
--- a/tasks/cops_documentation.rake
+++ b/tasks/cops_documentation.rake
@@ -155,7 +155,9 @@ task generate_cops_documentation:
:yard_for_generate_documentation do

   def references(config, cop)
     cop_config = config.for_cop(cop)
-    urls = RuboCop::Cop::MessageAnnotator.new(config, cop_config, {}).urls
+    urls = RuboCop::Cop::MessageAnnotator.new(
+      config, cop.name, cop_config, {}
+    ).urls
     return '' if urls.empty?

     content = h3('References')
```

AFAIK, rubocop-performance, rubocop-rails, rubocop-rspec, and
rubocop-minitest repositories are targeted.
  • Loading branch information
koic committed Aug 19, 2019
1 parent 28f8d94 commit 55c9543
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

* [#7274](https://github.com/rubocop-hq/rubocop/issues/7274): Add new `Lint/SendWithMixinArgument` cop. ([@koic][])
* [#7272](https://github.com/rubocop-hq/rubocop/pull/7272): Show warning message if passed string to `Enabled`, `Safe`, `SafeAutocorrect`, and `AutoCorrect` keys in .rubocop.yml. ([@unasuke][])
* [#7295](https://github.com/rubocop-hq/rubocop/pull/7295): Make it possible to set `StyleGuideBaseURL` per department. ([@koic][])

### Bug fixes

Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ def for_cop(cop)
@for_cop[cop.respond_to?(:cop_name) ? cop.cop_name : cop]
end

def for_department(department_name)
@for_cop[department_name]
end

def for_all_cops
@for_all_cops ||= self['AllCops'] || {}
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ def excluded_file?(file)

def annotate(message)
RuboCop::Cop::MessageAnnotator.new(
config, cop_config, @options
).annotate(message, name)
config, cop_name, cop_config, @options
).annotate(message)
end

def file_name_matches_any?(file, parameter, default_result)
Expand Down
23 changes: 16 additions & 7 deletions lib/rubocop/cop/message_annotator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ module Cop
#
# @example
# RuboCop::Cop::MessageAnnotator.new(
# config, cop_config, @options
# ).annotate('message', 'Cop/CopName')
# config, cop_name, cop_config, @options
# ).annotate('message')
# #=> 'Cop/CopName: message (http://example.org/styleguide)'
class MessageAnnotator
attr_reader :options, :config, :cop_config
attr_reader :options, :config, :cop_name, :cop_config

@style_guide_urls = {}

Expand All @@ -29,6 +29,7 @@ class << self
# :ExtraDetails [Boolean] Include cop details
# :DisplayCopNames [Boolean] Include cop name
#
# @param [String] cop_name for specific cop name
# @param [Hash] cop_config configs for specific cop, from config#for_cop
# @option cop_config [String] :StyleGuide Extension of base styleguide URL
# @option cop_config [String] :Reference Full reference URL
Expand All @@ -43,8 +44,9 @@ class << self
# Include debug output
# @option options [Boolean] :display_cop_names
# Include cop name
def initialize(config, cop_config, options)
def initialize(config, cop_name, cop_config, options)
@config = config
@cop_name = cop_name
@cop_config = cop_config || {}
@options = options
end
Expand All @@ -53,8 +55,8 @@ def initialize(config, cop_config, options)
# based on params passed into initializer
#
# @return [String] annotated message
def annotate(message, name)
message = "#{name}: #{message}" if display_cop_names?
def annotate(message)
message = "#{cop_name}: #{message}" if display_cop_names?
message += " #{details}" if extra_details? && details
if display_style_guide?
links = urls.join(', ')
Expand All @@ -74,7 +76,7 @@ def style_guide_url
return nil if url.nil? || url.empty?

self.class.style_guide_urls[url] ||= begin
base_url = config.for_all_cops['StyleGuideBaseURL']
base_url = style_guide_base_url
if base_url.nil? || base_url.empty?
url
else
Expand All @@ -83,6 +85,13 @@ def style_guide_url
end
end

def style_guide_base_url
department_name = cop_name.split('/').first

config.for_department(department_name)['StyleGuideBaseURL'] ||
config.for_all_cops['StyleGuideBaseURL']
end

def display_style_guide?
(options[:display_style_guide] ||
config.for_all_cops['DisplayStyleGuide']) &&
Expand Down
32 changes: 29 additions & 3 deletions spec/rubocop/cop/message_annotator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
RSpec.describe RuboCop::Cop::MessageAnnotator do
let(:options) { {} }
let(:config) { RuboCop::Config.new({}) }
let(:annotator) { described_class.new(config, config['Cop/Cop'], options) }
let(:cop_name) { 'Cop/Cop' }
let(:annotator) do
described_class.new(config, cop_name, config[cop_name], options)
end

describe '#annotate' do
subject(:annotate) do
annotator.annotate('message', 'Cop/Cop')
annotator.annotate('message')
end

context 'with default options' do
Expand Down Expand Up @@ -55,9 +58,10 @@

describe 'with style guide url' do
subject(:annotate) do
annotator.annotate('', 'Cop/Cop')
annotator.annotate('')
end

let(:cop_name) { 'Cop/Cop' }
let(:options) do
{
display_style_guide: true
Expand Down Expand Up @@ -103,6 +107,28 @@
expect(annotate).to include('http://example.org/styleguide#target_based_url')
end

context 'when a department other than AllCops is specified' do
let(:config) do
RuboCop::Config.new(
'AllCops' => {
'StyleGuideBaseURL' => 'http://example.org/styleguide'
},
'Foo' => {
'StyleGuideBaseURL' => 'http://foo.example.org'
}
)
end

let(:cop_name) { 'Foo/Cop' }
let(:urls) { annotator.urls }

it 'returns style guide url when it is specified' do
config['Foo/Cop'] = { 'StyleGuide' => '#target_style_guide' }

expect(urls).to eq(%w[http://foo.example.org#target_style_guide])
end
end

it 'can use a path-based setting' do
config['Cop/Cop'] = { 'StyleGuide' => 'cop/path/rule#target_based_url' }
expect(annotate).to include('http://example.org/cop/path/rule#target_based_url')
Expand Down
4 changes: 3 additions & 1 deletion tasks/cops_documentation.rake
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ task generate_cops_documentation: :yard_for_generate_documentation do

def references(config, cop)
cop_config = config.for_cop(cop)
urls = RuboCop::Cop::MessageAnnotator.new(config, cop_config, {}).urls
urls = RuboCop::Cop::MessageAnnotator.new(
config, cop.name, cop_config, {}
).urls
return '' if urls.empty?

content = h3('References')
Expand Down

0 comments on commit 55c9543

Please sign in to comment.