Place semicolon before inline comment (#197)

Author: rwestMSFT

SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.Comments.cs

@@ -33,6 +33,20 @@ internal abstract partial class SqlScriptGeneratorVisitor
         /// </summary>
         private bool _leadingCommentsEmitted = false;
 
+        /// <summary>
+        /// When true, suppresses trailing comment emission in HandleCommentsAfterFragment
+        /// for fragments whose LastTokenIndex matches or exceeds _suppressTrailingCommentsAfterIndex.
+        /// Used by GenerateStatementWithSemiColon to defer trailing comments until after
+        /// the semicolon has been placed, without affecting inter-clause comments.
+        /// </summary>
+        private bool _suppressTrailingComments = false;
+
+        /// <summary>
+        /// The LastTokenIndex of the statement for which trailing comments are being suppressed.
+        /// Only comments after this index are suppressed.
+        /// </summary>
+        private int _suppressTrailingCommentsAfterIndex = -1;
+
         #endregion
 
         #region Comment Preservation Methods
@@ -48,6 +62,8 @@ protected void SetTokenStreamForComments(IList<TSqlParserToken> tokenStream)
             _lastProcessedTokenIndex = -1;
             _emittedComments.Clear();
             _leadingCommentsEmitted = false;
+            _suppressTrailingComments = false;
+            _suppressTrailingCommentsAfterIndex = -1;
         }
 
         /// <summary>
@@ -192,6 +208,16 @@ protected void HandleCommentsAfterFragment(TSqlFragment fragment)
                 return;
             }
 
+            // When trailing comments are suppressed (e.g., during statement body generation
+            // before semicolon placement), skip emitting trailing comments only for fragments
+            // whose last token is at or past the statement boundary. Inter-clause comments
+            // (within the statement) are still emitted normally.
+            if (_suppressTrailingComments && fragment.LastTokenIndex >= _suppressTrailingCommentsAfterIndex)
+            {
+                UpdateLastProcessedIndex(fragment);
+                return;
+            }
+
             // Emit trailing comments and update tracking
             EmitTrailingComments(fragment);
             UpdateLastProcessedIndex(fragment);

SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CommonPhrases.cs

@@ -440,6 +440,46 @@ protected void GenerateSemiColonWhenNecessary(TSqlStatement node)
             }
         }
 
+        /// <summary>
+        /// Generates a statement fragment with semicolon placed before any trailing comments.
+        /// This prevents semicolons from being appended after single-line comments (-- style),
+        /// which would make them part of the comment text.
+        /// </summary>
+        protected void GenerateStatementWithSemiColon(TSqlStatement statement)
+        {
+            if (statement == null)
+            {
+                return;
+            }
+
+            // Handle comments before
+            HandleCommentsBeforeFragment(statement);
+
+            // Suppress trailing comment emission during statement body generation
+            // so that the semicolon can be placed before trailing comments.
+            // Only suppress for fragments at the statement boundary (LastTokenIndex).
+            bool previousSuppressState = _suppressTrailingComments;
+            int previousSuppressIndex = _suppressTrailingCommentsAfterIndex;
+            if (_options.PreserveComments && _generateSemiColon && !StatementsThatCannotHaveSemiColon.Contains(statement.GetType()))
+            {
+                _suppressTrailingComments = true;
+                _suppressTrailingCommentsAfterIndex = statement.LastTokenIndex;
+            }
+
+            // Generate the statement body
+            statement.Accept(this);
+
+            // Restore suppression state and emit semicolon before trailing comments
+            _suppressTrailingComments = previousSuppressState;
+            _suppressTrailingCommentsAfterIndex = previousSuppressIndex;
+
+            // Semicolon BEFORE trailing comments
+            GenerateSemiColonWhenNecessary(statement);
+
+            // Now emit trailing comments (after the semicolon)
+            HandleCommentsAfterFragment(statement);
+        }
+		
         protected void GenerateCommaSeparatedWithClause<T>(IList<T> fragments, bool indent, bool includeParentheses) where T : TSqlFragment
         {
             if (fragments != null && fragments.Count > 0)

SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.IfStatement.cs

@@ -18,8 +18,7 @@ public override void ExplicitVisit(IfStatement node)
 
             NewLineAndIndent();
             MarkAndPushAlignmentPoint(branchStatements);
-            GenerateFragmentIfNotNull(node.ThenStatement);
-            GenerateSemiColonWhenNecessary(node.ThenStatement);
+            GenerateStatementWithSemiColon(node.ThenStatement);
             PopAlignmentPoint();
 
             if (node.ElseStatement != null)
@@ -28,8 +27,7 @@ public override void ExplicitVisit(IfStatement node)
                 GenerateKeyword(TSqlTokenType.Else);
                 NewLineAndIndent();
                 MarkAndPushAlignmentPoint(branchStatements);
-                GenerateFragmentIfNotNull(node.ElseStatement);
-                GenerateSemiColonWhenNecessary(node.ElseStatement);
+                GenerateStatementWithSemiColon(node.ElseStatement);
                 PopAlignmentPoint();
             }
         }

SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.StatementList.cs

@@ -29,8 +29,7 @@ public override void ExplicitVisit(StatementList node)
                         }
                     }
 
-                    GenerateFragmentIfNotNull(statement);
-                    GenerateSemiColonWhenNecessary(statement);
+                    GenerateStatementWithSemiColon(statement);
                 }
             }
         }

SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.TSqlBatch.cs

