remove redundant semicolons, always add semicolon after last statement in lambdas and actions

commit_hash:d2b19f44d3c6f6e89f03c2c27e897d2b55e6174e

Author: zhvv117

yql/essentials/sql/v1/format/sql_format.cpp

@@ -43,6 +43,39 @@ TTokenIterator SkipWSOrComment(TTokenIterator curr, TTokenIterator end) {
     return curr;
 }
 
+TTokenIterator SkipWSOrCommentBackward(TTokenIterator curr, TTokenIterator begin) {
+    while (curr != begin && (curr->Name == "WS" || curr->Name == "COMMENT")) {
+        --curr;
+    }
+    return curr;
+}
+
+void SkipForValidate(
+    TTokenIterator& in,
+    TTokenIterator& out,
+    const TParsedTokenList& query,
+    const TParsedTokenList& formattedQuery
+) {
+    in = SkipWS(in, query.end());
+    out = SkipWS(out, formattedQuery.end());
+
+    while (
+        in != query.end() && in->Name == "SEMICOLON" &&
+        (out == formattedQuery.end() || out->Name != "SEMICOLON") &&
+        in != query.begin() && IsIn({"SEMICOLON", "LBRACE_CURLY", "AS"}, SkipWSOrCommentBackward(in - 1, query.begin())->Name)
+    ) {
+        in = SkipWS(++in, query.end());
+    }
+
+    auto inSkippedComments = SkipWSOrComment(in, query.end());
+    if (
+        out != formattedQuery.end() && out->Name == "SEMICOLON" &&
+        inSkippedComments != query.end() && IsIn({"RBRACE_CURLY", "END"}, inSkippedComments->Name)
+    ) {
+        out = SkipWS(++out, formattedQuery.end());
+    }
+}
+
 TParsedToken TransformTokenForValidate(TParsedToken token) {
     if (token.Name == "EQUALS2") {
         token.Name = "EQUALS";
@@ -61,8 +94,7 @@ bool Validate(const TParsedTokenList& query, const TParsedTokenList& formattedQu
     auto outEnd = formattedQuery.end();
 
     while (in != inEnd && out != outEnd) {
-        in = SkipWS(in, inEnd);
-        out = SkipWS(out, outEnd);
+        SkipForValidate(in, out, query, formattedQuery);
         if (in != inEnd && out != outEnd) {
             auto inToken = TransformTokenForValidate(*in);
             auto outToken = TransformTokenForValidate(*out);
@@ -82,8 +114,7 @@ bool Validate(const TParsedTokenList& query, const TParsedTokenList& formattedQu
             ++out;
         }
     }
-    in = SkipWS(in, inEnd);
-    out = SkipWS(out, outEnd);
+    SkipForValidate(in, out, query, formattedQuery);
     return in == inEnd && out == outEnd;
 }
 
@@ -777,20 +808,34 @@ friend struct TStaticData;
         }
     }
 
+    template <typename T>
+    void SkipSemicolons(const ::google::protobuf::RepeatedPtrField<T>& field, bool printOne = false) {
+        for (const auto& m : field) {
+            if (printOne) {
+                Visit(m);
+                printOne = false;
+            } else {
+                ++TokenIndex;
+            }
+        }
+        if (printOne) {
+            Out(';');
+        }
+    }
+
     void VisitDefineActionOrSubqueryBody(const TRule_define_action_or_subquery_body& msg) {
-        VisitRepeated(msg.GetBlock1());
+        SkipSemicolons(msg.GetBlock1());
         if (msg.HasBlock2()) {
             const auto& b = msg.GetBlock2();
             Visit(b.GetRule_sql_stmt_core1());
             for (auto block : b.GetBlock2()) {
-                VisitRepeated(block.GetBlock1());
+                SkipSemicolons(block.GetBlock1(), /* printOne = */ true);
                 if (!IsSimpleStatement(block.GetRule_sql_stmt_core2()).GetOrElse(false)) {
                     Out('\n');
                 }
                 Visit(block.GetRule_sql_stmt_core2());
             }
-
-            VisitRepeated(b.GetBlock3());
+            SkipSemicolons(b.GetBlock3(), /* printOne = */ true);
         }
     }
 
@@ -2344,18 +2389,19 @@ friend struct TStaticData;
     void VisitLambdaBody(const TRule_lambda_body& msg) {
         PushCurrentIndent();
         NewLine();
-        VisitRepeated(msg.GetBlock1());
+        SkipSemicolons(msg.GetBlock1());
         for (const auto& block : msg.GetBlock2()) {
-            Visit(block);
+            Visit(block.GetRule_lambda_stmt1());
+            SkipSemicolons(block.GetBlock2(), /* printOne = */ true);
             NewLine();
         }
 
         Visit(msg.GetToken3());
         ExprLineIndent = CurrentIndent;
 
         Visit(msg.GetRule_expr4());
-        VisitRepeated(msg.GetBlock5());
 
+        SkipSemicolons(msg.GetBlock5(), /* printOne = */ true);
         ExprLineIndent = 0;
 
         PopCurrentIndent();
@@ -3068,7 +3114,7 @@ class TSqlFormatter : public NSQLFormat::ISqlFormatter {
                 return false;
             }
 
-            if (!Validate(stmtFormattedTokens, stmtTokens)) {
+            if (!Validate(stmtTokens, stmtFormattedTokens)) {
                 issues.AddIssue(NYql::TIssue({}, TStringBuilder() << "Validation failed: " << currentQuery.Quote() << " != " << currentFormattedQuery.Quote()));
                 return false;
             }

yql/essentials/sql/v1/format/sql_format_ut.h

@@ -557,6 +557,25 @@ Y_UNIT_TEST(DefineActionOrSubquery) {
             "VALUES\n\t\t\t(1)\n\t\t;\n\tEND DEFINE;\n\n\t"
             "DEFINE SUBQUERY $c() AS\n\t\tSELECT\n\t\t\t1\n\t\t;\n\t"
             "END DEFINE;\n\tDO\n\t\t$b()\n\t;\n\n\tPROCESS $c();\nEND DEFINE;\n"},
+        {"define action $foo($bar) as;"
+            "$a = 10;; "
+            "$b = 20;;; "
+            "$c = $a + $b "
+            "end define",
+            "DEFINE ACTION $foo($bar) AS\n\t"
+            "$a = 10;\n\t"
+            "$b = 20;\n\t"
+            "$c = $a + $b;\n"
+            "END DEFINE;\n"},
+        {"define subquery $s() as;"
+            "select * from $t1 "
+            "union all select * from $t2 "
+            "end define",
+            "DEFINE SUBQUERY $s() AS\n\t"
+            "SELECT\n\t\t*\n\tFROM\n\t\t$t1\n\t"
+            "UNION ALL\n\t"
+            "SELECT\n\t\t*\n\tFROM\n\t\t$t2\n\t;\n"
+            "END DEFINE;\n"},
     };
 
     TSetup setup;
@@ -872,9 +891,11 @@ Y_UNIT_TEST(CompositeTypesAndQuestions) {
 Y_UNIT_TEST(Lambda) {
     TCases cases = {
         {"$f=($a,$b)->{$x=$a+$b;return $a*$x};$g=($a,$b?)->($a+$b??0);select $f(10,4),$g(1,2);",
-            "$f = ($a, $b) -> {\n\t$x = $a + $b;\n\tRETURN $a * $x\n};\n\n"
+            "$f = ($a, $b) -> {\n\t$x = $a + $b;\n\tRETURN $a * $x;\n};\n\n"
             "$g = ($a, $b?) -> ($a + $b ?? 0);\n\n"
             "SELECT\n\t$f(10, 4),\n\t$g(1, 2)\n;\n"},
+        {"$f=($arg)->{;$a=10;;$b=20;;;RETURN $a+$b}",
+            "$f = ($arg) -> {\n\t$a = 10;\n\t$b = 20;\n\tRETURN $a + $b;\n};\n"},
     };
 
     TSetup setup;
@@ -1444,7 +1465,7 @@ Y_UNIT_TEST(ExistsExpr) {
 Y_UNIT_TEST(LambdaInsideExpr) {
     TCases cases = {
         {"SELECT ListMap(AsList(1,2),($x)->{return $x+1});",
-            "SELECT\n\tListMap(\n\t\tAsList(1, 2), ($x) -> {\n\t\t\tRETURN $x + 1\n\t\t}\n\t)\n;\n"},
+            "SELECT\n\tListMap(\n\t\tAsList(1, 2), ($x) -> {\n\t\t\tRETURN $x + 1;\n\t\t}\n\t)\n;\n"},
     };
 
     TSetup setup;

yql/essentials/tests/sql/sql2yql/canondata/test_sql_format.test_action-combine_subqueries_with_table_param_/formatted.sql

@@ -8,7 +8,7 @@ $combineQueries = ($query, $list) -> {
             ($world) -> {
                 $queries = ListMap(
                     $list, ($arg) -> {
-                        RETURN FuncCode("Apply", QuoteCode($query), $world, ReprCode($arg))
+                        RETURN FuncCode("Apply", QuoteCode($query), $world, ReprCode($arg));
                     }
                 );
                 RETURN FuncCode("UnionAll", $queries);

yql/essentials/tests/sql/sql2yql/canondata/test_sql_format.test_action-dep_world_quote_code_/formatted.sql

@@ -8,7 +8,7 @@ $combineQueries = ($query, $list) -> {
             ($world) -> {
                 $queries = ListMap(
                     $list, ($arg) -> {
-                        RETURN FuncCode("Apply", QuoteCode($query), $world, ReprCode($arg))
+                        RETURN FuncCode("Apply", QuoteCode($query), $world, ReprCode($arg));
                     }
                 );
                 RETURN FuncCode("Extend", $queries);

yql/essentials/tests/sql/sql2yql/canondata/test_sql_format.test_action-eval_code_/formatted.sql

@@ -7,7 +7,7 @@ SELECT
 $inc = EvaluateCode(
     LambdaCode(
         ($x) -> {
-            RETURN FuncCode("+", $x, FuncCode("Int32", AtomCode("1")))
+            RETURN FuncCode("+", $x, FuncCode("Int32", AtomCode("1")));
         }
     )
 );

yql/essentials/tests/sql/sql2yql/canondata/test_sql_format.test_action-eval_filter_/formatted.sql

@@ -5,7 +5,7 @@ USE plato;
 $arg1 = "" || "";
 
 $arg2 = ($_item) -> {
-    RETURN TRUE
+    RETURN TRUE;
 };
 
 $arg3 = "" || "";

yql/essentials/tests/sql/sql2yql/canondata/test_sql_format.test_action-eval_for_/formatted.sql

@@ -28,15 +28,15 @@ EVALUATE FOR $i IN ListMap(ListFromRange(0, 0), $f) DO
 
 EVALUATE FOR $i IN Yql::Map(
     1 / 1, ($x) -> {
-        RETURN AsList($x)
+        RETURN AsList($x);
     }
 ) DO
     $action1($i)
 ;
 
 EVALUATE FOR $i IN Yql::Map(
     1 / 0, ($x) -> {
-        RETURN AsList($x)
+        RETURN AsList($x);
     }
 ) DO
     $action1($i)

yql/essentials/tests/sql/sql2yql/canondata/test_sql_format.test_action-eval_unresolved_type_arg_/formatted.sql

@@ -21,7 +21,7 @@ $myAddSuffix = ($row, $value) -> {
                                     FuncCode("Member", $r, AtomCode($i.Name)),
                                     ReprCode($value)
                                 )
-                            )
+                            );
                         }
                     )
                 );

yql/essentials/tests/sql/sql2yql/canondata/test_sql_format.test_action-evaluate_match_type_/formatted.sql

@@ -4,7 +4,7 @@ USE plato;
 
 $keep_only_last = ($row) -> {
     $members = ListFilter(StructMembers($row), ($x) -> (FIND($x, "key") IS NOT NULL));
-    RETURN ChooseMembers($row, $members)
+    RETURN ChooseMembers($row, $members);
 };
 
 SELECT

yql/essentials/tests/sql/sql2yql/canondata/test_sql_format.test_action-mixed_eval_typeof_world1_/formatted.sql

@@ -9,22 +9,22 @@ $force_remove_members = ($struct, $to_remove) -> {
             ($st) -> {
                 $to_keep = ListFlatMap(
                     StructTypeComponents(TypeHandle(TypeOf($struct))), ($x) -> {
-                        RETURN IF($x.Name NOT IN $to_remove, $x.Name)
+                        RETURN IF($x.Name NOT IN $to_remove, $x.Name);
                     }
                 );
                 RETURN FuncCode(
                     "AsStruct",
                     ListMap(
                         $to_keep,
                         ($x) -> {
-                            RETURN ListCode(AtomCode($x), FuncCode("Member", $st, AtomCode($x)))
+                            RETURN ListCode(AtomCode($x), FuncCode("Member", $st, AtomCode($x)));
                         }
                     )
-                )
+                );
             }
         )
     );
-    RETURN $remover($struct)
+    RETURN $remover($struct);
 };
 
 DEFINE ACTION $func($input, $output) AS