Merge branch 'remove-complex-expressions-feature-flag' into 'master'

Removing ci_variables_complex_expressions feature flag and deprecated code branches

See merge request gitlab-org/gitlab-ce!30717

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

lib/gitlab/ci/pipeline/expression/lexer.rb

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

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

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

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

spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb

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