Skip to content

Commit

Permalink
[Fix rubocop#7849] Add new Lint/AmbiguousOperatorPrecedence cop.
Browse files Browse the repository at this point in the history
  • Loading branch information
dvandersluis committed Aug 26, 2021
1 parent 93e6afc commit f0e1373
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_new_lintambiguousoperatorprecedence.md
@@ -0,0 +1 @@
* [#7849](https://github.com/rubocop/rubocop/issues/7849): Add new `Lint/AmbiguousOperatorPrecedence` cop. ([@dvandersluis][])
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -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: '<<next>>'

Lint/AmbiguousRange:
Description: Checks for ranges with ambiguous boundaries.
Enabled: pending
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -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'
Expand Down
107 changes: 107 additions & 0 deletions 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
123 changes: 123 additions & 0 deletions 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

0 comments on commit f0e1373

Please sign in to comment.