Skip to content

Commit

Permalink
Call .value to get gemspec required_ruby_version only if str_type?
Browse files Browse the repository at this point in the history
This change avoids an exception (NoMethodError: undefined method `value'
for an instance of RuboCop::AST::SendNode) that currently occurs in
`rubocop` version 1.61.0 when the project has a gemspec file and that
gemspec specifies a `required_ruby_version` by reading from another file
(such as `.ruby-version`).

This bug began occurring in several of my projects as a result of #12645
(d1746be) ("Check gemspec `required_ruby_version` before
`.ruby-version` and other sources"). My projects have both a
`.ruby-version` file and a gemspec that sets the `required_ruby_version`
by reading from that `.ruby-version` file. Prior to #12645, the latent
bug that this change fixes was not occurring for me, because the
GemspecFile TargetRuby finder was never running, because the
RubyVersionFile TargetRuby finder had higher precedence (and a target
Ruby version was found using this finder), and so the GemspecFile
TargetRubyVersion finder was never invoked.

With #12645, because the GemspecFile finder now runs before the
RubyVersionFile finder, this exception has started occurring in these
projects, when I try to run rubocop 1.61.0.

This exception can be fixed/avoided by checking that the
`right_hand_side` is a `str_type?` before attempting to call
`right_hand_side.value`. In a setup like mentioned in my projects above,
`right_hand_side` will be a `RuboCop::AST::SendNode` and `str_type?`
will be false, so we avoid the NoMethodError exception that otherwise
will occur if we attempt to call `.value` on that node.

I also updated several of the specs in the `target_ruby_spec.rb` file.
The motivation for this change is that several of these specs were at
risk of "falsely passing", because they had spec setups that intended
for `2.7` to be returned as the target Ruby version, but Ruby 2.7 is
also currently the `RuboCop::TargetRuby::DEFAULT_VERSION`. So, in
working on the bug that is the focus of this change, although one of my
initial attempts to fix the bug substantially broke some relevant
functionality, all of the specs nevertheless still passed, because they
expected a target Ruby version of 2.7 to be returned, and it was still
being returned, even after I had broken some of the functionality,
simply because Ruby 2.7 is also the default. By setting up the specs to
expect a target Ruby version other than the default (Ruby 2.7), the
specs will now fail, if/when the relevant functionality under test is
broken.
  • Loading branch information
davidrunger committed Mar 2, 2024
1 parent c123b96 commit c3ba8dc
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 11 deletions.
@@ -0,0 +1 @@
* [#x](https://github.com/rubocop/rubocop/pull/x): Fix error determining target Ruby when gemspec `required_ruby_version` is read from another file. ([@davidrunger][])
2 changes: 1 addition & 1 deletion lib/rubocop/target_ruby.rb
Expand Up @@ -107,7 +107,7 @@ def version_from_right_hand_side(right_hand_side)
version_from_array(right_hand_side)
elsif gem_requirement_versions
gem_requirement_versions.map(&:value)
else
elsif right_hand_side.str_type?
right_hand_side.value
end
end
Expand Down
35 changes: 25 additions & 10 deletions spec/rubocop/target_ruby_spec.rb
Expand Up @@ -42,39 +42,39 @@
content = <<~HEREDOC
Gem::Specification.new do |s|
s.name = 'test'
s.required_ruby_version = '>= 2.7.2'
s.required_ruby_version = '>= 3.2.2'
s.licenses = ['MIT']
end
HEREDOC

create_file(gemspec_file_path, content)
expect(target_ruby.version).to eq 2.7
expect(target_ruby.version).to eq 3.2
end

it 'sets target_ruby from exclusive range' do
content = <<~HEREDOC
Gem::Specification.new do |s|
s.name = 'test'
s.required_ruby_version = '> 2.7.8'
s.required_ruby_version = '> 3.2.2'
s.licenses = ['MIT']
end
HEREDOC

create_file(gemspec_file_path, content)
expect(target_ruby.version).to eq 2.7
expect(target_ruby.version).to eq 3.2
end

it 'sets target_ruby from approximate version' do
content = <<~HEREDOC
Gem::Specification.new do |s|
s.name = 'test'
s.required_ruby_version = '~> 2.7.0'
s.required_ruby_version = '~> 3.2.0'
s.licenses = ['MIT']
end
HEREDOC

create_file(gemspec_file_path, content)
expect(target_ruby.version).to eq 2.7
expect(target_ruby.version).to eq 3.2
end
end

Expand Down Expand Up @@ -163,26 +163,41 @@
content = <<~HEREDOC
Gem::Specification.new do |s|
s.name = 'test'
s.required_ruby_version = ['<=3.0.4', '>=2.7.5']
s.required_ruby_version = ['<=3.3.0', '>=3.1.3']
s.licenses = ['MIT']
end
HEREDOC

create_file(gemspec_file_path, content)
expect(target_ruby.version).to eq 2.7
expect(target_ruby.version).to eq 3.1
end

it 'sets target_ruby from required_ruby_version with many requirements' do
content = <<~HEREDOC
Gem::Specification.new do |s|
s.name = 'test'
s.required_ruby_version = ['<=3.1.0', '>2.6.8', '~>2.7.1']
s.required_ruby_version = ['<=3.3.0', '>3.1.1', '~>3.2.1']
s.licenses = ['MIT']
end
HEREDOC

create_file(gemspec_file_path, content)
expect(target_ruby.version).to eq 3.2
end
end

context 'when file reads the required_ruby_version from another file' do
it 'uses the default target ruby version' do
content = <<~HEREDOC
Gem::Specification.new do |s|
s.name = 'test'
s.required_ruby_version = Gem::Requirement.new(">= \#{File.read('.ruby-version').rstrip}")
s.licenses = ['MIT']
end
HEREDOC

create_file(gemspec_file_path, content)
expect(target_ruby.version).to eq 2.7
expect(target_ruby.version).to eq default_version
end
end

Expand Down

0 comments on commit c3ba8dc

Please sign in to comment.