diff --git a/CHANGELOG.md b/CHANGELOG.md index 8db44b8b79d..120deeb36b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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][]) @@ -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][]) diff --git a/config/default.yml b/config/default.yml index 7f01a2ce4f3..10d1abc5640 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 @@ -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 diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 9fa43c23c60..d35658724c0 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -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] @@ -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] diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index 2b0e0333362..48e6d40e41e 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -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 |=== @@ -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 |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index aa05076f8f3..e4b06706b70 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -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' @@ -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' diff --git a/lib/rubocop/config_obsoletion.rb b/lib/rubocop/config_obsoletion.rb index fcc7ef930fb..56c743affab 100644 --- a/lib/rubocop/config_obsoletion.rb +++ b/lib/rubocop/config_obsoletion.rb @@ -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 diff --git a/lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb b/lib/rubocop/cop/lint/binary_operator_with_identical_operands.rb new file mode 100644 index 00000000000..d28a3ff5c6a --- /dev/null +++ b/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 `%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 diff --git a/lib/rubocop/cop/lint/useless_comparison.rb b/lib/rubocop/cop/lint/useless_comparison.rb deleted file mode 100644 index 5b1ec89c4da..00000000000 --- a/lib/rubocop/cop/lint/useless_comparison.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - module Lint - # This cop checks for comparison of something with itself. - # - # @example - # - # # bad - # - # x.top >= x.top - class UselessComparison < Base - MSG = 'Comparison of something with itself detected.' - OPS = %w[== === != < > <= >= <=>].freeze - - def_node_matcher :useless_comparison?, - "(send $_match {:#{OPS.join(' :')}} $_match)" - - def on_send(node) - return unless useless_comparison?(node) - - add_offense(node.loc.selector) - end - end - end - end -end diff --git a/spec/rubocop/config_obsoletion_spec.rb b/spec/rubocop/config_obsoletion_spec.rb index 447ca104afc..b605c214e4f 100644 --- a/spec/rubocop/config_obsoletion_spec.rb +++ b/spec/rubocop/config_obsoletion_spec.rb @@ -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 }, @@ -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 diff --git a/spec/rubocop/cop/lint/binary_operator_with_identical_operands_spec.rb b/spec/rubocop/cop/lint/binary_operator_with_identical_operands_spec.rb new file mode 100644 index 00000000000..97f9f01b3af --- /dev/null +++ b/spec/rubocop/cop/lint/binary_operator_with_identical_operands_spec.rb @@ -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 diff --git a/spec/rubocop/cop/lint/useless_comparison_spec.rb b/spec/rubocop/cop/lint/useless_comparison_spec.rb deleted file mode 100644 index 9492eee6c81..00000000000 --- a/spec/rubocop/cop/lint/useless_comparison_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::Lint::UselessComparison do - subject(:cop) { described_class.new } - - described_class::OPS.each do |op| - it "registers an offense for a simple comparison with #{op}" do - expect_offense(<<~RUBY, op: op) - 5 %{op} 5 - ^{op} Comparison of something with itself detected. - a %{op} a - ^{op} Comparison of something with itself detected. - RUBY - end - - it "registers an offense for a complex comparison with #{op}" do - expect_offense(<<~RUBY, op: op) - 5 + 10 * 30 %{op} 5 + 10 * 30 - ^{op} Comparison of something with itself detected. - a.top(x) %{op} a.top(x) - ^{op} Comparison of something with itself detected. - RUBY - end - end - - it 'works with lambda.()' do - expect_offense(<<~RUBY) - a.(x) > a.(x) - ^ Comparison of something with itself detected. - RUBY - end -end