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

Add new `UnifiedInteger` cop #3492

Merged
merged 1 commit into from Sep 17, 2016

Conversation

Projects
None yet
3 participants
@pocke
Member

pocke commented Sep 13, 2016

Feature

This cop detects using Fixnum and Bignum class, e.g.

# test.rb
a.is_a? Fixnum
a.is_a? ::Bignum
a.is_a? MyNamespace::Bignum
$ rubocop test.rb
Inspecting 1 file
W

Offenses:

test.rb:2:9: W: Use Integer instead of Fixnum
a.is_a? Fixnum
        ^^^^^^
test.rb:3:9: W: Use Integer instead of Bignum
a.is_a? ::Bignum
        ^^^^^^^^

1 file inspected, 2 offenses detected

Target Problem

From Ruby 2.4, Fixnum and Bignum will be unified to Integer. https://bugs.ruby-lang.org/issues/12005

Fixnum and Bignum will become a reference of Integer from Ruby 2.4, e.g.

# ruby 2.4.0-preview2
Fixnum == Integer # => true
Bignum == Integer # => true

So, Using Fixnum or Bignum is confusing from Ruby 2.4.
And until Ruby 2.4, the cop is useful for migrating to Ruby 2.4.

See also #3491


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Used the same coding conventions as the rest of the project.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • All tests are passing.
  • The new code doesn't generate RuboCop offenses.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
Show outdated Hide outdated lib/rubocop/cop/lint/unified_integer.rb Outdated
Show outdated Hide outdated spec/rubocop/cop/lint/unified_integer_spec.rb Outdated
Show outdated Hide outdated spec/rubocop/cop/lint/unified_integer_spec.rb Outdated
Show outdated Hide outdated spec/rubocop/cop/lint/unified_integer_spec.rb Outdated
Show outdated Hide outdated spec/rubocop/cop/lint/unified_integer_spec.rb Outdated
@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Sep 15, 2016

Collaborator

You'll also have to rebase this on top of the current master.

Collaborator

bbatsov commented Sep 15, 2016

You'll also have to rebase this on top of the current master.

#
# # good
# 1.is_a?(Integer)
class UnifiedInteger < Cop

This comment has been minimized.

@bbatsov

bbatsov Sep 15, 2016

Collaborator

I wonder if the cop should be enabled for all Ruby versions or just for 2.4+.

@bbatsov

bbatsov Sep 15, 2016

Collaborator

I wonder if the cop should be enabled for all Ruby versions or just for 2.4+.

This comment has been minimized.

@backus

backus Sep 16, 2016

Contributor

The semantics slightly change starting in 2.4 so it probably makes most sense to only enable for 2.4 https://blog.blockscore.com/new-features-in-ruby-2-4/#simplified-integers

@backus

backus Sep 16, 2016

Contributor

The semantics slightly change starting in 2.4 so it probably makes most sense to only enable for 2.4 https://blog.blockscore.com/new-features-in-ruby-2-4/#simplified-integers

This comment has been minimized.

@pocke

pocke Sep 17, 2016

Member

I don't think so.

In many cases, using Fixnum is a mistake in Ruby 2.3 or older too. Then we can use Integer instead of Fixnum.
I've found the mistake in some projects.

e.g.

# This statement does nothing when x is a Bignum.
case x
when Fixnum # it's should be `Integer`
  do_something
when Float
  do_something2
end

So, I think this cop shoud be enabled for all Ruby versions. It is useful to detect the mistake, and migration to Ruby 2.4.

@pocke

pocke Sep 17, 2016

Member

I don't think so.

In many cases, using Fixnum is a mistake in Ruby 2.3 or older too. Then we can use Integer instead of Fixnum.
I've found the mistake in some projects.

e.g.

# This statement does nothing when x is a Bignum.
case x
when Fixnum # it's should be `Integer`
  do_something
when Float
  do_something2
end

So, I think this cop shoud be enabled for all Ruby versions. It is useful to detect the mistake, and migration to Ruby 2.4.

@pocke

This comment has been minimized.

Show comment
Hide comment
@pocke

pocke Sep 17, 2016

Member

I've fixed the message, spec, and typo. and rebased.

Member

pocke commented Sep 17, 2016

I've fixed the message, spec, and typo. and rebased.

@bbatsov bbatsov merged commit 57273d9 into rubocop-hq:master Sep 17, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Sep 17, 2016

Collaborator

👍

Collaborator

bbatsov commented Sep 17, 2016

👍

@pocke pocke deleted the pocke:bignum-fixnum-integer-cop branch Sep 17, 2016

Neodelf added a commit to Neodelf/rubocop that referenced this pull request Oct 15, 2016

bergholdt added a commit to bergholdt/artifactory-client that referenced this pull request Feb 20, 2017

Fix test to match rule Lint/UnifiedInteger
New rule introduced in Rubocop 0.43 rubocop-hq/rubocop#3492

Signed-off-by: Rasmus Bergholdt <rasmus.bergholdt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment