Return boolean from Lexeme::Matches#evaluateboolean-values-from-matches-operator

3 files changed, 28 insertions, 59 deletions

diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
index d7f47c0e7e6..942e4e55323 100644
--- a/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
+++ b/lib/gitlab/ci/pipeline/expression/lexeme/matches.rb
@@ -13,16 +13,6 @@ module Gitlab
               regexp = @right.evaluate(variables)
 
               regexp.scan(text.to_s).any?
-
-              if ci_variables_complex_expressions?
-                # return offset of first match, or nil if no matches
-                if match = regexp.scan(text.to_s).first
-                  text.to_s.index(match)
-                end
-              else
-                # return true or false
-                regexp.scan(text.to_s).any?
-              end
             end
 
             def self.build(_value, behind, ahead)
@@ -32,12 +22,6 @@ module Gitlab
             def self.precedence
               10 # See: https://ruby-doc.org/core-2.5.0/doc/syntax/precedence_rdoc.html
             end
-
-            private
-
-            def ci_variables_complex_expressions?
-              Feature.enabled?(:ci_variables_complex_expressions)
-            end
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb
index 97da66d2bcc..a6fdec832a3 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/matches_spec.rb
@@ -50,21 +50,21 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do
       let(:left_value)  { 'my-string' }
       let(:right_value) { Gitlab::UntrustedRegexp.new('something') }
 
-      it { is_expected.to eq(nil) }
+      it { is_expected.to eq(false) }
     end
 
     context 'when left and right match' do
       let(:left_value)  { 'my-awesome-string' }
       let(:right_value) { Gitlab::UntrustedRegexp.new('awesome.string$') }
 
-      it { is_expected.to eq(3) }
+      it { is_expected.to eq(true) }
     end
 
     context 'when left is nil' do
       let(:left_value)  { nil }
       let(:right_value) { Gitlab::UntrustedRegexp.new('pattern') }
 
-      it { is_expected.to eq(nil) }
+      it { is_expected.to eq(false) }
     end
 
     context 'when left is a multiline string and matches right' do
@@ -78,7 +78,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do
 
       let(:right_value) { Gitlab::UntrustedRegexp.new('text-string') }
 
-      it { is_expected.to eq(24) }
+      it { is_expected.to eq(true) }
     end
 
     context 'when left is a multiline string and does not match right' do
@@ -92,7 +92,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do
 
       let(:right_value) { Gitlab::UntrustedRegexp.new('text-string') }
 
-      it { is_expected.to eq(nil) }
+      it { is_expected.to eq(false) }
     end
 
     context 'when a matching pattern uses regex flags' do
@@ -104,7 +104,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do
 
       let(:right_value) { Gitlab::UntrustedRegexp.new('(?i)awesome') }
 
-      it { is_expected.to eq(3) }
+      it { is_expected.to eq(true) }
     end
 
     context 'when a non-matching pattern uses regex flags' do
@@ -116,7 +116,7 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::Matches do
 
       let(:right_value) { Gitlab::UntrustedRegexp.new('(?i)terrible') }
 
-      it { is_expected.to eq(nil) }
+      it { is_expected.to eq(false) }
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb
index 057e2f3fbe8..a2c2e3653d5 100644
--- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb
@@ -41,17 +41,17 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do
       'null == $UNDEFINED_VARIABLE'                                 | true
       '$PRESENT_VARIABLE'                                           | 'my variable'
       '$UNDEFINED_VARIABLE'                                         | nil
-      "$PRESENT_VARIABLE =~ /var.*e$/"                              | 3
-      '$PRESENT_VARIABLE =~ /va\r.*e$/'                             | nil
-      '$PRESENT_VARIABLE =~ /va\/r.*e$/'                            | nil
-      "$PRESENT_VARIABLE =~ /var.*e$/"                              | 3
-      "$PRESENT_VARIABLE =~ /^var.*/"                               | nil
-      "$EMPTY_VARIABLE =~ /var.*/"                                  | nil
-      "$UNDEFINED_VARIABLE =~ /var.*/"                              | nil
-      "$PRESENT_VARIABLE =~ /VAR.*/i"                               | 3
-      '$PATH_VARIABLE =~ /path\/variable/'                          | 2
-      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/' | 0
-      '$FULL_PATH_VARIABLE =~ /\\/path\\/variable\\/value$/'        | 7
+      "$PRESENT_VARIABLE =~ /var.*e$/"                              | true
+      '$PRESENT_VARIABLE =~ /va\r.*e$/'                             | false
+      '$PRESENT_VARIABLE =~ /va\/r.*e$/'                            | false
+      "$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
+      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/' | true
+      '$FULL_PATH_VARIABLE =~ /\\/path\\/variable\\/value$/'        | true
       '$PRESENT_VARIABLE != "my variable"'                          | false
       '"my variable" != $PRESENT_VARIABLE'                          | false
       '$PRESENT_VARIABLE != null'                                   | true
