Skip to content

Corrective action: Add cop to detect && and || in policy rule blocks

corrective action arising from #290710 (comment 459250761)

Document this gotcha

A condition used inside a rule block is not evaluated immediately.

So you cannot use boolean operators such as &&, and || on conditions inside rule blocks. Similarly ternary operators do not work as well. To illustrate:

condition(:batteries) { @subject.has_batteries }
condition(:conducts_electricity) { @subject.is_metal? }
# bad, `conducts_electricity` returns a Rule object, not a boolean!
rule { conducts_electricity && batteries }.enable :light_bulb

# good
rule { conducts_electricity & batteries }.enable :light_bulb

# bad, `conducts_electricity` returns a Rule object, so the ternary is always going to be true
rule { conducts_electricity ? can?(:magnetize) : batteries }.enable :motor

# good
rule { conducts_electricity & can?(:magnetize) }.enable :motor
rule { ~conducts_electricity & batteries }.enable :motor

References:

  1. #9727 (closed)
  2. #290710 (closed)

Rubocop

I have the beginnings of a cop, which identified #291007 (closed). If anyone is free, feel free to take over, as I will need to sign off for the weekend soon.

# frozen_string_literal: true

module RuboCop
  module Cop
    module Gitlab
      class PolicyRuleBoolean < RuboCop::Cop::Cop
        def_node_search :has_and_operator?, <<~PATTERN
          (and ...)
        PATTERN

        def_node_search :has_or_operator?, <<~PATTERN
          (or ...)
        PATTERN

        def_node_search :has_if?, <<~PATTERN
          (if ...)
        PATTERN

        def on_block(node)
          return unless node.method_name == :rule

          if has_and_operator?(node)
            add_offense(node, message: '&& is not allowed within a rule block. Did you mean to use `&`?')
          end

          if has_or_operator?(node)
            add_offense(node, message: '|| is not allowed within a rule block. Did you mean to use `|`?')
          end

          if has_if?(node)
            add_offense(node, message: 'if and ternary operators are not allowed within a rule block.')
          end
        end
      end
    end
  end
end
Edited by Thong Kuah