@@ -13,9 +13,7 @@ public override void ExplicitVisit(TSqlBatch node)
         {
             foreach (TSqlStatement statement in node.Statements)
             {
-                GenerateFragmentIfNotNull(statement);
-
-                GenerateSemiColonWhenNecessary(statement);
+                GenerateStatementWithSemiColon(statement);
 
                 if (statement is TSqlStatementSnippet == false)
                 {

SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.WhileStatement.cs

@@ -18,8 +18,7 @@ public override void ExplicitVisit(WhileStatement node)
 
             NewLineAndIndent();
             MarkAndPushAlignmentPoint(whileBody);
-            GenerateFragmentIfNotNull(node.Statement);
-            GenerateSemiColonWhenNecessary(node.Statement);
+            GenerateStatementWithSemiColon(node.Statement);
             PopAlignmentPoint();
         }
     }

Test/SqlDom/ScriptGeneratorTests.cs

@@ -1256,6 +1256,102 @@ public void TestPreserveCommentsEnabled_CommentInNestedSubquery()
                 $"inner_q alias should appear before outer_q alias. inner_q at {innerQAliasIdx}, outer_q at {outerQAliasIdx}");
         }
 
+        [TestMethod]
+        [Priority(0)]
+        [SqlStudioTestCategory(Category.UnitTest)]
+        public void TestPreserveCommentsEnabled_SemicolonBeforeTrailingComment()
+        {
+            // Test that semicolons are placed BEFORE trailing single-line comments,
+            // not after them (which would make the semicolon part of the comment text).
+            // Bug fix: previously "SELECT 1 -- comment;" was generated instead of "SELECT 1; -- comment"
+            var sqlWithComments = "SELECT 1 -- trailing comment";
+            var parser = new TSql170Parser(true);
+            var fragment = parser.Parse(new StringReader(sqlWithComments), out var errors);
+
+            Assert.AreEqual(0, errors.Count);
+
+            var generatorOptions = new SqlScriptGeneratorOptions
+            {
+                PreserveComments = true,
+                IncludeSemicolons = true
+            };
+            var generator = new Sql170ScriptGenerator(generatorOptions);
+            generator.GenerateScript(fragment, out var generatedSql);
+
+            // The semicolon must appear BEFORE the trailing comment
+            int semicolonIndex = generatedSql.IndexOf(";");
+            int commentIndex = generatedSql.IndexOf("-- trailing comment");
+
+            Assert.IsTrue(semicolonIndex >= 0, "Semicolon should be present in output. Actual: " + generatedSql);
+            Assert.IsTrue(commentIndex >= 0, "Trailing comment should be preserved. Actual: " + generatedSql);
+            Assert.IsTrue(semicolonIndex < commentIndex,
+                $"Semicolon should appear before trailing comment. Semicolon at {semicolonIndex}, comment at {commentIndex}. Actual: " + generatedSql);
+        }
+
+        [TestMethod]
+        [Priority(0)]
+        [SqlStudioTestCategory(Category.UnitTest)]
+        public void TestPreserveCommentsEnabled_SemicolonBeforeTrailingComment_MultipleStatements()
+        {
+            // Test semicolon placement with multiple statements each having trailing comments
+            var sqlWithComments = @"SELECT 1 -- first comment
+SELECT 2 -- second comment";
+            var parser = new TSql170Parser(true);
+            var fragment = parser.Parse(new StringReader(sqlWithComments), out var errors);
+
+            Assert.AreEqual(0, errors.Count);
+
+            var generatorOptions = new SqlScriptGeneratorOptions
+            {
+                PreserveComments = true,
+                IncludeSemicolons = true
+            };
+            var generator = new Sql170ScriptGenerator(generatorOptions);
+            generator.GenerateScript(fragment, out var generatedSql);
+
+            // Both comments should be preserved
+            Assert.IsTrue(generatedSql.Contains("-- first comment"),
+                "First trailing comment should be preserved. Actual: " + generatedSql);
+            Assert.IsTrue(generatedSql.Contains("-- second comment"),
+                "Second trailing comment should be preserved. Actual: " + generatedSql);
+
+            // Verify semicolons appear before their respective comments, not after
+            Assert.IsFalse(generatedSql.Contains("-- first comment;"),
+                "Semicolon should not appear after first comment text. Actual: " + generatedSql);
+            Assert.IsFalse(generatedSql.Contains("-- second comment;"),
+                "Semicolon should not appear after second comment text. Actual: " + generatedSql);
+        }
+
+        [TestMethod]
+        [Priority(0)]
+        [SqlStudioTestCategory(Category.UnitTest)]
+        public void TestPreserveCommentsEnabled_SemicolonBeforeTrailingBlockComment()
+        {
+            // Test semicolon placement with trailing block comments
+            var sqlWithComments = "SELECT 1 /* trailing block comment */";
+            var parser = new TSql170Parser(true);
+            var fragment = parser.Parse(new StringReader(sqlWithComments), out var errors);
+
+            Assert.AreEqual(0, errors.Count);
+
+            var generatorOptions = new SqlScriptGeneratorOptions
+            {
+                PreserveComments = true,
+                IncludeSemicolons = true
+            };
+            var generator = new Sql170ScriptGenerator(generatorOptions);
+            generator.GenerateScript(fragment, out var generatedSql);
+
+            // The semicolon must appear BEFORE the trailing block comment
+            int semicolonIndex = generatedSql.IndexOf(";");
+            int commentIndex = generatedSql.IndexOf("/* trailing block comment */");
+
+            Assert.IsTrue(semicolonIndex >= 0, "Semicolon should be present in output. Actual: " + generatedSql);
+            Assert.IsTrue(commentIndex >= 0, "Trailing block comment should be preserved. Actual: " + generatedSql);
+            Assert.IsTrue(semicolonIndex < commentIndex,
+                $"Semicolon should appear before trailing block comment. Semicolon at {semicolonIndex}, comment at {commentIndex}. Actual: " + generatedSql);
+        }
+
         #endregion
     }
 }