[Fix rubocop#5388] Add new Style/UnlessLogicalOperators cop (ruboco…

…p#9386)

changelog/new_add_new_styleunlesslogicaloperators_cop.md

@@ -0,0 +1 @@
+* [#5388](https://github.com/rubocop-hq/rubocop/issues/5388): Add new `Style/UnlessLogicalOperators` cop. ([@caalberts][])

config/default.yml

@@ -4740,6 +4740,16 @@ Style/UnlessElse:
   Enabled: true
   VersionAdded: '0.9'
 
+Style/UnlessLogicalOperators:
+  Description: >-
+                 Checks for use of logical operators in an unless condition.
+  Enabled: false
+  VersionAdded: '<<next>>'
+  EnforcedStyle: forbid_mixed_logical_operators
+  SupportedStyles:
+    - forbid_mixed_logical_operators
+    - forbid_logical_operators
+
 Style/UnpackFirst:
   Description: >-
                  Checks for accessing the first element of `String#unpack`

lib/rubocop.rb

@@ -605,6 +605,7 @@
 require_relative 'rubocop/cop/style/trailing_underscore_variable'
 require_relative 'rubocop/cop/style/trivial_accessors'
 require_relative 'rubocop/cop/style/unless_else'
+require_relative 'rubocop/cop/style/unless_logical_operators'
 require_relative 'rubocop/cop/style/unpack_first'
 require_relative 'rubocop/cop/style/variable_interpolation'
 require_relative 'rubocop/cop/style/when_then'

lib/rubocop/cop/style/unless_logical_operators.rb

@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Style
+      # This cop checks for the use of logical operators in an `unless` condition.
+      # It discourages such code, as the condition becomes more difficult
+      # to read and understand.
+      #
+      # This cop supports two styles:
+      # - `forbid_mixed_logical_operators` (default)
+      # - `forbid_logical_operators`
+      #
+      # `forbid_mixed_logical_operators` style forbids the use of more than one type
+      # of logical operators. This makes the `unless` condition easier to read
+      # because either all conditions need to be met or any condition need to be met
+      # in order for the expression to be truthy or falsey.
+      #
+      # `forbid_logical_operators` style forbids any use of logical operator.
+      # This makes it even more easy to read the `unless` condition as
+      # there is only one condition in the expression.
+      #
+      # @example EnforcedStyle: forbid_mixed_logical_operators (default)
+      #   # bad
+      #   return unless a || b && c
+      #   return unless a && b || c
+      #   return unless a && b and c
+      #   return unless a || b or c
+      #   return unless a && b or c
+      #   return unless a || b and c
+      #
+      #   # good
+      #   return unless a && b && c
+      #   return unless a || b || c
+      #   return unless a and b and c
+      #   return unless a or b or c
+      #   return unless a?
+      #
+      # @example EnforcedStyle: forbid_logical_operators
+      #   # bad
+      #   return unless a || b
+      #   return unless a && b
+      #   return unless a or b
+      #   return unless a and b
+      #
+      #   # good
+      #   return unless a
+      #   return unless a?
+      class UnlessLogicalOperators < Base
+        include ConfigurableEnforcedStyle
+
+        FORBID_MIXED_LOGICAL_OPERATORS = 'Do not use mixed logical operators in an `unless`.'
+        FORBID_LOGICAL_OPERATORS = 'Do not use any logical operator in an `unless`.'
+
+        def_node_matcher :or_with_and?, <<~PATTERN
+          (if (or <`and ...> ) ...)
+        PATTERN
+        def_node_matcher :and_with_or?, <<~PATTERN
+          (if (and <`or ...> ) ...)
+        PATTERN
+        def_node_matcher :logical_operator?, <<~PATTERN
+          (if ({and or} ... ) ...)
+        PATTERN
+
+        def on_if(node)
+          return unless node.unless?
+
+          if style == :forbid_mixed_logical_operators && mixed_logical_operator?(node)
+            add_offense(node, message: FORBID_MIXED_LOGICAL_OPERATORS)
+          elsif style == :forbid_logical_operators && logical_operator?(node)
+            add_offense(node, message: FORBID_LOGICAL_OPERATORS)
+          end
+        end
+
+        private
+
+        def mixed_logical_operator?(node)
+          or_with_and?(node) ||
+            and_with_or?(node) ||
+            mixed_precedence_and?(node) ||
+            mixed_precedence_or?(node)
+        end
+
+        def mixed_precedence_and?(node)
+          node.source.include?('&&') && node.source.include?('and')
+        end
+
+        def mixed_precedence_or?(node)
+          node.source.include?('||') && node.source.include?('or')
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/style/unless_logical_operators_spec.rb

@@ -0,0 +1,181 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Style::UnlessLogicalOperators, :config do
+  context 'EnforcedStyle is `forbid_mixed_logical_operators`' do
+    let(:cop_config) do
+      { 'EnforcedStyle' => 'forbid_mixed_logical_operators' }
+    end
+
+    it 'registers an offense when using `&&` and `||`' do
+      expect_offense(<<~RUBY)
+        return unless a && b || c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+
+      expect_offense(<<~RUBY)
+        return unless a || b && c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using `&&` and `and`' do
+      expect_offense(<<~RUBY)
+        return unless a && b and c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+
+      expect_offense(<<~RUBY)
+        return unless a and b && c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using `&&` and `or`' do
+      expect_offense(<<~RUBY)
+        return unless a && b or c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+
+      expect_offense(<<~RUBY)
+        return unless a or b && c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using `||` and `or`' do
+      expect_offense(<<~RUBY)
+        return unless a || b or c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+
+      expect_offense(<<~RUBY)
+        return unless a or b || c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using `||` and `and`' do
+      expect_offense(<<~RUBY)
+        return unless a || b and c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+
+      expect_offense(<<~RUBY)
+        return unless a and b || c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using parentheses' do
+      expect_offense(<<~RUBY)
+        return unless a || (b && c) || d
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use mixed logical operators in an `unless`.
+      RUBY
+    end
+
+    it 'does not register an offense when using only `&&`s' do
+      expect_no_offenses(<<~RUBY)
+        return unless a && b && c
+      RUBY
+    end
+
+    it 'does not register an offense when using only `||`s' do
+      expect_no_offenses(<<~RUBY)
+        return unless a || b || c
+      RUBY
+    end
+
+    it 'does not register an offense when using only `and`s' do
+      expect_no_offenses(<<~RUBY)
+        return unless a and b and c
+      RUBY
+    end
+
+    it 'does not register an offense when using only `or`s' do
+      expect_no_offenses(<<~RUBY)
+        return unless a or b or c
+      RUBY
+    end
+
+    it 'does not register an offense when using if' do
+      expect_no_offenses(<<~RUBY)
+        return if a || b && c || d
+      RUBY
+    end
+
+    it 'does not register an offense when not used in unless' do
+      expect_no_offenses(<<~RUBY)
+        def condition?
+          a or b && c || d
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when not using logical operator' do
+      expect_no_offenses(<<~RUBY)
+        return unless a?
+      RUBY
+    end
+  end
+
+  context 'EnforcedStyle is `forbid_logical_operators`' do
+    let(:cop_config) do
+      { 'EnforcedStyle' => 'forbid_logical_operators' }
+    end
+
+    it 'registers an offense when using only `&&`' do
+      expect_offense(<<~RUBY)
+        return unless a && b
+        ^^^^^^^^^^^^^^^^^^^^ Do not use any logical operator in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using only `||`' do
+      expect_offense(<<~RUBY)
+        return unless a || b
+        ^^^^^^^^^^^^^^^^^^^^ Do not use any logical operator in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using only `and`' do
+      expect_offense(<<~RUBY)
+        return unless a and b
+        ^^^^^^^^^^^^^^^^^^^^^ Do not use any logical operator in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using only `or`' do
+      expect_offense(<<~RUBY)
+        return unless a or b
+        ^^^^^^^^^^^^^^^^^^^^ Do not use any logical operator in an `unless`.
+      RUBY
+    end
+
+    it 'registers an offense when using `&&` followed by ||' do
+      expect_offense(<<~RUBY)
+        return unless a && b || c
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use any logical operator in an `unless`.
+      RUBY
+    end
+
+    it 'does not register an offense when using if' do
+      expect_no_offenses(<<~RUBY)
+        return if a || b
+      RUBY
+    end
+
+    it 'does not register an offense when not used in unless' do
+      expect_no_offenses(<<~RUBY)
+        def condition?
+          a || b
+        end
+      RUBY
+    end
+
+    it 'does not register an offense when not using logical operator' do
+      expect_no_offenses(<<~RUBY)
+        return unless a?
+      RUBY
+    end
+  end
+end