forked from rubocop/rubocop
-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Closes rubocop#13050. This PR adds new `Style/BitwisePredicates` cop, which checks for usage of `&` operator in bitwise operations involving comparisons. ```ruby # bad - checks any set bits (variable & flags).positive? # good variable.anybits?(flags) # bad - checks all set bits (variable & flags) == flags # good variable.allbits?(flags) # bad - checks no set bits (variable & flags).zero? (variable & flags) == 0 # good variable.nobits?(flags) ``` I've opened a proposal in the style guide at rubocop/ruby-style-guide#944.
- Loading branch information
Showing
5 changed files
with
250 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#13050](https://github.com/rubocop/rubocop/issues/13050): Add new `Style/BitwisePredicate` cop. ([@koic][]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Style | ||
# Prefer bitwise predicate methods over direct comparison operations. | ||
# | ||
# @example | ||
# | ||
# # bad - checks any set bits | ||
# (variable & flags).positive? | ||
# | ||
# # good | ||
# variable.anybits?(flags) | ||
# | ||
# # bad - checks all set bits | ||
# (variable & flags) == flags | ||
# | ||
# # good | ||
# variable.allbits?(flags) | ||
# | ||
# # bad - checks no set bits | ||
# (variable & flags).zero? | ||
# | ||
# # good | ||
# variable.nobits?(flags) | ||
# | ||
class BitwisePredicate < Base | ||
extend AutoCorrector | ||
extend TargetRubyVersion | ||
|
||
MSG = 'Replace with `%<preferred>s`.' | ||
RESTRICT_ON_SEND = %i[positive? == > >= zero?].freeze | ||
|
||
minimum_target_ruby_version 2.5 | ||
|
||
# @!method anybits?(node) | ||
def_node_matcher :anybits?, <<~PATTERN | ||
{ | ||
(send #bit_operation? :positive?) | ||
(send #bit_operation? :> (int 0)) | ||
(send #bit_operation? :>= (int 1)) | ||
} | ||
PATTERN | ||
|
||
# @!method allbits?(node) | ||
def_node_matcher :allbits?, <<~PATTERN | ||
{ | ||
(send (begin (send _ :& _flags)) :== _flags) | ||
(send (begin (send _flags :& _)) :== _flags) | ||
} | ||
PATTERN | ||
|
||
# @!method nobits?(node) | ||
def_node_matcher :nobits?, <<~PATTERN | ||
{ | ||
(send #bit_operation? :zero?) | ||
(send #bit_operation? :== (int 0)) | ||
} | ||
PATTERN | ||
|
||
# @!method bit_operation?(node) | ||
def_node_matcher :bit_operation?, <<~PATTERN | ||
(begin | ||
(send _ :& _)) | ||
PATTERN | ||
|
||
def on_send(node) | ||
return unless node.receiver.begin_type? | ||
|
||
bit_operation = node.receiver.children.first | ||
lhs, _operator, rhs = *bit_operation | ||
|
||
if anybits?(node) | ||
preferred = "#{lhs.source}.anybits?(#{rhs.source})" | ||
elsif allbits?(node) | ||
preferred = "#{lhs.source}.allbits?(#{rhs.source})" | ||
elsif nobits?(node) | ||
preferred = "#{lhs.source}.nobits?(#{rhs.source})" | ||
else | ||
return | ||
end | ||
|
||
add_offense(node, message: format(MSG, preferred: preferred)) do |corrector| | ||
corrector.replace(node, preferred) | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,152 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Style::BitwisePredicate, :config do | ||
context 'when checking any set bits' do | ||
context 'when Ruby >= 2.5', :ruby25 do | ||
it 'registers an offense when using `&` in conjunction with `predicate` for comparisons' do | ||
expect_offense(<<~RUBY) | ||
(variable & flags).positive? | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Replace with `variable.anybits?(flags)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
variable.anybits?(flags) | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `&` in conjunction with `> 0` for comparisons' do | ||
expect_offense(<<~RUBY) | ||
(variable & flags) > 0 | ||
^^^^^^^^^^^^^^^^^^^^^^ Replace with `variable.anybits?(flags)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
variable.anybits?(flags) | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `&` in conjunction with `>= 1` for comparisons' do | ||
expect_offense(<<~RUBY) | ||
(variable & flags) >= 1 | ||
^^^^^^^^^^^^^^^^^^^^^^^ Replace with `variable.anybits?(flags)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
variable.anybits?(flags) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `anybits?` method' do | ||
expect_no_offenses(<<~RUBY) | ||
variable.anybits?(flags) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `&` in conjunction with `> 1` for comparisons' do | ||
expect_no_offenses(<<~RUBY) | ||
(variable & flags) > 1 | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when comparing with no parentheses' do | ||
expect_no_offenses(<<~RUBY) | ||
foo == bar | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when Ruby <= 2.4', :ruby24 do | ||
it 'does not register an offense when using `&` in conjunction with `predicate` for comparisons' do | ||
expect_no_offenses(<<~RUBY) | ||
(variable & flags).positive? | ||
RUBY | ||
end | ||
end | ||
end | ||
|
||
context 'when checking all set bits' do | ||
context 'when Ruby >= 2.5', :ruby25 do | ||
it 'registers an offense when using `&` with RHS flags in conjunction with `==` for comparisons' do | ||
expect_offense(<<~RUBY) | ||
(variable & flags) == flags | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Replace with `variable.allbits?(flags)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
variable.allbits?(flags) | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `&` with LHS flags in conjunction with `==` for comparisons' do | ||
expect_offense(<<~RUBY) | ||
(flags & variable) == flags | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Replace with `flags.allbits?(variable)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
flags.allbits?(variable) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `allbits?` method' do | ||
expect_no_offenses(<<~RUBY) | ||
variable.allbits?(flags) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when flag variable names are mismatched' do | ||
expect_no_offenses(<<~RUBY) | ||
(flags & variable) == flagments | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when Ruby <= 2.4', :ruby24 do | ||
it 'does not register an offense when using `&` with RHS flags in conjunction with `==` for comparisons' do | ||
expect_no_offenses(<<~RUBY) | ||
(variable & flags) == flags | ||
RUBY | ||
end | ||
end | ||
end | ||
|
||
context 'when checking no set bits' do | ||
context 'when Ruby >= 2.5', :ruby25 do | ||
it 'registers an offense when using `&` in conjunction with `zero?` for comparisons' do | ||
expect_offense(<<~RUBY) | ||
(variable & flags).zero? | ||
^^^^^^^^^^^^^^^^^^^^^^^^ Replace with `variable.nobits?(flags)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
variable.nobits?(flags) | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `&` in conjunction with `== 0` for comparisons' do | ||
expect_offense(<<~RUBY) | ||
(variable & flags) == 0 | ||
^^^^^^^^^^^^^^^^^^^^^^^ Replace with `variable.nobits?(flags)`. | ||
RUBY | ||
|
||
expect_correction(<<~RUBY) | ||
variable.nobits?(flags) | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense when using `nobits?` method' do | ||
expect_no_offenses(<<~RUBY) | ||
variable.nobits?(flags) | ||
RUBY | ||
end | ||
end | ||
|
||
context 'when Ruby <= 2.4', :ruby24 do | ||
it 'does not register an offense when using `&` in conjunction with `zero?` for comparisons' do | ||
expect_no_offenses(<<~RUBY) | ||
(variable & flags).zero? | ||
RUBY | ||
end | ||
end | ||
end | ||
end |