Add new Gemspec/RequiredRubyVersion cop
#5057
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
koic
force-pushed
the
add_new_gemspec_required_ruby_version
branch
6 times, most recently
from
8273c66 to
5f3997c
Compare
bbatsov
reviewed
Nov 15, 2017
| # spec.required_ruby_version = ['>= 2.3.0', '< 2.5.0'] | ||
| # end | ||
| class RequiredRubyVersion < Cop | ||
| MSG = '`require_ruby_version` and `TargetRubyVersion` of ' \ |
There was a problem hiding this comment.
I'd mention in the message the actual versions that were detected. This would make it easier to understand this message.
There was a problem hiding this comment.
Certainly that information is useful, which makes it easier for users to take action against offense. I updated this message by 0d33957. Since that change had warned by Metrics/AbcSize cop, I refactored by Extract Method in this commit.
There was a problem hiding this comment.
Now the message is too informative I think. Let's shorten it to the following:
``require_ruby_version(x.x, declared in x.gemspect) andTargetRubyVersion` (x.x, declared in .rubocop.yml) should be equal.`
There was a problem hiding this comment.
Thanks for suggesting the message. I updated by c735ed0.
koic
force-pushed
the
add_new_gemspec_required_ruby_version
branch
6 times, most recently
from
0129575 to
b35d8ec
Compare
bbatsov
reviewed
Nov 16, 2017
| # end | ||
| class RequiredRubyVersion < Cop | ||
| MSG = '`required_ruby_version` (%<required_ruby_version>s, ' \ | ||
| 'declared in x.gemspec) and `TargetRubyVersion` ' \ |
There was a problem hiding this comment.
By x.gemspec I meant to replace x with the actual name of the gemspec :-)
There was a problem hiding this comment.
Oh, surely I should do that 💦 I updated by 18fa75c.
koic
force-pushed
the
add_new_gemspec_required_ruby_version
branch
2 times, most recently
from
18fa75c to
b7a291a
Compare
pocke
reviewed
Nov 17, 2017
| 'should be equal.'.freeze | ||
|
|
||
| def_node_search :required_ruby_version, <<-PATTERN | ||
| (send _ :required_ruby_version= $_) |
There was a problem hiding this comment.
This cop crashes if a version is specified as a variable.
version = '>= 2.1.0'
s.required_ruby_version = version$ rubocop rubocop.gemspec -d
For /home/pocke/ghq/github.com/bbatsov/rubocop: configuration from /home/pocke/ghq/github.com/bbatsov/rubocop/.rubocop.yml
configuration from /home/pocke/.gem/ruby/2.4.0/gems/rubocop-rspec-1.20.1/config/default.yml
configuration from /home/pocke/.gem/ruby/2.4.0/gems/rubocop-rspec-1.20.1/config/default.yml
Default configuration from /home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/config/default.yml
Inheriting configuration from /home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/config/enabled.yml
Inheriting configuration from /home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/config/disabled.yml
Inheriting configuration from /home/pocke/ghq/github.com/bbatsov/rubocop/.rubocop_todo.yml
Inspecting 1 file
Scanning /home/pocke/ghq/github.com/bbatsov/rubocop/rubocop.gemspec
undefined method `[]' for nil:NilClass
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/gemspec/required_ruby_version.rb:71:in `extract_ruby_version'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/gemspec/required_ruby_version.rb:50:in `block in investigate'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/gemspec/required_ruby_version.rb:48:in `block in required_ruby_version'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:528:in `block in visit_descendants'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:173:in `block in each_child_node'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:171:in `each'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:171:in `each_child_node'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:527:in `visit_descendants'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:529:in `block in visit_descendants'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:173:in `block in each_child_node'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:171:in `each'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:171:in `each_child_node'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:527:in `visit_descendants'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:529:in `block in visit_descendants'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:173:in `block in each_child_node'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:171:in `each'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:171:in `each_child_node'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:527:in `visit_descendants'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/ast/node.rb:248:in `each_node'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/gemspec/required_ruby_version.rb:46:in `required_ruby_version'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/gemspec/required_ruby_version.rb:49:in `investigate'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/commissioner.rb:100:in `block (2 levels) in invoke_custom_processing'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/commissioner.rb:109:in `with_cop_error_handling'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/commissioner.rb:99:in `block in invoke_custom_processing'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/commissioner.rb:96:in `each'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/commissioner.rb:96:in `invoke_custom_processing'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/commissioner.rb:58:in `investigate'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/team.rb:114:in `investigate'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/team.rb:102:in `offenses'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cop/team.rb:44:in `inspect_file'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:258:in `inspect_file'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:205:in `block in do_inspection_loop'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:237:in `block in iterate_until_no_changes'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:230:in `loop'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:230:in `iterate_until_no_changes'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:201:in `do_inspection_loop'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:111:in `block in file_offenses'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:121:in `file_offense_cache'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:109:in `file_offenses'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:100:in `process_file'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:78:in `block in each_inspected_file'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:75:in `each'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:75:in `reduce'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:75:in `each_inspected_file'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:67:in `inspect_files'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/runner.rb:39:in `run'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cli.rb:128:in `execute_runner'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cli.rb:60:in `execute_runners'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/lib/rubocop/cli.rb:31:in `run'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/bin/rubocop:13:in `block in <top (required)>'
/usr/lib/ruby/2.4.0/benchmark.rb:308:in `realtime'
/home/pocke/.gem/ruby/2.4.0/gems/rubocop-0.51.0/bin/rubocop:12:in `<top (required)>'
/home/pocke/.gem/ruby/2.4.0/bin/rubocop:23:in `load'
/home/pocke/.gem/ruby/2.4.0/bin/rubocop:23:in `<main>'
.
1 file inspected, no offenses detected
Finished in 0.3149815270007821 secondsSo I think the cop should check the argument type, the argument should be a string literal or an array of string literal.
There was a problem hiding this comment.
I did not notice this crashed case. I treated this case as a false positive.
I fixed it along with other improvements.
2315dad
pocke
reviewed
Nov 17, 2017
| end | ||
| end | ||
|
|
||
| required_ruby_version.children.last.match(/(\d\.\d)(\.\d)?/)[1] |
There was a problem hiding this comment.
/(\d\.\d)(\.\d)?/
I guess we can omit the last (\.\d)?. It matches same string even if omit the part of regexp.
pocke
reviewed
Nov 17, 2017
| end | ||
| end | ||
|
|
||
| required_ruby_version.children.last.match(/(\d\.\d)(\.\d)?/)[1] |
There was a problem hiding this comment.
We can use str_content method instead of required_ruby_version.children.last.
https://github.com/bbatsov/rubocop/blob/b7a291ab441e415e5657bf92710e4644803e604a/lib/rubocop/ast/node.rb#L273
BTW, probably the method should be an node extension. e.g. StrNode#content
pocke
reviewed
Nov 17, 2017
| subject(:cop) { described_class.new(config) } | ||
|
|
||
| before do | ||
| allow(File).to receive(:basename).with('(string)').and_return('foo.gemspec') |
There was a problem hiding this comment.
https://github.com/bbatsov/rubocop/blob/b7a291ab441e415e5657bf92710e4644803e604a/lib/rubocop/rspec/expect_offense.rb#L47
Can you try passing file name as expect_offense's 2nd argument instead of stub? It probably works well, and it does not magical.
koic
force-pushed
the
add_new_gemspec_required_ruby_version
branch
4 times, most recently
from
94ce550 to
2315dad
Compare
## Feature
Checks that `required_ruby_version` of gemspec and `TargetRubyVersion`
of .rubocop.yml are equal.
Thereby, RuboCop to perform static analysis working on the version
required by gemspec.
For example, when `TargetRubyVersion` of .rubocop.yml is `2.3`.
```console
% cat foo.gemspec
# frozen_string_literal: true
Gem::Specification.new do |spec|
spec.required_ruby_version = '>= 2.4.0'
end
```
```console
% rubocop foo.gemspec
Inspecting 1 file
C
Offenses:
foo.gemspec:4:32: C: required_ruby_version (2.4, declared in foo.gemspec)
and TargetRubyVersion (2.3, declared in .rubocop.yml) should be equal.
spec.required_ruby_version = '>= 2.4.0'
^^^^^^^^^^
1 file inspected, 1 offense detected
```
## Other Information
The following is the RubyGems specification reference.
http://guides.rubygems.org/specification-reference/#required_ruby_version=
Refine the offense message of `Gemspec/RequiredRubyVersion` cop
koic
force-pushed
the
add_new_gemspec_required_ruby_version
branch
from
2315dad to
633983c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature
Checks that
require_ruby_versionof gemspec andTargetRubyVersionof .rubocop.yml are equal.Thereby, RuboCop to perform static analysis working to the version required by gemspec.
For example, when
TargetRubyVersionof .rubocop.yml is2.3.Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).rake spec) are passing.rake internal_investigation.and description in grammatically correct, complete sentences.
rake generate_cops_documentation(required only when you've added a new cop or changed the configuration/documentation of an existing cop).