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

[Fix #7408] Make Gemspec/RequiredRubyVersion cop aware of Gem::Requirement #8816

Merged
merged 1 commit into from Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -16,6 +16,7 @@

* [#8646](https://github.com/rubocop-hq/rubocop/issues/8646): Faster find of all files in `TargetFinder` class which improves initial startup speed. ([@tleish][])
* [#8102](https://github.com/rubocop-hq/rubocop/issues/8102): Consider class length instead of block length for `Struct.new`. ([@tejasbubane][])
* [#7408](https://github.com/rubocop-hq/rubocop/issues/7408): Make `Gemspec/RequiredRubyVersion` cop aware of `Gem::Requirement`. ([@tejasbubane][])

## 0.92.0 (2020-09-25)

Expand Down
20 changes: 10 additions & 10 deletions lib/rubocop/cop/gemspec/required_ruby_version.rb
Expand Up @@ -58,24 +58,22 @@ class RequiredRubyVersion < Cop
(send _ :required_ruby_version= $_)
PATTERN

def_node_matcher :string_version?, <<~PATTERN
{(str _) (array (str _))}
def_node_matcher :defined_ruby_version, <<~PATTERN
{$(str _) $(array (str _) (str _))
(send (const (const nil? :Gem) :Requirement) :new $(str _))}
PATTERN

# rubocop:disable Metrics/AbcSize
def investigate(processed_source)
version = required_ruby_version(processed_source.ast).first
version_def = required_ruby_version(processed_source.ast).first

if version
return unless string_version?(version)

ruby_version = extract_ruby_version(version)

return if ruby_version == target_ruby_version.to_s
if version_def
ruby_version = extract_ruby_version(defined_ruby_version(version_def))
return if !ruby_version || ruby_version == target_ruby_version.to_s

add_offense(
processed_source.ast,
location: version.loc.expression,
location: version_def.loc.expression,
message: not_equal_message(ruby_version, target_ruby_version)
)
else
Expand All @@ -88,6 +86,8 @@ def investigate(processed_source)
private

def extract_ruby_version(required_ruby_version)
return unless required_ruby_version

if required_ruby_version.array_type?
required_ruby_version = required_ruby_version.children.detect do |v|
/[>=]/.match?(v.str_content)
Expand Down
18 changes: 18 additions & 0 deletions spec/rubocop/cop/gemspec/required_ruby_version_spec.rb
Expand Up @@ -20,6 +20,24 @@
RUBY
end

it 'registers an offense when `required_ruby_version` is specified in array and is lower than `TargetRubyVersion`' do
expect_offense(<<~RUBY, '/path/to/foo.gemspec')
Gem::Specification.new do |spec|
spec.required_ruby_version = ['>= 2.6.0', '< 3.0.0']
^^^^^^^^^^^^^^^^^^^^^^^ `required_ruby_version` (2.6, declared in foo.gemspec) and `TargetRubyVersion` (2.7, which may be specified in .rubocop.yml) should be equal.
end
RUBY
end

it 'recognizes Gem::Requirement and registers offense' do
expect_offense(<<~RUBY, '/path/to/foo.gemspec')
Gem::Specification.new do |spec|
spec.required_ruby_version = Gem::Requirement.new(">= 2.6.0")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `required_ruby_version` (2.6, declared in foo.gemspec) and `TargetRubyVersion` (2.7, which may be specified in .rubocop.yml) should be equal.
end
RUBY
end

describe 'false negatives' do
it 'does not register an offense when `required_ruby_version` ' \
'is assigned as a variable (string literal)' do
Expand Down