Skip to content

Commit

Permalink
Add option OnlyFor to the Bundler/GemComment cop (#7978)
Browse files Browse the repository at this point in the history
  • Loading branch information
ric2b committed May 31, 2020
1 parent 3d064b4 commit 8c3dd0b
Show file tree
Hide file tree
Showing 6 changed files with 261 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@
* [#6289](https://github.com/rubocop-hq/rubocop/issues/6289): Add new `CheckDefinitionPathHierarchy` option for `Naming/FileName`. ([@jschneid][])
* [#8069](https://github.com/rubocop-hq/rubocop/issues/8069): New option for `expect_offense` to help format offense templates. ([@marcandre][])
* [#7908](https://github.com/rubocop-hq/rubocop/pull/7908): Add new `Style/RedundantRegexpEscape` cop. ([@owst][])
* [#7978](https://github.com/rubocop-hq/rubocop/pull/7978): Add new option `OnlyFor` to the `Bundler/GemComment` cop. ([@ric2b][])

### Bug fixes

Expand Down Expand Up @@ -4553,3 +4554,4 @@
[@jethrodaniel]: https://github.com/jethrodaniel
[@CvX]: https://github.com/CvX
[@jschneid]: https://github.com/jschneid
[@ric2b]: https://github.com/ric2b
3 changes: 2 additions & 1 deletion config/default.yml
Expand Up @@ -153,12 +153,13 @@ Bundler/GemComment:
Description: 'Add a comment describing each gem.'
Enabled: false
VersionAdded: '0.59'
VersionChanged: '0.77'
VersionChanged: '0.85'
Include:
- '**/*.gemfile'
- '**/Gemfile'
- '**/gems.rb'
IgnoredGems: []
OnlyFor: []

Bundler/InsecureProtocolSource:
Description: >-
Expand Down
59 changes: 58 additions & 1 deletion docs/modules/ROOT/pages/cops_bundler.adoc
Expand Up @@ -59,13 +59,33 @@ gem 'rubocop', groups: [:development, :test]
| Yes
| No
| 0.59
| 0.77
| 0.85
|===

Add a comment describing each gem in your Gemfile.

Optionally, the "OnlyFor" configuration
can be used to only register offenses when the gems
use certain options or have version specifiers.
Add "version_specifiers" and/or the gem option names
you want to check.

A useful use-case is to enforce a comment when using
options that change the source of a gem:

- `bitbucket`
- `gist`
- `git`
- `github`
- `source`

For a full list of options supported by bundler,
you can check the https://bundler.io/man/gemfile.5.html[official documentation].

=== Examples

==== OnlyFor: [] (default)

[source,ruby]
----
# bad
Expand All @@ -78,6 +98,39 @@ gem 'foo'
gem 'foo'
----

==== OnlyFor: ['version_specifiers']

[source,ruby]
----
# bad
gem 'foo', '< 2.1'
# good
# Version 2.1 introduces breaking change baz
gem 'foo', '< 2.1'
----

==== OnlyFor: ['version_specifiers', 'github']

[source,ruby]
----
# bad
gem 'foo', github: 'some_account/some_fork_of_foo'
gem 'bar', '< 2.1'
# good
# Using this fork because baz
gem 'foo', github: 'some_account/some_fork_of_foo'
# Version 2.1 introduces breaking change baz
gem 'bar', '< 2.1'
----

=== Configurable attributes

|===
Expand All @@ -90,6 +143,10 @@ gem 'foo'
| IgnoredGems
| `[]`
| Array

| OnlyFor
| `[]`
| Array
|===

== Bundler/InsecureProtocolSource
Expand Down
38 changes: 38 additions & 0 deletions legacy-docs/cops_bundler.md
Expand Up @@ -47,8 +47,16 @@ Disabled | Yes | No | 0.59 | 0.77

Add a comment describing each gem in your Gemfile.

Optionally, the "OnlyWhenUsingAnyOf" configuration
can be used to only register offenses when the gems
use certain options or have version specifiers.
Add "with_version_specifiers" and/or the option names
(e.g. 'required', 'github', etc) you want to check.

### Examples

#### OnlyWhenUsingAnyOf: [] (default)

```ruby
# bad

Expand All @@ -59,13 +67,43 @@ gem 'foo'
# Helpers for the foo things.
gem 'foo'
```
#### OnlyWhenUsingAnyOf: ['with_version_specifiers']

```ruby
# bad

gem 'foo', '< 2.1'

# good

# Version 2.1 introduces breaking change baz
gem 'foo', '< 2.1'
```
#### OnlyWhenUsingAnyOf: ['with_version_specifiers', 'github']

```ruby
# bad

gem 'foo', github: 'some_account/some_fork_of_foo'

gem 'bar', '< 2.1'

# good

# Using this fork because baz
gem 'foo', github: 'some_account/some_fork_of_foo'

# Version 2.1 introduces breaking change baz
gem 'bar', '< 2.1'
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
Include | `**/*.gemfile`, `**/Gemfile`, `**/gems.rb` | Array
IgnoredGems | `[]` | Array
OnlyWhenUsingAnyOf | `[]` | Array

## Bundler/InsecureProtocolSource

Expand Down
71 changes: 70 additions & 1 deletion lib/rubocop/cop/bundler/gem_comment.rb
Expand Up @@ -5,7 +5,25 @@ module Cop
module Bundler
# Add a comment describing each gem in your Gemfile.
#
# @example
# Optionally, the "OnlyFor" configuration
# can be used to only register offenses when the gems
# use certain options or have version specifiers.
# Add "version_specifiers" and/or the gem option names
# you want to check.
#
# A useful use-case is to enforce a comment when using
# options that change the source of a gem:
#
# - `bitbucket`
# - `gist`
# - `git`
# - `github`
# - `source`
#
# For a full list of options supported by bundler,
# you can check the https://bundler.io/man/gemfile.5.html[official documentation].
#
# @example OnlyFor: [] (default)
# # bad
#
# gem 'foo'
Expand All @@ -15,17 +33,45 @@ module Bundler
# # Helpers for the foo things.
# gem 'foo'
#
# @example OnlyFor: ['version_specifiers']
# # bad
#
# gem 'foo', '< 2.1'
#
# # good
#
# # Version 2.1 introduces breaking change baz
# gem 'foo', '< 2.1'
#
# @example OnlyFor: ['version_specifiers', 'github']
# # bad
#
# gem 'foo', github: 'some_account/some_fork_of_foo'
#
# gem 'bar', '< 2.1'
#
# # good
#
# # Using this fork because baz
# gem 'foo', github: 'some_account/some_fork_of_foo'
#
# # Version 2.1 introduces breaking change baz
# gem 'bar', '< 2.1'
#
class GemComment < Cop
include DefNode

MSG = 'Missing gem description comment.'
CHECKED_OPTIONS_CONFIG = 'OnlyFor'
VERSION_SPECIFIERS_OPTION = 'version_specifiers'

def_node_matcher :gem_declaration?, '(send nil? :gem str ...)'

def on_send(node)
return unless gem_declaration?(node)
return if ignored_gem?(node)
return if commented?(node)
return if cop_config[CHECKED_OPTIONS_CONFIG].any? && !checked_options_present?(node)

add_offense(node)
end
Expand Down Expand Up @@ -58,6 +104,29 @@ def ignored_gem?(node)
ignored_gems = Array(cop_config['IgnoredGems'])
ignored_gems.include?(node.first_argument.value)
end

def checked_options_present?(node)
(cop_config[CHECKED_OPTIONS_CONFIG].include?(VERSION_SPECIFIERS_OPTION) &&
version_specified_gem?(node)) ||
contains_checked_options?(node)
end

# Besides the gem name, all other *positional* arguments to `gem` are version specifiers,
# as long as it has one we know there's at least one version specifier.
def version_specified_gem?(node)
# arguments[0] is the gem name
node.arguments[1]&.str_type?
end

def contains_checked_options?(node)
(Array(cop_config[CHECKED_OPTIONS_CONFIG]) & gem_options(node).map(&:to_s)).any?
end

def gem_options(node)
return [] unless node.arguments.last&.type == :hash

node.arguments.last.keys.map(&:value)
end
end
end
end
Expand Down
92 changes: 91 additions & 1 deletion spec/rubocop/cop/bundler/gem_comment_spec.rb
Expand Up @@ -4,7 +4,8 @@
let(:cop_config) do
{
'Include' => ['**/Gemfile'],
'IgnoredGems' => ['rake']
'IgnoredGems' => ['rake'],
'OnlyFor' => []
}
end

Expand Down Expand Up @@ -61,5 +62,94 @@
GEM
end
end

context 'when the "OnlyFor" option is set' do
before { cop_config['OnlyFor'] = checked_options }

context 'when the version specifiers are checked' do
let(:checked_options) { ['version_specifiers'] }

context 'when a gem is commented' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY, 'Gemfile')
# Style-guide enforcer.
gem 'rubocop'
RUBY
end
end

context 'when a gem is uncommented and has no extra options' do
it 'does not register an offense' do
expect_no_offenses(<<-GEM, 'Gemfile')
gem 'rubocop'
GEM
end
end

context 'when a gem is uncommented and has options but no version specifiers' do
it 'does not register an offense' do
expect_no_offenses(<<-GEM, 'Gemfile')
gem 'rubocop', group: development
GEM
end
end

context 'when a gem is uncommented and has a version specifier' do
it 'registers an offense' do
expect_offense(<<-GEM, 'Gemfile')
gem 'rubocop', '~> 12.0'
^^^^^^^^^^^^^^^^^^^^^^^^ Missing gem description comment.
GEM
end
end

context 'when a gem is uncommented and has multiple version specifiers' do
it 'registers an offense' do
expect_offense(<<-GEM, 'Gemfile')
gem 'rubocop', '~> 12.0', '>= 11.0'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing gem description comment.
GEM
end
end

context 'when a gem is uncommented and has a version specifier along with unrelated options' do
it 'registers an offense' do
expect_offense(<<-GEM, 'Gemfile')
gem 'rubocop', '~> 12.0', required: true
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing gem description comment.
GEM
end
end
end

context 'and some other options are checked' do
let(:checked_options) { %w[github required] }

context 'when a gem is uncommented and has one of the checked options' do
it 'registers an offense' do
expect_offense(<<-GEM, 'Gemfile')
gem 'rubocop', github: 'some_user/some_fork'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing gem description comment.
GEM
end
end

context 'when a gem is uncommented and has a version specifier but no other options' do
it 'does not register an offense' do
expect_no_offenses(<<-GEM, 'Gemfile')
gem 'rubocop', '~> 12.0'
GEM
end
end

context 'when a gem is uncommented and only unchecked options' do
it 'does not register an offense' do
expect_no_offenses(<<-GEM, 'Gemfile')
gem 'rubocop', group: development
GEM
end
end
end
end
end
end

0 comments on commit 8c3dd0b

Please sign in to comment.