Only parse target Ruby from gemspec if array elements are strings

[davidrunger]

This fixes an issue mentioned in this comment on issue #12579. (I believe that the main/original issue mentioned in issue #12579 has essentially already been fixed by #12732.)

This change is a related/similar followup to #12732 (5026393). That PR avoided an exception that was occurring in the
RuboCop::TargetRuby::GemspecFile target Ruby finder when a project's gemspec determines the value of required_ruby_version in some dynamic way (such as by reading from a .ruby-version file).

However, #12732 did not handle a case where the dynamic value for the gemspec's required_ruby_version is wrapped within an array, and, in such a case, currently, an exception like the following will occur:

Gem::Requirement::BadRequirementError:
  Illformed requirement [">= \#{File.read('.ruby-version').rstrip}"]
  ./lib/rubocop/target_ruby.rb:123:in `new'
  ./lib/rubocop/target_ruby.rb:123:in `find_default_minimal_known_ruby'
  ./lib/rubocop/target_ruby.rb:83:in `find_version'
  ./lib/rubocop/target_ruby.rb:29:in `initialize'
  ./lib/rubocop/target_ruby.rb:259:in `new'

(As with the scenario that was fixed by #12732, I believe that this is essentially a "latent" issue that is now occurring in more projects than previously, due to #12645 having increased the precedence of the GemspecFile target Ruby finder in the RuboCop::TargetRuby::SOURCES list.)

This change avoids this exception by only attempting to parse a target Ruby version from an array value for a gemspec's required_ruby_version if every value in the array is a string. When the values within the required_ruby_version array are not all strings, now, rather than raising an unhandled exception, the GemspecFile finder will return nil for the target Ruby version, and so we will continue on to the other TargetRuby finder classes, in order to find a target Ruby version.

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

[davidrunger]

cc @Earlopain (Thank you for connecting the dots here between that issue #12579 and my recent PR #12732 !)

[koic]

Suggested change

	content = <<~HEREDOC
	Gem::Specification.new do |s|
	s.name = 'test'
	s.required_ruby_version = [">= \#{File.read('.ruby-version').rstrip}"]
	s.licenses = ['MIT']
	end
	HEREDOC
	content = <<~'HEREDOC'
	Gem::Specification.new do |s|
	s.name = 'test'
	s.required_ruby_version = [">= #{File.read('.ruby-version').rstrip}"]
	s.licenses = ['MIT']
	end
	HEREDOC

[davidrunger]

@koic : Thank you for the review and the good suggestion! I have applied your suggestion both to the spec that I am adding in this PR (as you suggested) and also to the similar spec just above, which I added in #12732. (Unfortunately, this makes the diff in GitHub look a little bit more complicated/confusing than necessary/ideal.)

[davidrunger]

cc @capripot ; I believe that this will fix the issue that you mentioned here.

[bbatsov]

I'd also suggest expanding the docs here a bit https://docs.rubocop.org/rubocop/1.62/configuration.html#setting-the-target-ruby-version (the section about the target version discovery is pretty short, so it might not be clear to people how this is expected to work exactly)

[davidrunger]

(This is an existing spec that I only modified to have a heredoc without interpolation; it is otherwise unchanged.)

[davidrunger]

This is the new spec that I am adding for this change. Unfortunately (due to how it looks similar to the spec changed above), GitHub is actually not even including the whole new spec in its rendering of this diff. You'll have to click the down arrow to see the last few lines of this new spec, which GitHub is rendering as if they are preexisting lines.

[bbatsov]

I had pasted the wrong link (updated) - the right one is https://docs.rubocop.org/rubocop/1.62/configuration.html#setting-the-target-ruby-version Basically I'm referring to:

Otherwise, RuboCop will then check your project for a series of files where the version may be specified already. The files that will be looked for are *.gemspec, .ruby-version, .tool-versions, and Gemfile.lock. If Gemspec file has an array for required_ruby_version, the lowest version will be used. If none of the files are found a default version value will be used.

Would be good to clarify when can RuboCop actually extract a version from the gemspec or the Gemfile.lock. We can also clarify which is the default value that will be used if inference fails.

[davidrunger]

I had pasted the wrong link (updated) - the right one is https://docs.rubocop.org/rubocop/1.62/configuration.html#setting-the-target-ruby-version Basically I'm referring to:

Otherwise, RuboCop will then check your project for a series of files where the version may be specified already. The files that will be looked for are *.gemspec, .ruby-version, .tool-versions, and Gemfile.lock. If Gemspec file has an array for required_ruby_version, the lowest version will be used. If none of the files are found a default version value will be used.

Would be good to clarify when can RuboCop actually extract a version from the gemspec or the Gemfile.lock. We can also clarify which is the default value that will be used if inference fails.

@bbatsov : I made an effort to expand the documentation a bit in 64ca11b (before reading your latest comment). I have a tendency to be too verbose, and I'm not sure I touched on the key points in the best way. Feel free to suggest any additions, deletions, or changes, or to simply make those changes on this branch yourself, if that's easier/preferable. (I have granted you permission to do so.)

[davidrunger]

We can also clarify which is the default value that will be used if inference fails.

@bbatsov : I had thought about adding that, but I didn't want to put that as a maintenance burden on you or to have that be something that we might forget to update and which would be prone to maybe becoming outdated and inaccurate. I can add that, if you'd like me to, though (or, again, feel free to add it yourself, if that's easier).

[bbatsov]

Great work!

I had thought about adding that, but I didn't want to put that as a maintenance burden on you or to have that be something that we might forget to update and which would be prone to maybe becoming outdated and inaccurate.

Fair point, although I don't expect many changes in this area going forward. Time will tell. :-)