Merge pull request #138 from AlWoSp/awa/fix-semicolon-reformatting

Improve Formatter for Semicolon Rule

Author: CppCXY

CodeFormatCore/include/CodeFormatCore/Config/LuaStyleEnum.h

@@ -87,7 +87,7 @@ enum class AlignChainExpr {
 
 enum class EndStmtWithSemicolon {
     Keep,
-    Never,
+    ReplaceWithNewline,
     Always,
     SameLine
 };

CodeFormatCore/include/CodeFormatCore/Format/Analyzer/SemicolonAnalyzer.h

@@ -17,11 +17,11 @@ class SemicolonAnalyzer : public FormatAnalyzer {
 
 private:
     void AddSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t);
-    void InsertNewLineBeforeNode(LuaSyntaxNode n, const LuaSyntaxTree &t);
+    void InsertNewLineBeforeNextNode(LuaSyntaxNode n, const LuaSyntaxTree &t);
     void RemoveSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t);
-    bool IsFirstStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree &t);
     bool IsLastStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree &t);
     bool EndsWithSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t);
+    bool ContainsSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t);
     LuaSyntaxNode GetLastNonCommentToken(LuaSyntaxNode n, const LuaSyntaxTree &t);
 
     std::unordered_map<std::size_t, SemicolonStrategy> _semicolon;

CodeFormatCore/src/Config/LuaStyle.cpp

@@ -293,8 +293,8 @@ void LuaStyle::Parse(std::map<std::string, std::string, std::less<>> &configMap)
             end_statement_with_semicolon = EndStmtWithSemicolon::Keep;
         } else if (configMap.at("end_statement_with_semicolon") == "always") {
             end_statement_with_semicolon = EndStmtWithSemicolon::Always;
-        } else if (configMap.at("end_statement_with_semicolon") == "never") {
-            end_statement_with_semicolon = EndStmtWithSemicolon::Never;
+        } else if (configMap.at("end_statement_with_semicolon") == "replace_with_newline") {
+            end_statement_with_semicolon = EndStmtWithSemicolon::ReplaceWithNewline;
         } else if (configMap.at("end_statement_with_semicolon") == "same_line") {
             end_statement_with_semicolon = EndStmtWithSemicolon::SameLine;
         }

CodeFormatCore/src/Diagnostic/CodeStyle/CodeStyleChecker.cpp