@@ -82,7 +82,7 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do
       '"string" && "string"'                   | 'string'
       'null && null'                           | nil
 
-      '$PRESENT_VARIABLE =~ /my var/ && $EMPTY_VARIABLE =~ /nope/' | nil
+      '$PRESENT_VARIABLE =~ /my var/ && $EMPTY_VARIABLE =~ /nope/' | false
       '$EMPTY_VARIABLE == "" && $PRESENT_VARIABLE'                 | 'my variable'
       '$EMPTY_VARIABLE == "" && $PRESENT_VARIABLE != "nope"'       | true
       '$PRESENT_VARIABLE && $EMPTY_VARIABLE'                       | ''
@@ -90,17 +90,17 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do
       '$UNDEFINED_VARIABLE && $EMPTY_VARIABLE'                     | nil
       '$UNDEFINED_VARIABLE && $PRESENT_VARIABLE'                   | nil
 
-      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/ && $PATH_VARIABLE =~ /path\/variable/'      | 2
-      '$FULL_PATH_VARIABLE =~ /^\/a\/bad\/path\/variable\/value$/ && $PATH_VARIABLE =~ /path\/variable/'       | nil
-      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/ && $PATH_VARIABLE =~ /bad\/path\/variable/' | nil
-      '$FULL_PATH_VARIABLE =~ /^\/a\/bad\/path\/variable\/value$/ && $PATH_VARIABLE =~ /bad\/path\/variable/'  | nil
+      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/ && $PATH_VARIABLE =~ /path\/variable/'      | true
+      '$FULL_PATH_VARIABLE =~ /^\/a\/bad\/path\/variable\/value$/ && $PATH_VARIABLE =~ /path\/variable/'       | false
+      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/ && $PATH_VARIABLE =~ /bad\/path\/variable/' | false
+      '$FULL_PATH_VARIABLE =~ /^\/a\/bad\/path\/variable\/value$/ && $PATH_VARIABLE =~ /bad\/path\/variable/'  | false
 
-      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/ || $PATH_VARIABLE =~ /path\/variable/'      | 0
-      '$FULL_PATH_VARIABLE =~ /^\/a\/bad\/path\/variable\/value$/ || $PATH_VARIABLE =~ /path\/variable/'       | 2
-      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/ || $PATH_VARIABLE =~ /bad\/path\/variable/' | 0
-      '$FULL_PATH_VARIABLE =~ /^\/a\/bad\/path\/variable\/value$/ || $PATH_VARIABLE =~ /bad\/path\/variable/'  | nil
+      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/ || $PATH_VARIABLE =~ /path\/variable/'      | true
+      '$FULL_PATH_VARIABLE =~ /^\/a\/bad\/path\/variable\/value$/ || $PATH_VARIABLE =~ /path\/variable/'       | true
+      '$FULL_PATH_VARIABLE =~ /^\/a\/full\/path\/variable\/value$/ || $PATH_VARIABLE =~ /bad\/path\/variable/' | true
+      '$FULL_PATH_VARIABLE =~ /^\/a\/bad\/path\/variable\/value$/ || $PATH_VARIABLE =~ /bad\/path\/variable/'  | false
 
-      '$PRESENT_VARIABLE =~ /my var/ || $EMPTY_VARIABLE =~ /nope/' | 0
+      '$PRESENT_VARIABLE =~ /my var/ || $EMPTY_VARIABLE =~ /nope/' | true
       '$EMPTY_VARIABLE == "" || $PRESENT_VARIABLE'                 | true
       '$PRESENT_VARIABLE != "nope" || $EMPTY_VARIABLE == ""'       | true
 
@@ -117,21 +117,6 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do
       it "evaluates to `#{params[:value].inspect}`" do
         expect(subject.evaluate).to eq(value)
       end
-
-      # This test is used to ensure that our parser
-      # returns exactly the same results as if we
-      # were evaluating using ruby's `eval`
-      context 'when using Ruby eval' do
-        let(:expression_ruby) do
-          expression
-            .gsub(/null/, 'nil')
-            .gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)/) { "variables['#{Regexp.last_match(1)}']" }
-        end
-
-        it 'behaves exactly the same' do
-          expect(instance_eval(expression_ruby)).to eq(subject.evaluate)
-        end
-      end
     end
 
     context 'with the ci_variables_complex_expressions feature flag disabled' do