Skip to content

Commit

Permalink
[Fix rubocop#7849] Add ExplicitOperatorPrecedence cop
Browse files Browse the repository at this point in the history
Fixes rubocop#7849

This cop checks if binary operators of different precedents are
used without explicit use of parenthesis.

```ruby
a && b || c
a * b + c
a ** b * c / d % e + f - g << h >> i & j | k ^ l

(a && b) || c
(a * b) + c
(((((a**b) * c / d % e) + f - g) << h >> i) & j) | k ^ l
```
  • Loading branch information
Rahul Ahuja committed Apr 26, 2020
1 parent db23f5e commit ee2f8dc
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -70,6 +70,7 @@
* [#7823](https://github.com/rubocop-hq/rubocop/pull/7823): Add `IgnoredMethods` configuration in `Metrics/AbcSize`, `Metrics/CyclomaticComplexity`, and `Metrics/PerceivedComplexity` cops. ([@drenmi][])
* [#7816](https://github.com/rubocop-hq/rubocop/pull/7816): Support Ruby 2.7's numbered parameter for `Style/Lambda`. ([@koic][])
* [#7829](https://github.com/rubocop-hq/rubocop/issues/7829): Fix an error for `Style/OneLineConditional` when one of the branches contains `next` keyword. ([@koic][])
* [#7849](https://github.com/rubocop-hq/rubocop/issues/7849): Add new `Lint/ExplicitOperatorPrecedence` cop. ([@rahul404][])

### Bug fixes

Expand Down Expand Up @@ -4479,3 +4480,4 @@
[@saurabhmaurya15]: https://github.com/saurabhmaurya15
[@DracoAter]: https://github.com/DracoAter
[@diogoosorio]: https://github.com/diogoosorio
[@rahul404]: https://github.com/rahul404
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -1425,6 +1425,13 @@ Lint/ErbNewArguments:
Enabled: true
VersionAdded: '0.56'

Lint/ExplicitOperatorPrecedence:
Description: >-
Catches if multiple binary operators of different precedents
are used without parenthesis.
Enabled: pending
VersionAdded: '0.83'

Lint/FlipFlop:
Description: 'Checks for flip-flops.'
StyleGuide: '#no-flip-flops'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -303,6 +303,7 @@
require_relative 'rubocop/cop/lint/empty_when'
require_relative 'rubocop/cop/lint/ensure_return'
require_relative 'rubocop/cop/lint/erb_new_arguments'
require_relative 'rubocop/cop/lint/explicit_operator_precedence'
require_relative 'rubocop/cop/lint/flip_flop'
require_relative 'rubocop/cop/lint/float_out_of_range'
require_relative 'rubocop/cop/lint/format_parameter_mismatch'
Expand Down
68 changes: 68 additions & 0 deletions lib/rubocop/cop/lint/explicit_operator_precedence.rb
@@ -0,0 +1,68 @@
# frozen_string_literal: true
# TODO: when finished, run `rake generate_cops_documentation` to update the docs
module RuboCop
module Cop
module Lint
# This cop checks if binary operators of different precedents are used without explicit use of parenthesis.
# when operators are used without parenthesis
#
# @example
#
# # bad
# a && b || c
# a * b + c
# a ** b * c / d % e + f - g << h >> i & j | k ^ l
#
# @example
#
# # good
# # With parenthesis, there is no ambiguity.
# (a && b) || c
# (a * b) + c
# (((((a**b) * c / d % e) + f - g) << h >> i) & j) | k ^ l
class ExplicitOperatorPrecedence < Cop
MSG = 'Operators with varied precedents used in a single statement.'

PRECEDENCE_PRIORITY = [[:**], [:*, :/, :%], [:+, :-], [:<<, :>>], [:&], [:|, :^]]

def on_and(node)
add_offense(node) if node.parent.or_type?
end

def on_send(node)
return unless cop_required?(node)
add_offense(node) if multiple_precedences_used?(node)
end

def autocorrect(node)
->(corrector) { corrector.replace(replacement_range(node), correction(node)) }
end

private

def cop_required?(node)
node.parent && node.parent.respond_to?(:method_name) &&
PRECEDENCE_PRIORITY.flatten.include?(node.method_name) && PRECEDENCE_PRIORITY.flatten.include?(node.method_name)
end

def multiple_precedences_used?(node)
operator_priority(node.method_name) < operator_priority(node.parent.method_name)
end

def operator_priority(operator)
PRECEDENCE_PRIORITY.find_index { |operators| operators.include?(operator) }
end

def correction(node)
"(#{node.source})"
end

def replacement_range(node)
Parser::Source::Range.new(node.loc.expression.source_buffer,
node.loc.expression.begin_pos,
node.loc.expression.end_pos)
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Expand Up @@ -198,6 +198,7 @@ In the following section you find all available cops:
* [Lint/EmptyWhen](cops_lint.md#lintemptywhen)
* [Lint/EnsureReturn](cops_lint.md#lintensurereturn)
* [Lint/ErbNewArguments](cops_lint.md#linterbnewarguments)
* [Lint/ExplicitOperatorPrecedence](cops_lint.md#lintexplicitoperatorprecedence)
* [Lint/FlipFlop](cops_lint.md#lintflipflop)
* [Lint/FloatOutOfRange](cops_lint.md#lintfloatoutofrange)
* [Lint/FormatParameterMismatch](cops_lint.md#lintformatparametermismatch)
Expand Down
25 changes: 25 additions & 0 deletions manual/cops_lint.md
Expand Up @@ -719,6 +719,31 @@ else
end
```

## Lint/ExplicitOperatorPrecedence

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Pending | Yes | Yes | 0.83 | -

This cop checks if binary operators of different precedents are used without explicit use of parenthesis.
when operators are used without parenthesis

### Examples

```ruby
# bad
a && b || c
a * b + c
a ** b * c / d % e + f - g << h >> i & j | k ^ l
```
```ruby
# good
# With parenthesis, there is no ambiguity.
(a && b) || c
(a * b) + c
(((((a**b) * c / d % e) + f - g) << h >> i) & j) | k ^ l
```

## Lint/FlipFlop

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
58 changes: 58 additions & 0 deletions spec/rubocop/cop/lint/explicit_operator_precedence_spec.rb
@@ -0,0 +1,58 @@
# frozen_string_literal: true
RSpec.describe RuboCop::Cop::Lint::ExplicitOperatorPrecedence, :config do
subject(:cop) { described_class.new(config) }

before do
inspect_source(source)
end

shared_examples 'code with offense' do |code, expected = nil, offense_count = 1|
context "when checking #{code}" do
let(:source) { code }

let(:offense_count) { offense_count }

it 'registers an offense' do
expect(cop.offenses.size).to eq(offense_count)
expect(cop.messages).to eq(offense_count.times.map { |_| message })
end

if expected
it 'auto-corrects' do
expect(autocorrect_source(code)).to eq(expected)
end
else
it 'does not auto-correct' do
expect(autocorrect_source(code)).to eq(code)
end
end
end
end

shared_examples 'code without offense' do |code|
let(:source) { code }

it 'does not register an offense' do
expect(cop.offenses.empty?).to be(true)
end
end

let(:message) { 'Operators with varied precedents used in a single statement.' }

context 'when `&&` is used with `||` without parenthesis' do
it_behaves_like 'code with offense', 'foo && baz || bar', '(foo && baz) || bar'
end

context 'when `&&` is used with `||` with parenthesis' do
it_behaves_like 'code without offense', '(foo && baz) || bar'
end

context "when operators with different precedents are used without parenthesis" do
it_behaves_like 'code with offense', 'a ** b * c / d % e + f - g << h >> i & j | k ^ l',
'(((((a**b) * c / d % e) + f - g) << h >> i) & j) | k ^ l', 5
end

context "when operators with different precedents are used with parenthesis" do
it_behaves_like 'code without offense', '(((((a**b) * c / d % e) + f - g) << h >> i) & j) | k ^ l'
end
end

0 comments on commit ee2f8dc

Please sign in to comment.