diff --git a/changelog/new_add_new_lintambiguousoperatorprecedence.md b/changelog/new_add_new_lintambiguousoperatorprecedence.md new file mode 100644 index 00000000000..fe332d13a22 --- /dev/null +++ b/changelog/new_add_new_lintambiguousoperatorprecedence.md @@ -0,0 +1 @@ +* [#7849](https://github.com/rubocop/rubocop/issues/7849): Add new `Lint/AmbiguousOperatorPrecedence` cop. ([@dvandersluis][]) diff --git a/config/default.yml b/config/default.yml index d23e6333e1a..d16c8faebeb 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1448,6 +1448,13 @@ Lint/AmbiguousOperator: VersionAdded: '0.17' VersionChanged: '0.83' +Lint/AmbiguousOperatorPrecedence: + Description: >- + Checks for expressions containing multiple binary operations with + ambiguous precedence. + Enabled: pending + VersionAdded: '<>' + Lint/AmbiguousRange: Description: Checks for ranges with ambiguous boundaries. Enabled: pending diff --git a/lib/rubocop.rb b/lib/rubocop.rb index ab6171ba700..1a19102b70b 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -263,6 +263,7 @@ require_relative 'rubocop/cop/lint/ambiguous_assignment' require_relative 'rubocop/cop/lint/ambiguous_block_association' require_relative 'rubocop/cop/lint/ambiguous_operator' +require_relative 'rubocop/cop/lint/ambiguous_operator_precedence' require_relative 'rubocop/cop/lint/ambiguous_range' require_relative 'rubocop/cop/lint/ambiguous_regexp_literal' require_relative 'rubocop/cop/lint/assignment_in_condition' diff --git a/lib/rubocop/cop/lint/ambiguous_operator_precedence.rb b/lib/rubocop/cop/lint/ambiguous_operator_precedence.rb new file mode 100644 index 00000000000..10969a5a346 --- /dev/null +++ b/lib/rubocop/cop/lint/ambiguous_operator_precedence.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cop looks for expressions containing multiple binary operators + # where precedence is ambiguous due to lack of parentheses. For example, + # in `1 + 2 * 3`, the multiplication will happen before the addition, but + # lexically it appears that the addition will happen first. + # + # The cop does not consider unary operators (ie. `!a` or `-b`) or comparison + # operators (ie. `a =~ b`) because those are not ambiguous. + # + # NOTE: Ranges are handled by `Lint/AmbiguousRange`. + # + # @example + # # bad + # a + b * c + # a || b && c + # a ** b + c + # + # # good (different precedence) + # a + (b * c) + # a || (b && c) + # (a ** b) + c + # + # # good (same precedence) + # a + b + c + # a * b / c % d + class AmbiguousOperatorPrecedence < Base + extend AutoCorrector + + # See https://ruby-doc.org/core-3.0.2/doc/syntax/precedence_rdoc.html + PRECEDENCE = [ + %i[**], + %i[* / %], + %i[+ -], + %i[<< >>], + %i[&], + %i[| ^], + %i[&&], + %i[||] + ].freeze + RESTRICT_ON_SEND = PRECEDENCE.flatten.freeze + MSG = 'Wrap expressions with varying precedence with parentheses to avoid ambiguity.' + + def on_new_investigation + # Cache the precedence of each node being investigated + # so that we only need to calculate it once + @node_precedences = {} + super + end + + def on_and(node) + return unless (parent = node.parent) + + return if parent.begin_type? # if the `and` is in a `begin`, it's parenthesized already + return unless parent.or_type? + + add_offense(node) do |corrector| + autocorrect(corrector, node) + end + end + + def on_send(node) + return if node.parenthesized? + + return unless (parent = node.parent) + return unless operator?(parent) + return unless greater_precedence?(node, parent) + + add_offense(node) do |corrector| + autocorrect(corrector, node) + end + end + + private + + def precedence(node) + @node_precedences.fetch(node) do + PRECEDENCE.index { |operators| operators.include?(operator_name(node)) } + end + end + + def operator?(node) + (node.send_type? && RESTRICT_ON_SEND.include?(node.method_name)) || node.operator_keyword? + end + + def greater_precedence?(node1, node2) + precedence(node2) > precedence(node1) + end + + def operator_name(node) + if node.send_type? + node.method_name + else + node.operator.to_sym + end + end + + def autocorrect(corrector, node) + corrector.wrap(node, '(', ')') + end + end + end + end +end diff --git a/spec/rubocop/cop/lint/ambiguous_operator_precedence_spec.rb b/spec/rubocop/cop/lint/ambiguous_operator_precedence_spec.rb new file mode 100644 index 00000000000..1d653212e7f --- /dev/null +++ b/spec/rubocop/cop/lint/ambiguous_operator_precedence_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::AmbiguousOperatorPrecedence, :config do + it 'does not register an offense when there is only one operator in the expression' do + expect_no_offenses(<<~RUBY) + a + b + RUBY + end + + it 'does not register an offense when all operators in the expression have the same precedence' do + expect_no_offenses(<<~RUBY) + a + b + c + a * b / c % d + a && b && c + RUBY + end + + it 'does not register an offense when expressions are wrapped in parentheses by precedence' do + expect_no_offenses(<<~RUBY) + a + (b * c) + (a ** b) + c + RUBY + end + + it 'does not register an offense when expressions are wrapped in parentheses by reverse precedence' do + expect_no_offenses(<<~RUBY) + (a + b) * c + a ** (b + c) + RUBY + end + + it 'does not register an offense when boolean expressions are wrapped in parens' do + expect_no_offenses(<<~RUBY) + (a && b) || c + a || (b & c) + RUBY + end + + it 'registers an offense when an expression with mixed precedence has no parens' do + expect_offense(<<~RUBY) + a + b * c + ^^^^^ Wrap expressions with varying precedence with parentheses to avoid ambiguity. + RUBY + + expect_correction(<<~RUBY) + a + (b * c) + RUBY + end + + it 'registers an offense when an expression with mixed boolean operators has no parens' do + expect_offense(<<~RUBY) + a && b || c + ^^^^^^ Wrap expressions with varying precedence with parentheses to avoid ambiguity. + a || b && c + ^^^^^^ Wrap expressions with varying precedence with parentheses to avoid ambiguity. + RUBY + + expect_correction(<<~RUBY) + (a && b) || c + a || (b && c) + RUBY + end + + it 'registers an offense for expressions containing booleans and operators' do + expect_offense(<<~RUBY) + a && b * c + ^^^^^ Wrap expressions with varying precedence with parentheses to avoid ambiguity. + a * b && c + ^^^^^ Wrap expressions with varying precedence with parentheses to avoid ambiguity. + RUBY + + expect_correction(<<~RUBY) + a && (b * c) + (a * b) && c + RUBY + end + + it 'registers an offense when the entire expression is wrapped in parentheses' do + expect_offense(<<~RUBY) + (a + b * c ** d) + ^^^^^^ Wrap expressions with varying precedence with parentheses to avoid ambiguity. + ^^^^^^^^^^ Wrap expressions with varying precedence with parentheses to avoid ambiguity. + RUBY + + expect_correction(<<~RUBY) + (a + (b * (c ** d))) + RUBY + end + + it 'corrects a super long expression in precedence order' do + expect_offense(<<~RUBY) + a ** b * c / d % e + f - g << h >> i & j | k ^ l && m || n + ^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + RUBY + + expect_correction(<<~RUBY) + (((((((a ** b) * c / d % e) + f - g) << h >> i) & j) | k ^ l) && m) || n + RUBY + end + + it 'corrects a super long expression in reverse precedence order' do + expect_offense(<<~RUBY) + a || b && c | d ^ e & f << g >> h + i - j * k / l % n ** m + ^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrap expressions [...] + RUBY + + expect_correction(<<~RUBY) + a || (b && (c | d ^ (e & (f << g >> (h + i - (j * k / l % (n ** m))))))) + RUBY + end +end