@@ -96,7 +96,7 @@ void CodeStyleChecker::BasicResolve(LuaSyntaxNode syntaxNode, const LuaSyntaxTre
             }
             case TokenStrategy::StmtEndSemicolon: {
                 switch (d.GetState().GetStyle().end_statement_with_semicolon) {
-                    case EndStmtWithSemicolon::Never: {
+                    case EndStmtWithSemicolon::ReplaceWithNewline: {
                         d.PushDiagnostic(DiagnosticType::Semicolon, textRange,
                                          LText("expected statement not to end with ;"));
                         break;

CodeFormatCore/src/Format/Analyzer/SemicolonAnalyzer.cpp

@@ -17,20 +17,20 @@ void SemicolonAnalyzer::Analyze(FormatState &f, const LuaSyntaxTree &t) {
                 switch (f.GetStyle().end_statement_with_semicolon) {
                     case EndStmtWithSemicolon::Always: {
                         if (syntaxNode.GetSyntaxKind(t) == LuaSyntaxNodeKind::LabelStatement) {
-                            break; // labels should not end with semicolons
+                            break;// labels should not end with semicolons
                         }
                         if (!EndsWithSemicolon(syntaxNode, t)) {
                             AddSemicolon(syntaxNode, t);
                         }
                         break;
                     }
-                    case EndStmtWithSemicolon::Never: {
-                        // is on same line as other statement -> needs to go on new line
-                        if (!IsFirstStmtOfLine(syntaxNode, t)) {
-                            InsertNewLineBeforeNode(syntaxNode, t);
-                        }
-                        if (EndsWithSemicolon(syntaxNode, t)) {
-                            RemoveSemicolon(syntaxNode, t);
+                    case EndStmtWithSemicolon::ReplaceWithNewline: {
+                        // no action needed when there's no semicolons at all!
+                        if (ContainsSemicolon(syntaxNode, t)) {
+                            if (EndsWithSemicolon(syntaxNode, t)) {
+                                RemoveSemicolon(syntaxNode, t);
+                                InsertNewLineBeforeNextNode(syntaxNode, t);
+                            }
                         }
                         break;
                     }
@@ -82,9 +82,9 @@ void SemicolonAnalyzer::AddSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t) {
     }
 }
 
-void SemicolonAnalyzer::InsertNewLineBeforeNode(LuaSyntaxNode n, const LuaSyntaxTree &t) {
-    auto token = n.GetFirstToken(t); // line breaks are put in front of the statement itself by non-first statements
-    if (token.IsToken(t)) {
+void SemicolonAnalyzer::InsertNewLineBeforeNextNode(LuaSyntaxNode n, const LuaSyntaxTree &t) {
+    auto token = n.GetNextTokenSkipComment(t);
+    if (token.IsToken(t) && token.GetStartLine(t) == n.GetEndLine(t)) {
         _semicolon[token.GetIndex()] = SemicolonStrategy::InsertNewLine;
     }
 }
@@ -96,17 +96,6 @@ void SemicolonAnalyzer::RemoveSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t)
     }
 }
 
-bool SemicolonAnalyzer::IsFirstStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree &t) {
-    // check if current stmt starts on same line as the previous one ends
-    auto startCurrent = n.GetStartLine(t);
-    auto prev = n.GetPrevToken(t);
-    if (!prev.IsNull(t)) {
-        auto endPrev = prev.GetEndLine(t);
-        return endPrev < startCurrent;
-    }
-    return true;// there's no previous token
-}
-
 bool SemicolonAnalyzer::IsLastStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree &t) {
     // check if next stmt starts on same line as the current one ends
     auto currentEnd = n.GetEndLine(t);
@@ -123,6 +112,10 @@ bool SemicolonAnalyzer::EndsWithSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &
     return token.GetTokenKind(t) == ';';
 }
 
+bool SemicolonAnalyzer::ContainsSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t) {
+    return n.GetChildToken(';', t).IsToken(t);
+}
+
 LuaSyntaxNode SemicolonAnalyzer::GetLastNonCommentToken(LuaSyntaxNode n, const LuaSyntaxTree &t) {
     auto token = n.GetLastToken(t);
     switch (token.GetTokenKind(t)) {

Test/src/FormatStyle_unitest.cpp

@@ -1621,7 +1621,7 @@ local table1 = { 1, 2, 3 };
 )",
             style));
 
-    style.end_statement_with_semicolon = EndStmtWithSemicolon::Never;
+    style.end_statement_with_semicolon = EndStmtWithSemicolon::ReplaceWithNewline;
     EXPECT_TRUE(TestHelper::TestFormatted(
             R"(
 local func = testFunc; print(type(nil)); -- nil;
@@ -1634,6 +1634,8 @@ for i, v in ipairs(values) do
     end;
 end;
 local table1 = { 1, 2, 3 }
+local opts = { on_exit = function() job = nil end }
+local opts = { on_exit = function() job = nil; end };
 )",
             R"(
 local func = testFunc
@@ -1649,6 +1651,9 @@ for i, v in ipairs(values) do
     end
 end
 local table1 = { 1, 2, 3 }
+local opts = { on_exit = function() job = nil end }
+local opts = { on_exit = function() job = nil
+end }
 )",
             style));
 

docs/format_config.md

@@ -538,5 +538,5 @@ end_statement_with_semicolon
     每条语句都应该以分号结束，缺失的分号将被添加。
 - same_line
     单行中的多个语句可以用必要的分号分隔，但可选的分号将被移除
-- never
+- replace_with_newline
     任何语句都不能以分号结束，语句末尾的所有分号都将被移除，包含多个语句的行将被分隔成多行

docs/format_config_EN.md

@@ -535,5 +535,5 @@ The possible values are:
     every statement should end with semicolon, missing semicolons will be added
 - same_line
     multiple statements in a single line may be separated by necessary semicolons, but optional semicolons will be removed
-- never
+- replace_with_newline
     no statement should end with semicolon, all semicolons at the end of statements will be removed, lines with multiple statements will be separated into multiple lines