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.

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

Add option OnlyFor to the Bundler/GemComment cop. #7978

Merged
merged 21 commits into from May 31, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c3bfc90
Add tests for the new configuration.
ric2b May 15, 2020
91e6211
Update the cop to support 'OnlyIfVersionRestricted' configuration.
ric2b May 15, 2020
c1c90df
Update the cop's documentation.
ric2b May 15, 2020
acab071
Update docs.
ric2b May 15, 2020
e7ed684
Update CHANGELOG.md
ric2b May 15, 2020
c64c375
Fix documentation mistakes.
ric2b May 15, 2020
db46a0f
Generalize the configuration and support checking keyword arguments.
ric2b May 21, 2020
638178d
Update docs and changelog.
ric2b May 21, 2020
1cfcb73
Fix typo in changelog update.
ric2b May 24, 2020
3323708
Simplify `version_specified_gem?`
ric2b May 24, 2020
4153cd9
Update test context name to be more consistent
ric2b May 24, 2020
d52e515
Update test context name to be more consistent
ric2b May 24, 2020
7769628
Clarify `checked_options_present?` boolean expression
ric2b May 24, 2020
cd3e921
Use more rspec contexts to make test cases more readable.
ric2b May 24, 2020
dd9ec11
Merge branch 'master' into gem-comment-cop-add-pinned-only-option
ric2b May 26, 2020
c4bcc3a
Merge branch 'master' into gem-comment-cop-add-pinned-only-option
ric2b May 30, 2020
0f9be81
Merge remote-tracking branch 'origin/master' into gem-comment-cop-add…
ric2b May 30, 2020
4ffcb8b
update documentation
ric2b May 30, 2020
e5597a6
Simplify option names and update version changed to 0.85.
ric2b May 31, 2020
a308431
Add common options and link to bundler documentation.
ric2b May 31, 2020
965c6bd
Merge branch 'master' into gem-comment-cop-add-pinned-only-option
ric2b May 31, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@

### New features

* [#7978](https://github.com/rubocop-hq/rubocop/pull/7978): Add new option `OnlyWhenUsingAnyOf` to the `Bumdler/GemComment` cop. ([@ric2b][])
ric2b marked this conversation as resolved.
Show resolved Hide resolved
* [#7735](https://github.com/rubocop-hq/rubocop/issues/7735): `NodePattern` and `AST` classes have been moved to the [`rubocop-ast` gem](https://github.com/rubocop-hq/rubocop-ast). ([@marcandre][])
* [#7950](https://github.com/rubocop-hq/rubocop/pull/7950): Add new `Lint/DeprecatedOpenSSLConstant` cop. ([@bdewater][])

Expand Down Expand Up @@ -4524,3 +4525,4 @@
[@jeffcarbs]: https://github.com/jeffcarbs
[@laurmurclar]: https://github.com/laurmurclar
[@jethrodaniel]: https://github.com/jethrodaniel
[@ric2b]: https://github.com/ric2b
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -159,6 +159,7 @@ Bundler/GemComment:
- '**/Gemfile'
- '**/gems.rb'
IgnoredGems: []
OnlyWhenUsingAnyOf: []
ric2b marked this conversation as resolved.
Show resolved Hide resolved

Bundler/InsecureProtocolSource:
Description: >-
Expand Down
58 changes: 57 additions & 1 deletion lib/rubocop/cop/bundler/gem_comment.rb
Expand Up @@ -5,7 +5,13 @@ module Cop
module Bundler
# Add a comment describing each gem in your Gemfile.
#
# @example
# 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.
#
# @example OnlyWhenUsingAnyOf: [] (default)
# # bad
#
# gem 'foo'
Expand All @@ -15,17 +21,45 @@ module Bundler
# # Helpers for the foo things.
# gem 'foo'
#
# @example OnlyWhenUsingAnyOf: ['with_version_specifiers']
ric2b marked this conversation as resolved.
Show resolved Hide resolved
# # bad
#
# gem 'foo', '< 2.1'
#
# # good
#
# # Version 2.1 introduces breaking change baz
# gem 'foo', '< 2.1'
#
# @example OnlyWhenUsingAnyOf: ['with_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 = 'OnlyWhenUsingAnyOf'
VERSION_SPECIFIERS_OPTION = 'with_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 +92,28 @@ 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) \
or contains_checked_options?(node)
ric2b marked this conversation as resolved.
Show resolved Hide resolved
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? == true
ric2b marked this conversation as resolved.
Show resolved Hide resolved
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
38 changes: 38 additions & 0 deletions manual/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
74 changes: 73 additions & 1 deletion spec/rubocop/cop/bundler/gem_comment_spec.rb
Expand Up @@ -6,7 +6,8 @@
let(:cop_config) do
{
'Include' => ['**/Gemfile'],
'IgnoredGems' => ['rake']
'IgnoredGems' => ['rake'],
'OnlyWhenUsingAnyOf' => []
}
end

Expand Down Expand Up @@ -63,5 +64,76 @@
GEM
end
end

context 'and the OnlyWhenUsingAnyOf option is set' do
ric2b marked this conversation as resolved.
Show resolved Hide resolved
before { cop_config['OnlyWhenUsingAnyOf'] = checked_options }

context 'and version specifiers are checked' do
ric2b marked this conversation as resolved.
Show resolved Hide resolved
let(:checked_options) { ['with_version_specifiers'] }

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

it 'does not register an offense if an uncommented gem has no options' do
expect_no_offenses(<<-GEM, 'Gemfile')
gem 'rubocop'
GEM
end

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

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

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

it 'registers an offense if an uncommented gem has version specifiers and unrelated options' do
expect_offense(<<-GEM, 'Gemfile')
gem 'rubocop', '~> 12.0', required: true
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing gem description comment.
GEM
end
end

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

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

it 'does not register an offense if an uncommented gem has version specifiers but no other options' do
expect_no_offenses(<<-GEM, 'Gemfile')
gem 'rubocop', '~> 12.0'
GEM
end

it 'does not register an offense if an uncommented gem has only unchecked options' do
expect_no_offenses(<<-GEM, 'Gemfile')
gem 'rubocop', group: development
GEM
end
end
end
ric2b marked this conversation as resolved.
Show resolved Hide resolved
end
end