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.

Sign up for GitHub

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

Jump to bottom

StyleGuideBaseURL is broken for rubocop cops #107

Closed
pbstriker38 opened this issue Aug 16, 2019 · 3 comments · Fixed by #109 or rubocop/rubocop#7295
Closed

StyleGuideBaseURL is broken for rubocop cops #107

pbstriker38 opened this issue Aug 16, 2019 · 3 comments · Fixed by #109 or rubocop/rubocop#7295
Labels
bug Something isn't working

Comments

@pbstriker38
Copy link

The StyleGuideBaseURL was added here. This overwrites the one defined in rubocop.

Expected behavior

https://rubystyle.guide/#if-as-a-modifier

Actual behavior

https://rails.rubystyle.guide/#if-as-a-modifier

Steps to reproduce the problem

Run rubocop against the following code

def test
  if expected?
    return 'expected'
  end

  'not_expected'
end

rubocop (0.74.0)
rubocop-rails: (2.2.1)
Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||. (https://rubystyle.guide#if-as-a-modifier)

rubocop (0.74.0)
rubocop-rails: (2.3.0)
Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||. (https://rails.rubystyle.guide#if-as-a-modifier)

RuboCop version

0.74.0 (using Parser 2.6.3.0, running on ruby 2.5.5 x86_64-darwin17)
@koic koic added the bug Something isn't working label Aug 16, 2019
@koic
Copy link
Member

koic commented Aug 16, 2019

Thanks for the feedback. A patch to RuboCop core is required to use StyleGuideBaseURL in RuboCop Rails. I revert #88 with the new Rails Style Guide URL.

koic added a commit to koic/rubocop-rails that referenced this issue Aug 16, 2019
@koic
[Fix rubocop#107] Fix style guide URLs
9b802df 
Fixes rubocop#107.

This PR fixes style guide URLs when specifying
`rubocop --display-style-guide` option.

The following is an execution using RuboCop Rails.

```console
% bundle exec rubocop --display-style-guide --only Style/StringLiterals
Inspecting 2 files
C.

Offenses:

Gemfile:3:8: C: Style/StringLiterals: Prefer single-quoted strings when
you don't need string interpolation or special
symbols. (https://rails.rubystyle.guide#consistent-string-literals)
source "https://rubygems.org"
       ^^^^^^^^^^^^^^^^^^^^^^

2 files inspected, 1 offense detected
```

The correct URL is `https://rubystyle.guide#consistent-string-literals`
instead of `https://rails.rubystyle.guide#consistent-string-literals`.
@koic koic mentioned this issue Aug 16, 2019
Merged
8 tasks
@pbstriker38
Copy link
Author

Thanks

@koic koic mentioned this issue Aug 19, 2019
Merged
8 tasks
koic added a commit to koic/rubocop that referenced this issue Aug 19, 2019
@koic
Make it possible to set StyleGuideBaseURL per department
55c9543 
### 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.
koic added a commit to koic/rubocop-rails that referenced this issue Aug 19, 2019
@koic
[Fix rubocop#107] Fix style guide URLs
0092e77 
Fixes rubocop#107.

This PR fixes style guide URLs when specifying
`rubocop --display-style-guide` option.

The following is an execution using RuboCop Rails.

```console
% bundle exec rubocop --display-style-guide --only Style/StringLiterals
Inspecting 2 files
C.

Offenses:

Gemfile:3:8: C: Style/StringLiterals: Prefer single-quoted strings when
you don't need string interpolation or special
symbols. (https://rails.rubystyle.guide#consistent-string-literals)
source "https://rubygems.org"
       ^^^^^^^^^^^^^^^^^^^^^^

2 files inspected, 1 offense detected
```

The correct URL is `https://rubystyle.guide#consistent-string-literals`
instead of `https://rails.rubystyle.guide#consistent-string-literals`.
bbatsov pushed a commit to rubocop/rubocop that referenced this issue Aug 19, 2019
@koic @bbatsov
Make it possible to set StyleGuideBaseURL per department
95b86f8 
### 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.
koic added a commit that referenced this issue Aug 26, 2019
@koic
Merge pull request #109 from koic/fix_a_style_guild_url
e0bfc5e 
[Fix #107] Fix style guide URLs
@koic
Copy link
Member

koic commented Aug 26, 2019

RuboCop Rails 2.3.1 has been released to resolve this issue. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
2 participants
@koic @pbstriker38