Removing ci_variables_complex_expressions feature flag and deprecated code branches
This commit is contained in:
drew cimino
parent
d50125a03f
commit
0ca1e51cf5
|
|
@ -8,11 +8,10 @@ module Gitlab
|
|||
require_dependency 're2'
|
||||
|
||||
class Pattern < Lexeme::Value
|
||||
PATTERN = %r{^/.+/[ismU]*$}.freeze
|
||||
NEW_PATTERN = %r{^\/([^\/]|\\/)+[^\\]\/[ismU]*}.freeze
|
||||
PATTERN = %r{^\/([^\/]|\\/)+[^\\]\/[ismU]*}.freeze
|
||||
|
||||
def initialize(regexp)
|
||||
@value = self.class.eager_matching_with_escape_characters? ? regexp.gsub(/\\\//, '/') : regexp
|
||||
@value = regexp.gsub(/\\\//, '/')
|
||||
|
||||
unless Gitlab::UntrustedRegexp::RubySyntax.valid?(@value)
|
||||
raise Lexer::SyntaxError, 'Invalid regular expression!'
|
||||
|
|
@ -26,16 +25,12 @@ module Gitlab
|
|||
end
|
||||
|
||||
def self.pattern
|
||||
eager_matching_with_escape_characters? ? NEW_PATTERN : PATTERN
|
||||
PATTERN
|
||||
end
|
||||
|
||||
def self.build(string)
|
||||
new(string)
|
||||
end
|
||||
|
||||
def self.eager_matching_with_escape_characters?
|
||||
Feature.enabled?(:ci_variables_complex_expressions, default_enabled: true)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -10,17 +10,6 @@ module Gitlab
|
|||
SyntaxError = Class.new(Expression::ExpressionError)
|
||||
|
||||
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
|
||||
].freeze
|
||||
|
||||
NEW_LEXEMES = [
|
||||
Expression::Lexeme::Variable,
|
||||
Expression::Lexeme::String,
|
||||
Expression::Lexeme::Pattern,
|
||||
|
|
@ -58,7 +47,7 @@ module Gitlab
|
|||
|
||||
return tokens if @scanner.eos?
|
||||
|
||||
lexeme = available_lexemes.find do |type|
|
||||
lexeme = LEXEMES.find do |type|
|
||||
type.scan(@scanner).tap do |token|
|
||||
tokens.push(token) if token.present?
|
||||
end
|
||||
|
|
@ -71,10 +60,6 @@ module Gitlab
|
|||
|
||||
raise Lexer::SyntaxError, 'Too many tokens!'
|
||||
end
|
||||
|
||||
def available_lexemes
|
||||
Feature.enabled?(:ci_variables_complex_expressions, default_enabled: true) ? NEW_LEXEMES : LEXEMES
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -13,39 +13,6 @@ module Gitlab
|
|||
end
|
||||
|
||||
def tree
|
||||
if Feature.enabled?(:ci_variables_complex_expressions, default_enabled: true)
|
||||
rpn_parse_tree
|
||||
else
|
||||
reverse_descent_parse_tree
|
||||
end
|
||||
end
|
||||
|
||||
def self.seed(statement)
|
||||
new(Expression::Lexer.new(statement).tokens)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# This produces a reverse descent parse tree.
|
||||
# It does not support precedence of operators.
|
||||
def reverse_descent_parse_tree
|
||||
while token = @tokens.next
|
||||
case token.type
|
||||
when :operator
|
||||
token.build(@nodes.pop, tree).tap do |node|
|
||||
@nodes.push(node)
|
||||
end
|
||||
when :value
|
||||
token.build.tap do |leaf|
|
||||
@nodes.push(leaf)
|
||||
end
|
||||
end
|
||||
end
|
||||
rescue StopIteration
|
||||
@nodes.last || Lexeme::Null.new
|
||||
end
|
||||
|
||||
def rpn_parse_tree
|
||||
results = []
|
||||
|
||||
tokens_rpn.each do |token|
|
||||
|
|
@ -70,6 +37,12 @@ module Gitlab
|
|||
results.pop
|
||||
end
|
||||
|
||||
def self.seed(statement)
|
||||
new(Expression::Lexer.new(statement).tokens)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# Parse the expression into Reverse Polish Notation
|
||||
# (See: Shunting-yard algorithm)
|
||||
def tokens_rpn
|
||||
|
|
|
|||
|
|
@ -107,42 +107,6 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Pattern do
|
|||
expect(token.build.evaluate)
|
||||
.to eq Gitlab::UntrustedRegexp.new('some numeric \$ pattern')
|
||||
end
|
||||
|
||||
context 'with the ci_variables_complex_expressions feature flag disabled' do
|
||||
before do
|
||||
stub_feature_flags(ci_variables_complex_expressions: false)
|
||||
end
|
||||
|
||||
it 'is a greedy scanner for regexp boundaries' do
|
||||
scanner = StringScanner.new('/some .* / pattern/')
|
||||
|
||||
token = described_class.scan(scanner)
|
||||
|
||||
expect(token).not_to be_nil
|
||||
expect(token.build.evaluate)
|
||||
.to eq Gitlab::UntrustedRegexp.new('some .* / pattern')
|
||||
end
|
||||
|
||||
it 'does not recognize the \ escape character for /' do
|
||||
scanner = StringScanner.new('/some .* \/ pattern/')
|
||||
|
||||
token = described_class.scan(scanner)
|
||||
|
||||
expect(token).not_to be_nil
|
||||
expect(token.build.evaluate)
|
||||
.to eq Gitlab::UntrustedRegexp.new('some .* \/ pattern')
|
||||
end
|
||||
|
||||
it 'does not recognize the \ escape character for $' do
|
||||
scanner = StringScanner.new('/some numeric \$ pattern/')
|
||||
|
||||
token = described_class.scan(scanner)
|
||||
|
||||
expect(token).not_to be_nil
|
||||
expect(token.build.evaluate)
|
||||
.to eq Gitlab::UntrustedRegexp.new('some numeric \$ pattern')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#evaluate' do
|
||||
|
|
|
|||
|
|
@ -80,34 +80,6 @@ describe Gitlab::Ci::Pipeline::Expression::Lexer do
|
|||
it { is_expected.to eq(tokens) }
|
||||
end
|
||||
end
|
||||
|
||||
context 'with the ci_variables_complex_expressions feature flag turned off' do
|
||||
before do
|
||||
stub_feature_flags(ci_variables_complex_expressions: false)
|
||||
end
|
||||
|
||||
it 'incorrectly tokenizes conjunctive match statements as one match statement' do
|
||||
tokens = described_class.new('$PRESENT_VARIABLE =~ /my var/ && $EMPTY_VARIABLE =~ /nope/').tokens
|
||||
|
||||
expect(tokens.map(&:value)).to eq(['$PRESENT_VARIABLE', '=~', '/my var/ && $EMPTY_VARIABLE =~ /nope/'])
|
||||
end
|
||||
|
||||
it 'incorrectly tokenizes disjunctive match statements as one statement' do
|
||||
tokens = described_class.new('$PRESENT_VARIABLE =~ /my var/ || $EMPTY_VARIABLE =~ /nope/').tokens
|
||||
|
||||
expect(tokens.map(&:value)).to eq(['$PRESENT_VARIABLE', '=~', '/my var/ || $EMPTY_VARIABLE =~ /nope/'])
|
||||
end
|
||||
|
||||
it 'raises an error about && operators' do
|
||||
expect { described_class.new('$EMPTY_VARIABLE == "" && $PRESENT_VARIABLE').tokens }
|
||||
.to raise_error(Gitlab::Ci::Pipeline::Expression::Lexer::SyntaxError).with_message('Unknown lexeme found!')
|
||||
end
|
||||
|
||||
it 'raises an error about || operators' do
|
||||
expect { described_class.new('$EMPTY_VARIABLE == "" || $PRESENT_VARIABLE').tokens }
|
||||
.to raise_error(Gitlab::Ci::Pipeline::Expression::Lexer::SyntaxError).with_message('Unknown lexeme found!')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#lexemes' do
|
||||
|
|
|
|||
|
|
@ -1,6 +1,4 @@
|
|||
# TODO switch this back after the "ci_variables_complex_expressions" feature flag is removed
|
||||
# require 'fast_spec_helper'
|
||||
require 'spec_helper'
|
||||
require 'fast_spec_helper'
|
||||
require 'rspec-parameterized'
|
||||
|
||||
describe Gitlab::Ci::Pipeline::Expression::Statement do
|
||||
|
|
@ -118,54 +116,6 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do
|
|||
expect(subject.evaluate).to eq(value)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with the ci_variables_complex_expressions feature flag disabled' do
|
||||
before do
|
||||
stub_feature_flags(ci_variables_complex_expressions: false)
|
||||
end
|
||||
|
||||
where(:expression, :value) do
|
||||
'$PRESENT_VARIABLE == "my variable"' | true
|
||||
'"my variable" == $PRESENT_VARIABLE' | true
|
||||
'$PRESENT_VARIABLE == null' | false
|
||||
'$EMPTY_VARIABLE == null' | false
|
||||
'"" == $EMPTY_VARIABLE' | true
|
||||
'$EMPTY_VARIABLE' | ''
|
||||
'$UNDEFINED_VARIABLE == null' | true
|
||||
'null == $UNDEFINED_VARIABLE' | true
|
||||
'$PRESENT_VARIABLE' | 'my variable'
|
||||
'$UNDEFINED_VARIABLE' | nil
|
||||
"$PRESENT_VARIABLE =~ /var.*e$/" | true
|
||||
"$PRESENT_VARIABLE =~ /^var.*/" | false
|
||||
"$EMPTY_VARIABLE =~ /var.*/" | false
|
||||
"$UNDEFINED_VARIABLE =~ /var.*/" | false
|
||||
"$PRESENT_VARIABLE =~ /VAR.*/i" | true
|
||||
'$PATH_VARIABLE =~ /path/variable/' | true
|
||||
'$PATH_VARIABLE =~ /path\/variable/' | true
|
||||
'$FULL_PATH_VARIABLE =~ /^/a/full/path/variable/value$/' | true
|
||||
'$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/' | true
|
||||
'$PRESENT_VARIABLE != "my variable"' | false
|
||||
'"my variable" != $PRESENT_VARIABLE' | false
|
||||
'$PRESENT_VARIABLE != null' | true
|
||||
'$EMPTY_VARIABLE != null' | true
|
||||
'"" != $EMPTY_VARIABLE' | false
|
||||
'$UNDEFINED_VARIABLE != null' | false
|
||||
'null != $UNDEFINED_VARIABLE' | false
|
||||
"$PRESENT_VARIABLE !~ /var.*e$/" | false
|
||||
"$PRESENT_VARIABLE !~ /^var.*/" | true
|
||||
"$EMPTY_VARIABLE !~ /var.*/" | true
|
||||
"$UNDEFINED_VARIABLE !~ /var.*/" | true
|
||||
"$PRESENT_VARIABLE !~ /VAR.*/i" | false
|
||||
end
|
||||
|
||||
with_them do
|
||||
let(:text) { expression }
|
||||
|
||||
it "evaluates to `#{params[:value].inspect}`" do
|
||||
expect(subject.evaluate).to eq value
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#truthful?' do
|
||||
|
|
|
|||
Loading…
Reference in New Issue