Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
d8ed6ab
commit 243095a
Showing
5 changed files
with
239 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
* [#7849](https://github.com/rubocop/rubocop/issues/7849): Add new `Lint/AmbiguousOperatorPrecedence` cop. ([@dvandersluis][]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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
123
spec/rubocop/cop/lint/ambiguous_operator_precedence_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |