diff options
author | drew cimino <dcimino@gitlab.com> | 2019-07-12 20:20:44 -0400 |
---|---|---|
committer | drew cimino <dcimino@gitlab.com> | 2019-07-12 20:20:44 -0400 |
commit | 0ca1e51cf5fee4f7b45ba149b945f5aebf71e8bf (patch) | |
tree | a6ee87289bcd98b06539b4fc561866b96db5e33f | |
parent | d50125a03fffe64cd8976f1c8b2e5b66e5fc5b2d (diff) | |
download | gitlab-ce-0ca1e51cf5fee4f7b45ba149b945f5aebf71e8bf.tar.gz |
Removing ci_variables_complex_expressions feature flag and deprecated code branchesremove-complex-expressions-feature-flag
-rw-r--r-- | lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb | 11 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/expression/lexer.rb | 17 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/expression/parser.rb | 39 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb | 36 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb | 28 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb | 52 |
6 files changed, 11 insertions, 172 deletions
diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb index e4cf360a1c1..0212fa9d661 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/pattern.rb @@ -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 diff --git a/lib/gitlab/ci/pipeline/expression/lexer.rb b/lib/gitlab/ci/pipeline/expression/lexer.rb index 22c210ae26b..7d7582612f9 100644 --- a/lib/gitlab/ci/pipeline/expression/lexer.rb +++ b/lib/gitlab/ci/pipeline/expression/lexer.rb @@ -17,17 +17,6 @@ module Gitlab Expression::Lexeme::Equals, Expression::Lexeme::Matches, Expression::Lexeme::NotEquals, - Expression::Lexeme::NotMatches - ].freeze - - NEW_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 @@ -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 diff --git a/lib/gitlab/ci/pipeline/expression/parser.rb b/lib/gitlab/ci/pipeline/expression/parser.rb index 589bf32a4d7..edb55edf356 100644 --- a/lib/gitlab/ci/pipeline/expression/parser.rb +++ b/lib/gitlab/ci/pipeline/expression/parser.rb @@ -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 diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb index 30ea3f3e28e..6ce4b321397 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/pattern_spec.rb @@ -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 diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb index d8db9c262a1..7c98e729b0b 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexer_spec.rb @@ -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 diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index a2c2e3653d5..b259ef711aa 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -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 |