Skip to content

Commit

Permalink
[Fix rubocop#11191] Add new Lint/NumericOperationWithConstantResult
Browse files Browse the repository at this point in the history
… cop
  • Loading branch information
Zopolis4 committed Nov 7, 2024
1 parent ba37a8f commit 8d30a75
Show file tree
Hide file tree
Showing 7 changed files with 257 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11191](https://github.com/rubocop/rubocop/issues/11191): Add new `Lint/BinaryOperatorWithIdenticalOperands` cop. ([@zopolis4][])
7 changes: 6 additions & 1 deletion config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1634,7 +1634,7 @@ Lint/BinaryOperatorWithIdenticalOperands:
Enabled: true
Safe: false
VersionAdded: '0.89'
VersionChanged: '1.7'
VersionChanged: '<<next>>'

Lint/BooleanSymbol:
Description: 'Check for `:true` and `:false` symbols.'
Expand Down Expand Up @@ -2146,6 +2146,11 @@ Lint/NumberedParameterAssignment:
Enabled: pending
VersionAdded: '1.9'

Lint/NumericOperationWithConstantResult:
Description: 'Checks for numeric operations with constant results.'
Enabled: pending
VersionAdded: '<<next>>'

Lint/OrAssignmentToConstant:
Description: 'Checks unintended or-assignment to constant.'
Enabled: pending
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@
require_relative 'rubocop/cop/lint/non_local_exit_from_iterator'
require_relative 'rubocop/cop/lint/number_conversion'
require_relative 'rubocop/cop/lint/numbered_parameter_assignment'
require_relative 'rubocop/cop/lint/numeric_operation_with_constant_result'
require_relative 'rubocop/cop/lint/or_assignment_to_constant'
require_relative 'rubocop/cop/lint/ordered_magic_comments'
require_relative 'rubocop/cop/lint/out_of_range_regexp_ref'
Expand Down
14 changes: 6 additions & 8 deletions lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@ module Cop
module Lint
# Checks for places where binary operator has identical operands.
#
# It covers arithmetic operators: `-`, `/`, `%`;
# comparison operators: `==`, `===`, `=~`, `>`, `>=`, `<`, ``<=``;
# It covers comparison operators: `==`, `===`, `=~`, `>`, `>=`, `<`, ``<=``;
# bitwise operators: `|`, `^`, `&`;
# boolean operators: `&&`, `||`
# and "spaceship" operator - ``<=>``.
#
# Simple arithmetic operations are allowed by this cop: `+`, `*`, `**`, `<<` and `>>`.
# Although these can be rewritten in a different way, it should not be necessary to
# do so. This does not include operations such as `-` or `/` where the result will
# always be the same (`x - x` will always be 0; `x / x` will always be 1), and
# thus are legitimate offenses.
# do so. Operations such as `-` or `/` where the result will always be the same
# (`x - x` will always be 0; `x / x` will always be 1) are offenses, but these
# are covered by Lint/NumericOperationWithConstantResult instead.
#
# @safety
# This cop is unsafe as it does not consider side effects when calling methods
Expand All @@ -30,7 +29,6 @@ module Lint
#
# @example
# # bad
# x / x
# x.top >= x.top
#
# if a.x != 0 && a.x != 0
Expand All @@ -47,13 +45,13 @@ module Lint
#
class BinaryOperatorWithIdenticalOperands < Base
MSG = 'Binary operator `%<op>s` has identical operands.'
ALLOWED_MATH_OPERATORS = %i[+ * ** << >>].to_set.freeze
MATH_OPERATORS = %i[- + * / ** << >>].to_set.freeze

def on_send(node)
return unless node.binary_operation?

lhs, operation, rhs = *node
return if ALLOWED_MATH_OPERATORS.include?(node.method_name)
return if MATH_OPERATORS.include?(node.method_name)

add_offense(node, message: format(MSG, op: operation)) if lhs == rhs
end
Expand Down
106 changes: 106 additions & 0 deletions lib/rubocop/cop/lint/numeric_operation_with_constant_result.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# Certain numeric operations have a constant result, usually 0 or 1.
# Subtracting a number from itself or multiplying it by 0 will always return 0.
# Additionally, a variable modulo 0 or itself will always return 0.
# Dividing a number by itself or raising it to the power of 0 will always return 1.
# As such, they can be replaced with that result.
# These are probably leftover from debugging, or are mistakes.
# Similar numeric operations that are leftover from debugging or mistakes
# are handled by Lint/UselessNumericOperation.
#
# @example
#
# # bad
# x - x
# x * 0
# x % 1
# x % x
#
# # good
# 0
#
# # bad
# x -= x
# x *= 0
# x %= 1
# x %= x
#
# # good
# x = 0
#
# # bad
# x / x
# x ** 0
#
# # good
# 1
#
# # bad
# x /= x
# x **= 0
#
# # good
# x = 1
#
class NumericOperationWithConstantResult < Base
extend AutoCorrector
MSG = 'Numeric operation with a constant result detected.'
RESTRICT_ON_SEND = %i[- * / % **].freeze

# @!method operation_with_constant_result?(node)
def_node_matcher :operation_with_constant_result?,
'(send (send nil? $_) $_ ({int | send nil?} $_))'

# @!method abbreviated_assignment_with_constant_result?(node)
def_node_matcher :abbreviated_assignment_with_constant_result?,
'(op-asgn (lvasgn $_) $_ ({int | lvar} $_))'

def on_send(node)
return unless operation_with_constant_result?(node)

variable, operation, number = operation_with_constant_result?(node)
result = constant_result?(variable, operation, number)
return unless result

add_offense(node) do |corrector|
corrector.replace(node, result.to_s)
end
end

def on_op_asgn(node)
return unless abbreviated_assignment_with_constant_result?(node)

variable, operation, number = abbreviated_assignment_with_constant_result?(node)
result = constant_result?(variable, operation, number)
return unless result

add_offense(node) do |corrector|
corrector.replace(node, "#{variable} = #{result}")
end
end

private

# rubocop :disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def constant_result?(variable, operation, number)
if number.to_s == '0'
return 0 if operation == :*
return 1 if operation == :**
elsif number.to_s == '1'
return 0 if operation == :%
elsif number == variable
return 0 if %i[- %].include?(operation)
return 1 if operation == :/
end
# If we weren't able to find any matches, return false so we can bail out.
false
end
# rubocop :enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
end
end
end
end
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::BinaryOperatorWithIdenticalOperands, :config do
%i[== != === <=> =~ && || - > >= < <= / % | ^].each do |operator|
%i[== != === <=> =~ && || > >= < <= | ^].each do |operator|
it "registers an offense for `#{operator}` with duplicate operands" do
expect_offense(<<~RUBY, operator: operator)
y = a.x(arg) %{operator} a.x(arg)
Expand All @@ -10,7 +10,7 @@
end
end

%i[+ * ** << >>].each do |operator|
%i[- + * / ** << >>].each do |operator|
it "does not register an offense for `#{operator}` with duplicate operands" do
expect_no_offenses(<<~RUBY)
y = a.x(arg) #{operator} a.x(arg)
Expand Down
135 changes: 135 additions & 0 deletions spec/rubocop/cop/lint/numeric_operation_with_constant_result_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::NumericOperationWithConstantResult, :config do
it 'registers an offense when a variable is subtracted from itself' do
expect_offense(<<~RUBY)
x - x
^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
0
RUBY
end

it 'registers an offense when a variable is multiplied by 0' do
expect_offense(<<~RUBY)
x * 0
^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
0
RUBY
end

it 'registers an offense when the modulo of variable and 1 is taken' do
expect_offense(<<~RUBY)
x % 1
^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
0
RUBY
end

it 'registers an offense when the modulo of variable and itself is taken' do
expect_offense(<<~RUBY)
x % x
^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
0
RUBY
end

it 'registers an offense when a variable is divided by itself' do
expect_offense(<<~RUBY)
x / x
^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
1
RUBY
end

it 'registers an offense when a variable is raised to the power of 0' do
expect_offense(<<~RUBY)
x ** 0
^^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
1
RUBY
end

it 'registers an offense when a variable is subtracted from itself via abbreviated assignment' do
expect_offense(<<~RUBY)
x -= x
^^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
x = 0
RUBY
end

it 'registers an offense when a variable is multiplied by 0 via abbreviated assignment' do
expect_offense(<<~RUBY)
x *= 0
^^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
x = 0
RUBY
end

it 'registers an offense when the modulo of variable and 1 is taken via abbreviated assignment' do
expect_offense(<<~RUBY)
x %= 1
^^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
x = 0
RUBY
end

it 'registers an offense when the modulo of variable and itself is taken via abbreviated assignment' do
expect_offense(<<~RUBY)
x %= x
^^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
x = 0
RUBY
end

it 'registers an offense when a variable is divided by itself via abbreviated assignment' do
expect_offense(<<~RUBY)
x /= x
^^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
x = 1
RUBY
end

it 'egisters an offense when a variable is raised to the power of 0 via abbreviated assignment' do
expect_offense(<<~RUBY)
x **= 0
^^^^^^^ Numeric operation with a constant result detected.
RUBY

expect_correction(<<~RUBY)
x = 1
RUBY
end
end

0 comments on commit 8d30a75

Please sign in to comment.