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

New cop: RUBY_VERSION in gemspec #7137

Closed
pocke opened this issue Jun 13, 2019 · 2 comments
Closed

New cop: RUBY_VERSION in gemspec #7137

pocke opened this issue Jun 13, 2019 · 2 comments

Comments

@pocke
Copy link
Collaborator

pocke commented Jun 13, 2019

Is your feature request related to a problem? Please describe.

Gemspec should not contain RUBY_VERSION as a condition to switch dependencies.
For example:

Gem::Specification.new do |s|
  if RUBY_VERSION >= '2.5'
    s.add_runtime_dependency 'gem_a'
  else
    s.add_runtime_dependency 'gem_b'
  end
end

It has unexpected behavior. The RUBY_VERSION is determined by rake release.
If I use Ruby 2.5 to execute rake release, the gem always has gem_a as a dependency even if a user installs the gem with Ruby 2.4 or older.

We have several ways to fix the problem.

  • Remove the dependencies and use post_install_message to notice the gems.
  • Add both gems to the dependency.
  • If the dependencies are for development, move them to Gemfile.

Describe the solution you'd like

Detect reference of RUBY_VERSION in *.gemspec and add an offense to it.
For example: def_node_matcher :ruby_version?, '(const nil :RUBY_VERSION)'

Describe alternatives you've considered

The solution is loose. It checks RUBY_VERSION in anywhere of gemspec.
We can implement stricter detection. For example, detect RUBY_VERSION only if it is placed a conditon.

But I think the looser version is better. Because I guess the cop will have many false negative if we choose the stricter version. And I guess the looser version does not have too many false positives.

Additional context

Today I found using RUBY_VERSION in a gemspec.
rafaelsales/ulid#17
So I think the cop is meaningful.

@bbatsov
Copy link
Collaborator

bbatsov commented Jun 14, 2019

You've got my blessing! :-)

I was also thinking we should add a section on Gemfile/gemspec to https://rubystyle.guide/. Our cops have a lot of good advice that's not part of the guide yet.

malyshkosergey added a commit to malyshkosergey/rubocop that referenced this issue Jun 14, 2019
@pocke
Copy link
Collaborator Author

pocke commented Jun 15, 2019

I was also thinking we should add a section on Gemfile/gemspec to https://rubystyle.guide/. Our cops have a lot of good advice that's not part of the guide yet.

Definitely! It will be helpful for beginners of gem developer.

malyshkosergey added a commit to malyshkosergey/rubocop that referenced this issue Jun 22, 2019
malyshkosergey added a commit to malyshkosergey/rubocop that referenced this issue Jun 22, 2019
malyshkosergey added a commit to malyshkosergey/rubocop that referenced this issue Jun 23, 2019
mishina2228 added a commit to mishina2228/mocha that referenced this issue Apr 22, 2022
Gemspec should not contain `RUBY_VERSION` as a condition to switch dependencies,
because it is meaningless.

Please see the following for details:
- rubocop/ruby-style-guide#763
- rubocop/rubocop#7137
mishina2228 added a commit to mishina2228/mocha that referenced this issue Apr 22, 2022
Gemspec should not contain `RUBY_VERSION` as a condition to switch dependencies,
because it is meaningless.
Please see the following for details:
- rubocop/rubocop#7137
- rubocop/ruby-style-guide#763

In addition, Bundler no longer uses `add_development_dependency` when genererating new gems.
- rubygems/bundler#7222

For these reason, this PR moves development dependencies from gemspec to Gemfile.
floehopper pushed a commit to freerange/mocha that referenced this issue Apr 25, 2022
Gemspec should not contain `RUBY_VERSION` as a condition to switch dependencies,
because it is meaningless.
Please see the following for details:
- rubocop/rubocop#7137
- rubocop/ruby-style-guide#763

In addition, Bundler no longer uses `add_development_dependency` when genererating new gems.
- rubygems/bundler#7222

For these reason, this PR moves development dependencies from gemspec to Gemfile.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants