Skip to content

Commit

Permalink
Add new Lint/BinaryOperatorWithIdenticalOperands cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Aug 5, 2020
1 parent bf47e35 commit d94c4a4
Show file tree
Hide file tree
Showing 11 changed files with 139 additions and 91 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@
* [#8322](https://github.com/rubocop-hq/rubocop/pull/8322): Support autocorrect for `Style/CaseEquality` cop. ([@fatkodima][])
* [#8291](https://github.com/rubocop-hq/rubocop/pull/8291): Add new `Lint/SelfAssignment` cop. ([@fatkodima][])
* [#8389](https://github.com/rubocop-hq/rubocop/pull/8389): Add new `Lint/DuplicateRescueException` cop. ([@fatkodima][])
* [#8433](https://github.com/rubocop-hq/rubocop/pull/8433): Add new `Lint/BinaryOperatorWithIdenticalOperands` cop. ([@fatkodima][])
* [#8430](https://github.com/rubocop-hq/rubocop/pull/8430): Add new `Lint/UnreachableLoop` cop. ([@fatkodima][])
* [#8412](https://github.com/rubocop-hq/rubocop/pull/8412): Add new `Style/OptionalBooleanParameter` cop. ([@fatkodima][])
* [#8432](https://github.com/rubocop-hq/rubocop/pull/8432): Add new `Lint/FloatComparison` cop. ([@fatkodima][])
Expand Down Expand Up @@ -45,6 +46,7 @@
### Changes

* [#8376](https://github.com/rubocop-hq/rubocop/pull/8376): `Style/MethodMissingSuper` cop is removed in favor of new `Lint/MissingSuper` cop. ([@fatkodima][])
* [#8433](https://github.com/rubocop-hq/rubocop/pull/8433): `Lint/UselessComparison` cop is removed in favor of new `Lint/BinaryOperatorWithIdenticalOperands` cop. ([@fatkodima][])
* [#8350](https://github.com/rubocop-hq/rubocop/pull/8350): Set default max line length to 120 for `Style/MultilineMethodSignature`. ([@koic][])
* [#8338](https://github.com/rubocop-hq/rubocop/pull/8338): **potentially breaking**. Config#for_department now returns only the config specified for that department; the 'Enabled' attribute is no longer calculated. ([@marcandre][])
* [#8037](https://github.com/rubocop-hq/rubocop/pull/8037): **(Breaking)** Cop `Metrics/AbcSize` now counts ||=, &&=, multiple assignments, for, yield, iterating blocks. `&.` now count as conditions too (unless repeated on the same variable). Default bumped from 15 to 17. Consider using `rubocop -a --disable-uncorrectable` to ease transition. ([@marcandre][])
Expand Down
11 changes: 6 additions & 5 deletions config/default.yml
Expand Up @@ -1355,6 +1355,12 @@ Lint/BigDecimalNew:
Enabled: true
VersionAdded: '0.53'

Lint/BinaryOperatorWithIdenticalOperands:
Description: 'This cop checks for places where binary operator has identical operands.'
Enabled: pending
Safe: false
VersionAdded: '0.89'

Lint/BooleanSymbol:
Description: 'Check for `:true` and `:false` symbols.'
Enabled: true
Expand Down Expand Up @@ -1906,11 +1912,6 @@ Lint/UselessAssignment:
Enabled: true
VersionAdded: '0.11'

Lint/UselessComparison:
Description: 'Checks for comparison of something with itself.'
Enabled: true
VersionAdded: '0.11'

Lint/UselessElseWithoutRescue:
Description: 'Checks for useless `else` in `begin..end` without `rescue`.'
Enabled: true
Expand Down
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -185,6 +185,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintambiguousregexpliteral[Lint/AmbiguousRegexpLiteral]
* xref:cops_lint.adoc#lintassignmentincondition[Lint/AssignmentInCondition]
* xref:cops_lint.adoc#lintbigdecimalnew[Lint/BigDecimalNew]
* xref:cops_lint.adoc#lintbinaryoperatorwithidenticaloperands[Lint/BinaryOperatorWithIdenticalOperands]
* xref:cops_lint.adoc#lintbooleansymbol[Lint/BooleanSymbol]
* xref:cops_lint.adoc#lintcircularargumentreference[Lint/CircularArgumentReference]
* xref:cops_lint.adoc#lintconstantresolution[Lint/ConstantResolution]
Expand Down Expand Up @@ -269,7 +270,6 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#linturiregexp[Lint/UriRegexp]
* xref:cops_lint.adoc#lintuselessaccessmodifier[Lint/UselessAccessModifier]
* xref:cops_lint.adoc#lintuselessassignment[Lint/UselessAssignment]
* xref:cops_lint.adoc#lintuselesscomparison[Lint/UselessComparison]
* xref:cops_lint.adoc#lintuselesselsewithoutrescue[Lint/UselessElseWithoutRescue]
* xref:cops_lint.adoc#lintuselesssettercall[Lint/UselessSetterCall]
* xref:cops_lint.adoc#lintvoid[Lint/Void]
Expand Down
63 changes: 40 additions & 23 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -211,6 +211,46 @@ BigDecimal.new(123.456, 3)
BigDecimal(123.456, 3)
----

== Lint/BinaryOperatorWithIdenticalOperands

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| No
| No
| 0.89
| -
|===

This cop checks for places where binary operator has identical operands.

It covers arithmetic operators: `+`, `-`, `*`, `/`, `%`, `**`;
comparison operators: `==`, `===`, `=~`, `>`, `>=`, `<`, `<=`;
bitwise operators: `|`, `^`, `&`, `<<`, `>>`;
boolean operators: `&&`, `||`
and "spaceship" operator - `<=>`.

This cop is marked as unsafe as it does not consider side effects when calling methods
and thus can generate false positives:
if wr.take_char == '\0' && wr.take_char == '\0'

=== Examples

[source,ruby]
----
# bad
x.top >= x.top
if a.x != 0 && a.x != 0
do_something
end
def childs?
left_child || left_child
end
----

== Lint/BooleanSymbol

|===
Expand Down Expand Up @@ -4282,29 +4322,6 @@ end

* https://rubystyle.guide#underscore-unused-vars

== Lint/UselessComparison

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Enabled
| Yes
| No
| 0.11
| -
|===

This cop checks for comparison of something with itself.

=== Examples

[source,ruby]
----
# bad
x.top >= x.top
----

== Lint/UselessElseWithoutRescue

|===
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop.rb
Expand Up @@ -242,6 +242,7 @@
require_relative 'rubocop/cop/lint/ambiguous_regexp_literal'
require_relative 'rubocop/cop/lint/assignment_in_condition'
require_relative 'rubocop/cop/lint/big_decimal_new'
require_relative 'rubocop/cop/lint/binary_operator_with_identical_operands'
require_relative 'rubocop/cop/lint/boolean_symbol'
require_relative 'rubocop/cop/lint/circular_argument_reference'
require_relative 'rubocop/cop/lint/constant_resolution'
Expand Down Expand Up @@ -326,7 +327,6 @@
require_relative 'rubocop/cop/lint/uri_regexp'
require_relative 'rubocop/cop/lint/useless_access_modifier'
require_relative 'rubocop/cop/lint/useless_assignment'
require_relative 'rubocop/cop/lint/useless_comparison'
require_relative 'rubocop/cop/lint/useless_else_without_rescue'
require_relative 'rubocop/cop/lint/useless_setter_call'
require_relative 'rubocop/cop/lint/void'
Expand Down
5 changes: 4 additions & 1 deletion lib/rubocop/config_obsoletion.rb
Expand Up @@ -86,7 +86,10 @@ class ConfigObsoletion
'it was a duplicate of `Layout/SpaceBeforeFirstArg`. Please use ' \
'`Layout/SpaceBeforeFirstArg` instead',
'Style/MethodMissingSuper' => 'it has been superseded by `Lint/MissingSuper`. Please use ' \
'`Lint/MissingSuper` instead'
'`Lint/MissingSuper` instead',
'Lint/UselessComparison' => 'it has been superseded by '\
'`Lint/BinaryOperatorWithIdenticalOperands`. Please use '\
'`Lint/BinaryOperatorWithIdenticalOperands` instead'
}.map do |cop_name, reason|
[cop_name, "The `#{cop_name}` cop has been removed since #{reason}."]
end
Expand Down
49 changes: 49 additions & 0 deletions lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb
@@ -0,0 +1,49 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# This cop checks for places where binary operator has identical operands.
#
# It covers arithmetic operators: `+`, `-`, `*`, `/`, `%`, `**`;
# comparison operators: `==`, `===`, `=~`, `>`, `>=`, `<`, `<=`;
# bitwise operators: `|`, `^`, `&`, `<<`, `>>`;
# boolean operators: `&&`, `||`
# and "spaceship" operator - `<=>`.
#
# This cop is marked as unsafe as it does not consider side effects when calling methods
# and thus can generate false positives:
# if wr.take_char == '\0' && wr.take_char == '\0'
#
# @example
# # bad
# x.top >= x.top
#
# if a.x != 0 && a.x != 0
# do_something
# end
#
# def childs?
# left_child || left_child
# end
#
class BinaryOperatorWithIdenticalOperands < Base
MSG = 'Binary operator `%<op>s` has identical operands.'

def on_send(node)
return unless node.binary_operation?

lhs, operation, rhs = *node
return if node.arithmetic_operation? && lhs.basic_literal? && rhs.basic_literal?

add_offense(node, message: format(MSG, op: operation)) if lhs == rhs
end

def on_and(node)
add_offense(node, message: format(MSG, op: node.operator)) if node.lhs == node.rhs
end
alias on_or on_and
end
end
end
end
28 changes: 0 additions & 28 deletions lib/rubocop/cop/lint/useless_comparison.rb

This file was deleted.

3 changes: 3 additions & 0 deletions spec/rubocop/config_obsoletion_spec.rb
Expand Up @@ -70,6 +70,7 @@
'Layout/SpaceBeforeModifierKeyword' => { 'Enabled': true },
'Lint/InvalidCharacterLiteral' => { 'Enabled': true },
'Style/MethodMissingSuper' => { 'Enabled': true },
'Lint/UselessComparison' => { 'Enabled': true },
'Lint/RescueWithoutErrorClass' => { 'Enabled': true },
'Lint/SpaceBeforeFirstArg' => { 'Enabled': true },
'Style/SpaceAfterControlKeyword' => { 'Enabled': true },
Expand Down Expand Up @@ -199,6 +200,8 @@
(obsolete configuration found in example/.rubocop.yml, please update it)
The `Style/MethodMissingSuper` cop has been removed since it has been superseded by `Lint/MissingSuper`. Please use `Lint/MissingSuper` instead.
(obsolete configuration found in example/.rubocop.yml, please update it)
The `Lint/UselessComparison` cop has been removed since it has been superseded by `Lint/BinaryOperatorWithIdenticalOperands`. Please use `Lint/BinaryOperatorWithIdenticalOperands` instead.
(obsolete configuration found in example/.rubocop.yml, please update it)
The `Style/MethodMissing` cop has been split into `Style/MethodMissingSuper` and `Style/MissingRespondToMissing`.
(obsolete configuration found in example/.rubocop.yml, please update it)
OUTPUT
Expand Down
@@ -0,0 +1,33 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::BinaryOperatorWithIdenticalOperands do
subject(:cop) { described_class.new }

it 'registers an offense when binary operator has identical nodes' do
offenses = inspect_source(<<~RUBY)
x == x
y = x && x
y = a.x + a.x
a.x(arg) > a.x(arg)
a.(x) > a.(x)
RUBY

expect(offenses.size).to eq(5)
end

it 'does not register an offense when using binary operator with different operands' do
expect_no_offenses(<<~RUBY)
x == y
y = x && z
y = a.x + b.x
a.x(arg) > b.x(arg)
a.(x) > b.(x)
RUBY
end

it 'does not register an offense when using arithmetic operator with numerics' do
expect_no_offenses(<<~RUBY)
x = 2 + 2
RUBY
end
end
32 changes: 0 additions & 32 deletions spec/rubocop/cop/lint/useless_comparison_spec.rb

This file was deleted.

0 comments on commit d94c4a4

Please sign in to comment.