Commit 7f33b584 authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Fabio Pitino

Add parenthesis support for if: conditions

This extends RPN to support `()` in expressions
parent 156970ca
---
title: 'Add parenthesis support for if: conditions'
merge_request: 37574
author:
type: added
......@@ -742,6 +742,40 @@ Precedence of operators follows the
[Ruby 2.5 standard](https://ruby-doc.org/core-2.5.0/doc/syntax/precedence_rdoc.html),
so `&&` is evaluated before `||`.
#### Parentheses
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/230938) in GitLab 13.3
It is possible to use parentheses to group conditions. Parentheses have the highest
precedence of all operators. Expressions enclosed in parentheses are evaluated first,
and the result is used for the rest of the expression.
Many nested parentheses can be used to create complex conditions, and the inner-most
expressions in parentheses are evaluated first. For an expression to be valid an equal
number of `(` and `)` need to be used.
Examples:
- `($VARIABLE1 =~ /^content.*/ || $VARIABLE2) && ($VARIABLE3 =~ /thing$/ || $VARIABLE4)`
- `($VARIABLE1 =~ /^content.*/ || $VARIABLE2 =~ /thing$/) && $VARIABLE3`
- `$CI_COMMIT_BRANCH == "my-branch" || (($VARIABLE1 == "thing" || $VARIABLE2 == "thing") && $VARIABLE3)`
The feature is currently deployed behind a feature flag that is **disabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
can opt to enable it for your instance.
To enable it:
```ruby
Feature.enable(:ci_if_parenthesis_enabled)
```
To disable it:
```ruby
Feature.disable(:ci_if_parenthesis_enabled)
```
### Storing regular expressions in variables
It is possible to store a regular expression in a variable, to be used for pattern matching:
......
......@@ -70,6 +70,10 @@ module Gitlab
::Feature.enabled?(:ci_bulk_insert_on_create, project, default_enabled: true)
end
def self.ci_if_parenthesis_enabled?
::Feature.enabled?(:ci_if_parenthesis_enabled)
end
def self.allow_to_create_merge_request_pipelines_in_target_project?(target_project)
::Feature.enabled?(:ci_allow_to_create_merge_request_pipelines_in_target_project, target_project, default_enabled: true)
end
......
......@@ -5,7 +5,7 @@ module Gitlab
module Pipeline
module Expression
module Lexeme
class And < Lexeme::Operator
class And < Lexeme::LogicalOperator
PATTERN = /&&/.freeze
def evaluate(variables = {})
......
......@@ -10,6 +10,10 @@ module Gitlab
raise NotImplementedError
end
def name
self.class.name.demodulize.underscore
end
def self.build(token)
raise NotImplementedError
end
......@@ -23,6 +27,10 @@ module Gitlab
def self.pattern
self::PATTERN
end
def self.consume?(lexeme)
lexeme && precedence >= lexeme.precedence
end
end
end
end
......
......@@ -5,7 +5,7 @@ module Gitlab
module Pipeline
module Expression
module Lexeme
class Equals < Lexeme::Operator
class Equals < Lexeme::LogicalOperator
PATTERN = /==/.freeze
def evaluate(variables = {})
......
# frozen_string_literal: true
module Gitlab
module Ci
module Pipeline
module Expression
module Lexeme
class LogicalOperator < Lexeme::Operator
# This operator class is design to handle single operators that take two
# arguments. Expression::Parser was originally designed to read infix operators,
# and so the two operands are called "left" and "right" here. If we wish to
# implement an Operator that takes a greater or lesser number of arguments, a
# structural change or additional Operator superclass will likely be needed.
def initialize(left, right)
raise OperatorError, 'Invalid left operand' unless left.respond_to? :evaluate
raise OperatorError, 'Invalid right operand' unless right.respond_to? :evaluate
@left = left
@right = right
end
def inspect
"#{name}(#{@left.inspect}, #{@right.inspect})"
end
def self.type
:logical_operator
end
end
end
end
end
end
end
......@@ -5,7 +5,7 @@ module Gitlab
module Pipeline
module Expression
module Lexeme
class Matches < Lexeme::Operator
class Matches < Lexeme::LogicalOperator
PATTERN = /=~/.freeze
def evaluate(variables = {})
......
......@@ -5,7 +5,7 @@ module Gitlab
module Pipeline
module Expression
module Lexeme
class NotEquals < Lexeme::Operator
class NotEquals < Lexeme::LogicalOperator
PATTERN = /!=/.freeze
def evaluate(variables = {})
......
......@@ -5,7 +5,7 @@ module Gitlab
module Pipeline
module Expression
module Lexeme
class NotMatches < Lexeme::Operator
class NotMatches < Lexeme::LogicalOperator
PATTERN = /\!~/.freeze
def evaluate(variables = {})
......
......@@ -9,13 +9,17 @@ module Gitlab
PATTERN = /null/.freeze
def initialize(value = nil)
@value = nil
super
end
def evaluate(variables = {})
nil
end
def inspect
'null'
end
def self.build(_value)
self.new
end
......
......@@ -6,24 +6,10 @@ module Gitlab
module Expression
module Lexeme
class Operator < Lexeme::Base
# This operator class is design to handle single operators that take two
# arguments. Expression::Parser was originally designed to read infix operators,
# and so the two operands are called "left" and "right" here. If we wish to
# implement an Operator that takes a greater or lesser number of arguments, a
# structural change or additional Operator superclass will likely be needed.
OperatorError = Class.new(Expression::ExpressionError)
def initialize(left, right)
raise OperatorError, 'Invalid left operand' unless left.respond_to? :evaluate
raise OperatorError, 'Invalid right operand' unless right.respond_to? :evaluate
@left = left
@right = right
end
def self.type
:operator
raise NotImplementedError
end
def self.precedence
......
......@@ -5,7 +5,7 @@ module Gitlab
module Pipeline
module Expression
module Lexeme
class Or < Lexeme::Operator
class Or < Lexeme::LogicalOperator
PATTERN = /\|\|/.freeze
def evaluate(variables = {})
......
# frozen_string_literal: true
module Gitlab
module Ci
module Pipeline
module Expression
module Lexeme
class ParenthesisClose < Lexeme::Operator
PATTERN = /\)/.freeze
def self.type
:parenthesis_close
end
def self.precedence
900
end
end
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module Ci
module Pipeline
module Expression
module Lexeme
class ParenthesisOpen < Lexeme::Operator
PATTERN = /\(/.freeze
def self.type
:parenthesis_open
end
def self.precedence
# Needs to be higher than `ParenthesisClose` and all other Lexemes
901
end
end
end
end
end
end
end
......@@ -11,7 +11,7 @@ module Gitlab
PATTERN = %r{^\/([^\/]|\\/)+[^\\]\/[ismU]*}.freeze
def initialize(regexp)
@value = regexp.gsub(/\\\//, '/')
super(regexp.gsub(/\\\//, '/'))
unless Gitlab::UntrustedRegexp::RubySyntax.valid?(@value)
raise Lexer::SyntaxError, 'Invalid regular expression!'
......@@ -24,6 +24,10 @@ module Gitlab
raise Expression::RuntimeError, 'Invalid regular expression!'
end
def inspect
"/#{value}/"
end
def self.pattern
PATTERN
end
......
......@@ -9,13 +9,17 @@ module Gitlab
PATTERN = /("(?<string>.*?)")|('(?<string>.*?)')/.freeze
def initialize(value)
@value = value
super(value)
end
def evaluate(variables = {})
@value.to_s
end
def inspect
@value.inspect
end
def self.build(string)
new(string.match(PATTERN)[:string])
end
......
......@@ -9,6 +9,10 @@ module Gitlab
def self.type
:value
end
def initialize(value)
@value = value
end
end
end
end
......
......@@ -8,12 +8,12 @@ module Gitlab
class Variable < Lexeme::Value
PATTERN = /\$(?<name>\w+)/.freeze
def initialize(name)
@name = name
def evaluate(variables = {})
variables.with_indifferent_access.fetch(@value, nil)
end
def evaluate(variables = {})
variables.with_indifferent_access.fetch(@name, nil)
def inspect
"$#{@value}"
end
def self.build(string)
......
......@@ -10,6 +10,8 @@ module Gitlab
SyntaxError = Class.new(Expression::ExpressionError)
LEXEMES = [
Expression::Lexeme::ParenthesisOpen,
Expression::Lexeme::ParenthesisClose,
Expression::Lexeme::Variable,
Expression::Lexeme::String,
Expression::Lexeme::Pattern,
......@@ -22,6 +24,28 @@ module Gitlab
Expression::Lexeme::Or
].freeze
# To be removed with `ci_if_parenthesis_enabled`
LEGACY_LEXEMES = [
Expression::Lexeme::Variable,
Expression::Lexeme::String,
Expression::Lexeme::Pattern,
Expression::Lexeme::Null,
Expression::Lexeme::Equals,
Expression::Lexeme::Matches,
Expression::Lexeme::NotEquals,
Expression::Lexeme::NotMatches,
Expression::Lexeme::And,
Expression::Lexeme::Or
].freeze
def self.lexemes
if ::Gitlab::Ci::Features.ci_if_parenthesis_enabled?
LEXEMES
else
LEGACY_LEXEMES
end
end
MAX_TOKENS = 100
def initialize(statement, max_tokens: MAX_TOKENS)
......@@ -47,7 +71,7 @@ module Gitlab
return tokens if @scanner.eos?
lexeme = LEXEMES.find do |type|
lexeme = self.class.lexemes.find do |type|
type.scan(@scanner).tap do |token|
tokens.push(token) if token.present?
end
......
......@@ -15,11 +15,18 @@ module Gitlab
def tree
results = []
tokens_rpn.each do |token|
tokens =
if ::Gitlab::Ci::Features.ci_if_parenthesis_enabled?
tokens_rpn
else
legacy_tokens_rpn
end
tokens.each do |token|
case token.type
when :value
results.push(token.build)
when :operator
when :logical_operator
right_operand = results.pop
left_operand = results.pop
......@@ -27,7 +34,7 @@ module Gitlab
results.push(res)
end
else
raise ParseError, 'Unprocessable token found in parse tree'
raise ParseError, "Unprocessable token found in parse tree: #{token.type}"
end
end
......@@ -45,6 +52,7 @@ module Gitlab
# Parse the expression into Reverse Polish Notation
# (See: Shunting-yard algorithm)
# Taken from: https://en.wikipedia.org/wiki/Shunting-yard_algorithm#The_algorithm_in_detail
def tokens_rpn
output = []
operators = []
......@@ -53,7 +61,34 @@ module Gitlab
case token.type
when :value
output.push(token)
when :operator
when :logical_operator
output.push(operators.pop) while token.lexeme.consume?(operators.last&.lexeme)
operators.push(token)
when :parenthesis_open
operators.push(token)
when :parenthesis_close
output.push(operators.pop) while token.lexeme.consume?(operators.last&.lexeme)
raise ParseError, 'Unmatched parenthesis' unless operators.last
operators.pop if operators.last.lexeme.type == :parenthesis_open
end
end
output.concat(operators.reverse)
end
# To be removed with `ci_if_parenthesis_enabled`
def legacy_tokens_rpn
output = []
operators = []
@tokens.each do |token|
case token.type
when :value
output.push(token)
when :logical_operator
if operators.any? && token.lexeme.precedence >= operators.last.lexeme.precedence
output.push(operators.pop)
end
......
......@@ -24,7 +24,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::And do
describe '.type' do
it 'is an operator' do
expect(described_class.type).to eq :operator
expect(described_class.type).to eq :logical_operator
end
end
......
......@@ -27,7 +27,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::Equals do
describe '.type' do
it 'is an operator' do
expect(described_class.type).to eq :operator
expect(described_class.type).to eq :logical_operator
end
end
......
......@@ -28,7 +28,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do
describe '.type' do
it 'is an operator' do
expect(described_class.type).to eq :operator
expect(described_class.type).to eq :logical_operator
end
end
......
......@@ -27,7 +27,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::NotEquals do
describe '.type' do
it 'is an operator' do
expect(described_class.type).to eq :operator
expect(described_class.type).to eq :logical_operator
end
end
......
......@@ -28,7 +28,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::NotMatches do
describe '.type' do
it 'is an operator' do
expect(described_class.type).to eq :operator
expect(described_class.type).to eq :logical_operator
end
end
......
......@@ -24,7 +24,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexeme::Or do
describe '.type' do
it 'is an operator' do
expect(described_class.type).to eq :operator
expect(described_class.type).to eq :logical_operator
end
end
......
......@@ -81,6 +81,35 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexer do
with_them do
it { is_expected.to eq(tokens) }
end
context 'with parentheses are used' do
where(:expression, :tokens) do
'($PRESENT_VARIABLE =~ /my var/) && $EMPTY_VARIABLE =~ /nope/' | ['(', '$PRESENT_VARIABLE', '=~', '/my var/', ')', '&&', '$EMPTY_VARIABLE', '=~', '/nope/']
'$PRESENT_VARIABLE =~ /my var/ || ($EMPTY_VARIABLE =~ /nope/)' | ['$PRESENT_VARIABLE', '=~', '/my var/', '||', '(', '$EMPTY_VARIABLE', '=~', '/nope/', ')']
'($PRESENT_VARIABLE && (null || $EMPTY_VARIABLE == ""))' | ['(', '$PRESENT_VARIABLE', '&&', '(', 'null', '||', '$EMPTY_VARIABLE', '==', '""', ')', ')']
end
with_them do
context 'when ci_if_parenthesis_enabled is enabled' do
before do
stub_feature_flags(ci_if_parenthesis_enabled: true)
end
it { is_expected.to eq(tokens) }
end
context 'when ci_if_parenthesis_enabled is disabled' do
before do
stub_feature_flags(ci_if_parenthesis_enabled: false)
end
it do
expect { subject }
.to raise_error described_class::SyntaxError
end
end
end
end
end
end
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'spec_helper'
RSpec.describe Gitlab::Ci::Pipeline::Expression::Parser do
before do
stub_feature_flags(ci_if_parenthesis_enabled: true)
end
describe '#tree' do
context 'when using two operators' do
it 'returns a reverse descent parse tree' do
expect(described_class.seed('$VAR1 == "123"').tree)
.to be_a Gitlab::Ci::Pipeline::Expression::Lexeme::Equals
context 'validates simple operators' do
using RSpec::Parameterized::TableSyntax
where(:expression, :result_tree) do
'$VAR1 == "123"' | 'equals($VAR1, "123")'
'$VAR1 == "123" == $VAR2' | 'equals(equals($VAR1, "123"), $VAR2)'
'$VAR' | '$VAR'
'"some value"' | '"some value"'
'null' | 'null'
'$VAR1 || $VAR2 && $VAR3' | 'or($VAR1, and($VAR2, $VAR3))'
'$VAR1 && $VAR2 || $VAR3' | 'or(and($VAR1, $VAR2), $VAR3)'
'$VAR1 && $VAR2 || $VAR3 && $VAR4' | 'or(and($VAR1, $VAR2), and($VAR3, $VAR4))'
'$VAR1 && ($VAR2 || $VAR3) && $VAR4' | 'and(and($VAR1, or($VAR2, $VAR3)), $VAR4)'
end
with_them do
it { expect(described_class.seed(expression).tree.inspect).to eq(result_tree) }
end
end
context 'when using three operators' do
it 'returns a reverse descent parse tree' do
expect(described_class.seed('$VAR1 == "123" == $VAR2').tree)
.to be_a Gitlab::Ci::Pipeline::Expression::Lexeme::Equals
context 'when combining && and OR operators' do
subject { described_class.seed('$VAR1 == "a" || $VAR2 == "b" && $VAR3 == "c" || $VAR4 == "d" && $VAR5 == "e"').tree }
context 'when parenthesis engine is enabled' do
before do
stub_feature_flags(ci_if_parenthesis_enabled: true)
end
it 'returns operations in a correct order' do
expect(subject.inspect)
.to eq('or(or(equals($VAR1, "a"), and(equals($VAR2, "b"), equals($VAR3, "c"))), and(equals($VAR4, "d"), equals($VAR5, "e")))')
end
end
context 'when using a single variable token' do
it 'returns a single token instance' do
expect(described_class.seed('$VAR').tree)
.to be_a Gitlab::Ci::Pipeline::Expression::Lexeme::Variable
context 'when parenthesis engine is disabled (legacy)' do
before do
stub_feature_flags(ci_if_parenthesis_enabled: false)
end
it 'returns operations in a invalid order' do
expect(subject.inspect)
.to eq('or(equals($VAR1, "a"), and(equals($VAR2, "b"), or(equals($VAR3, "c"), and(equals($VAR4, "d"), equals($VAR5, "e")))))')
end
end
end
context 'when using parenthesis' do
subject { described_class.seed('(($VAR1 == "a" || $VAR2 == "b") && $VAR3 == "c" || $VAR4 == "d") && $VAR5 == "e"').tree }
before do
stub_feature_flags(ci_if_parenthesis_enabled: true)
end
context 'when using a single string token' do
it 'returns a single token instance' do
expect(described_class.seed('"some value"').tree)
.to be_a Gitlab::Ci::Pipeline::Expression::Lexeme::String
it 'returns operations in a correct order' do
expect(subject.inspect)
.to eq('and(or(and(or(equals($VAR1, "a"), equals($VAR2, "b")), equals($VAR3, "c")), equals($VAR4, "d")), equals($VAR5, "e"))')
end
end
context 'when expression is empty' do
it 'returns a null token' do
it 'raises a parsing error' do
expect { described_class.seed('').tree }
.to raise_error Gitlab::Ci::Pipeline::Expression::Parser::ParseError
end
end
context 'when expression is null' do
it 'returns a null token' do
expect(described_class.seed('null').tree)
.to be_a Gitlab::Ci::Pipeline::Expression::Lexeme::Null
end
end
context 'when two value tokens have no operator' do
it 'raises a parsing error' do
expect { described_class.seed('$VAR "text"').tree }
......@@ -66,5 +94,42 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Parser do
.to raise_error Gitlab::Ci::Pipeline::Expression::Lexeme::Operator::OperatorError
end
end
context 'when parenthesis are unmatched' do
context 'when parenthesis engine is enabled' do
before do
stub_feature_flags(ci_if_parenthesis_enabled: true)
end
where(:expression) do
[
'$VAR == (',
'$VAR2 == ("aa"',
'$VAR2 == ("aa"))',
'$VAR2 == "aa")',
'(($VAR2 == "aa")',
'($VAR2 == "aa"))'
]
end
with_them do
it 'raises a ParseError' do
expect { described_class.seed(expression).tree }
.to raise_error Gitlab::Ci::Pipeline::Expression::Parser::ParseError
end
end
end
context 'when parenthesis engine is disabled' do
before do
stub_feature_flags(ci_if_parenthesis_enabled: false)
end
it 'raises an SyntaxError' do
expect { described_class.seed('$VAR == (').tree }
.to raise_error Gitlab::Ci::Pipeline::Expression::Lexer::SyntaxError
end
end
end
end
end
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rspec-parameterized'
require 'spec_helper'
RSpec.describe Gitlab::Ci::Pipeline::Expression::Statement do
subject do
......@@ -109,6 +108,17 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Statement do
'$UNDEFINED_VARIABLE || $PRESENT_VARIABLE' | 'my variable'
'$UNDEFINED_VARIABLE == null || $PRESENT_VARIABLE' | true
'$PRESENT_VARIABLE || $UNDEFINED_VARIABLE == null' | 'my variable'
'($PRESENT_VARIABLE)' | 'my variable'
'(($PRESENT_VARIABLE))' | 'my variable'
'(($PRESENT_VARIABLE && null) || $EMPTY_VARIABLE == "")' | true
'($PRESENT_VARIABLE) && (null || $EMPTY_VARIABLE == "")' | true
'("string" || "test") == "string"' | true
'(null || ("test" == "string"))' | false
'("string" == ("test" && "string"))' | true
'("string" == ("test" || "string"))' | false
'("string" == "test" || "string")' | "string"
'("string" == ("string" || (("1" == "1") && ("2" == "3"))))' | true
end
with_them do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment