From 03c44cb253471c5091715df5d0946f8783b59a9c Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 18 Jul 2024 00:56:32 +0900 Subject: [PATCH] [Fix #13050] Add new `Style/BitwisePredicate` cop Closes #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 https://github.com/rubocop/ruby-style-guide/pull/944. --- ...new_add_new_style_bitwise_predicate_cop.md | 1 + config/default.yml | 5 + lib/rubocop.rb | 1 + lib/rubocop/cop/style/bitwise_predicate.rb | 95 +++++++++++ .../cop/style/bitwise_predicate_spec.rb | 152 ++++++++++++++++++ 5 files changed, 254 insertions(+) create mode 100644 changelog/new_add_new_style_bitwise_predicate_cop.md create mode 100644 lib/rubocop/cop/style/bitwise_predicate.rb create mode 100644 spec/rubocop/cop/style/bitwise_predicate_spec.rb diff --git a/changelog/new_add_new_style_bitwise_predicate_cop.md b/changelog/new_add_new_style_bitwise_predicate_cop.md new file mode 100644 index 0000000000000..f49cd563fe5b3 --- /dev/null +++ b/changelog/new_add_new_style_bitwise_predicate_cop.md @@ -0,0 +1 @@ +* [#13050](https://github.com/rubocop/rubocop/issues/13050): Add new `Style/BitwisePredicate` cop. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index c49fb6be3fdbf..b889dd089010c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -3232,6 +3232,11 @@ Style/BisectedAttrAccessor: Enabled: true VersionAdded: '0.87' +Style/BitwisePredicate: + Description: 'Prefer bitwise predicate methods over direct comparison operations.' + Enabled: pending + VersionAdded: '<>' + Style/BlockComments: Description: 'Do not use block comments.' StyleGuide: '#no-block-comments' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 4566f4c2873f8..5553e6496d4c6 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -466,6 +466,7 @@ require_relative 'rubocop/cop/style/bare_percent_literals' require_relative 'rubocop/cop/style/begin_block' require_relative 'rubocop/cop/style/bisected_attr_accessor' +require_relative 'rubocop/cop/style/bitwise_predicate' require_relative 'rubocop/cop/style/block_comments' require_relative 'rubocop/cop/style/block_delimiters' require_relative 'rubocop/cop/style/case_equality' diff --git a/lib/rubocop/cop/style/bitwise_predicate.rb b/lib/rubocop/cop/style/bitwise_predicate.rb new file mode 100644 index 0000000000000..678240c9111d9 --- /dev/null +++ b/lib/rubocop/cop/style/bitwise_predicate.rb @@ -0,0 +1,95 @@ +# 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 `%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? + return unless (preferred_method = preferred_method(node)) + + bit_operation = node.receiver.children.first + lhs, _operator, rhs = *bit_operation + preferred = "#{lhs.source}.#{preferred_method}(#{rhs.source})" + + add_offense(node, message: format(MSG, preferred: preferred)) do |corrector| + corrector.replace(node, preferred) + end + end + + private + + def preferred_method(node) + if anybits?(node) + 'anybits?' + elsif allbits?(node) + 'allbits?' + elsif nobits?(node) + 'nobits?' + end + end + end + end + end +end diff --git a/spec/rubocop/cop/style/bitwise_predicate_spec.rb b/spec/rubocop/cop/style/bitwise_predicate_spec.rb new file mode 100644 index 0000000000000..139c92fac8dbf --- /dev/null +++ b/spec/rubocop/cop/style/bitwise_predicate_spec.rb @@ -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, unsupported_on: :prism 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, unsupported_on: :prism 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, unsupported_on: :prism 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