From e957c078d58d176a01fb59d9a8c45f3022a532bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Guilherme=20Gon=C3=A7alves?= <ggoncalves@google.com>
Date: Wed, 29 Dec 2021 11:48:09 -0300
Subject: [PATCH] LibSQL: Properly parse ESCAPE expressions

The evaluation order of method parameters is unspecified in C++, and
so we couldn't rely on parse_statement() being called before
parse_escape() when building a MatchExpression.

With this patch, we explicitly parse what we need in the right order,
before building the MatchExpression object.
---
 Tests/LibSQL/TestSqlExpressionParser.cpp | 13 +++++++---
 Userland/Libraries/LibSQL/AST/Parser.cpp | 31 +++++++++++++++++-------
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/Tests/LibSQL/TestSqlExpressionParser.cpp b/Tests/LibSQL/TestSqlExpressionParser.cpp
index e11ff30208..67f6c31bd3 100644
--- a/Tests/LibSQL/TestSqlExpressionParser.cpp
+++ b/Tests/LibSQL/TestSqlExpressionParser.cpp
@@ -463,7 +463,7 @@ TEST_CASE(match_expression)
         EXPECT(parse(builder.build()).is_error());
     }
 
-    auto validate = [](StringView sql, SQL::AST::MatchOperator expected_operator, bool expected_invert_expression) {
+    auto validate = [](StringView sql, SQL::AST::MatchOperator expected_operator, bool expected_invert_expression, bool expect_escape) {
         auto result = parse(sql);
         EXPECT(!result.is_error());
 
@@ -475,6 +475,7 @@ TEST_CASE(match_expression)
         EXPECT(!is<SQL::AST::ErrorExpression>(*match.rhs()));
         EXPECT_EQ(match.type(), expected_operator);
         EXPECT_EQ(match.invert_expression(), expected_invert_expression);
+        EXPECT(match.escape() || !expect_escape);
     };
 
     for (auto op : operators) {
@@ -482,13 +483,19 @@ TEST_CASE(match_expression)
         builder.append("1 ");
         builder.append(op.key);
         builder.append(" 1");
-        validate(builder.build(), op.value, false);
+        validate(builder.build(), op.value, false, false);
 
         builder.clear();
         builder.append("1 NOT ");
         builder.append(op.key);
         builder.append(" 1");
-        validate(builder.build(), op.value, true);
+        validate(builder.build(), op.value, true, false);
+
+        builder.clear();
+        builder.append("1 NOT ");
+        builder.append(op.key);
+        builder.append(" 1 ESCAPE '+'");
+        validate(builder.build(), op.value, true, true);
     }
 }
 
diff --git a/Userland/Libraries/LibSQL/AST/Parser.cpp b/Userland/Libraries/LibSQL/AST/Parser.cpp
index d7c728a9ff..6ffd3c4877 100644
--- a/Userland/Libraries/LibSQL/AST/Parser.cpp
+++ b/Userland/Libraries/LibSQL/AST/Parser.cpp
@@ -739,22 +739,35 @@ RefPtr<Expression> Parser::parse_match_expression(NonnullRefPtr<Expression> lhs,
 {
     auto parse_escape = [this]() {
         RefPtr<Expression> escape;
-        if (consume_if(TokenType::Escape))
+        if (consume_if(TokenType::Escape)) {
             escape = parse_expression();
+        }
         return escape;
     };
 
-    if (consume_if(TokenType::Like))
-        return create_ast_node<MatchExpression>(MatchOperator::Like, move(lhs), parse_expression(), parse_escape(), invert_expression);
+    if (consume_if(TokenType::Like)) {
+        NonnullRefPtr<Expression> rhs = parse_expression();
+        RefPtr<Expression> escape = parse_escape();
+        return create_ast_node<MatchExpression>(MatchOperator::Like, move(lhs), move(rhs), move(escape), invert_expression);
+    }
 
-    if (consume_if(TokenType::Glob))
-        return create_ast_node<MatchExpression>(MatchOperator::Glob, move(lhs), parse_expression(), parse_escape(), invert_expression);
+    if (consume_if(TokenType::Glob)) {
+        NonnullRefPtr<Expression> rhs = parse_expression();
+        RefPtr<Expression> escape = parse_escape();
+        return create_ast_node<MatchExpression>(MatchOperator::Glob, move(lhs), move(rhs), move(escape), invert_expression);
+    }
 
-    if (consume_if(TokenType::Match))
-        return create_ast_node<MatchExpression>(MatchOperator::Match, move(lhs), parse_expression(), parse_escape(), invert_expression);
+    if (consume_if(TokenType::Match)) {
+        NonnullRefPtr<Expression> rhs = parse_expression();
+        RefPtr<Expression> escape = parse_escape();
+        return create_ast_node<MatchExpression>(MatchOperator::Match, move(lhs), move(rhs), move(escape), invert_expression);
+    }
 
-    if (consume_if(TokenType::Regexp))
-        return create_ast_node<MatchExpression>(MatchOperator::Regexp, move(lhs), parse_expression(), parse_escape(), invert_expression);
+    if (consume_if(TokenType::Regexp)) {
+        NonnullRefPtr<Expression> rhs = parse_expression();
+        RefPtr<Expression> escape = parse_escape();
+        return create_ast_node<MatchExpression>(MatchOperator::Regexp, move(lhs), move(rhs), move(escape), invert_expression);
+    }
 
     return {};
 }