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:
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