Skip to content
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 Lint/BinaryOperatorIdenticalOperands cop #8433

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.