From 692c7877a56d01d1edf878bc6ebd29091e5ef733 Mon Sep 17 00:00:00 2001 From: Randolph West MSFT <97149825+rwestMSFT@users.noreply.github.com> Date: Mon, 6 Apr 2026 23:59:29 -0600 Subject: [PATCH] Place semicolon before inline comment --- .../SqlScriptGeneratorVisitor.Comments.cs | 26 +++++ ...SqlScriptGeneratorVisitor.CommonPhrases.cs | 40 ++++++++ .../SqlScriptGeneratorVisitor.IfStatement.cs | 6 +- ...SqlScriptGeneratorVisitor.StatementList.cs | 3 +- .../SqlScriptGeneratorVisitor.TSqlBatch.cs | 4 +- ...qlScriptGeneratorVisitor.WhileStatement.cs | 3 +- Test/SqlDom/ScriptGeneratorTests.cs | 96 +++++++++++++++++++ 7 files changed, 167 insertions(+), 11 deletions(-) diff --git a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.Comments.cs b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.Comments.cs index 1ea9fec..697e8b1 100644 --- a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.Comments.cs +++ b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.Comments.cs @@ -33,6 +33,20 @@ internal abstract partial class SqlScriptGeneratorVisitor /// private bool _leadingCommentsEmitted = false; + /// + /// 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. + /// + private bool _suppressTrailingComments = false; + + /// + /// The LastTokenIndex of the statement for which trailing comments are being suppressed. + /// Only comments after this index are suppressed. + /// + private int _suppressTrailingCommentsAfterIndex = -1; + #endregion #region Comment Preservation Methods @@ -48,6 +62,8 @@ protected void SetTokenStreamForComments(IList tokenStream) _lastProcessedTokenIndex = -1; _emittedComments.Clear(); _leadingCommentsEmitted = false; + _suppressTrailingComments = false; + _suppressTrailingCommentsAfterIndex = -1; } /// @@ -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); diff --git a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CommonPhrases.cs b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CommonPhrases.cs index f66d6fe..bcfb13a 100644 --- a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CommonPhrases.cs +++ b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.CommonPhrases.cs @@ -440,6 +440,46 @@ protected void GenerateSemiColonWhenNecessary(TSqlStatement node) } } + /// + /// 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. + /// + 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(IList fragments, bool indent, bool includeParentheses) where T : TSqlFragment { if (fragments != null && fragments.Count > 0) diff --git a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.IfStatement.cs b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.IfStatement.cs index c5ea4f5..c73aa15 100644 --- a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.IfStatement.cs +++ b/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(); } } diff --git a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.StatementList.cs b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.StatementList.cs index 6dedb21..4c87298 100644 --- a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.StatementList.cs +++ b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.StatementList.cs @@ -29,8 +29,7 @@ public override void ExplicitVisit(StatementList node) } } - GenerateFragmentIfNotNull(statement); - GenerateSemiColonWhenNecessary(statement); + GenerateStatementWithSemiColon(statement); } } } diff --git a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.TSqlBatch.cs b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.TSqlBatch.cs index e40318a..0420590 100644 --- a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.TSqlBatch.cs +++ b/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) { diff --git a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.WhileStatement.cs b/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.WhileStatement.cs index e7cb6d8..4369666 100644 --- a/SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.WhileStatement.cs +++ b/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(); } } diff --git a/Test/SqlDom/ScriptGeneratorTests.cs b/Test/SqlDom/ScriptGeneratorTests.cs index 5a21409..34eebbc 100644 --- a/Test/SqlDom/ScriptGeneratorTests.cs +++ b/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 } }