Skip to content

adds formatter option for preserving comments (redo) #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dzsquared wants to merge 5 commits into
base: main
Choose a base branch
from
Open

adds formatter option for preserving comments (redo) #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6783258
revised approach
dzsquared Jan 29, 2026
db61757
expanded tests
dzsquared Jan 29, 2026
e134349
hybridized approach with v1
dzsquared Jan 30, 2026
6c81cbb
cleanup unused code
dzsquared Jan 30, 2026
b798359
excessive comment cleanup
dzsquared Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
284 changes: 284 additions & 0 deletions SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.Comments.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,284 @@
//------------------------------------------------------------------------------
// <copyright file="SqlScriptGeneratorVisitor.Comments.cs" company="Microsoft">
// Copyright (c) Microsoft Corporation. All rights reserved.
// </copyright>
//------------------------------------------------------------------------------
using System;
using System.Collections.Generic;

namespace Microsoft.SqlServer.TransactSql.ScriptDom.ScriptGenerator
{
internal abstract partial class SqlScriptGeneratorVisitor
{
#region Comment Tracking Fields

/// <summary>
/// Tracks the last token index processed for comment emission.
/// Used to find comments between visited fragments.
/// </summary>
private int _lastProcessedTokenIndex = -1;

/// <summary>
/// The current script's token stream, set when visiting begins.
/// </summary>
private IList<TSqlParserToken> _currentTokenStream;
Comment on lines +13 to +24
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _lastProcessedTokenIndex and _currentTokenStream fields are instance fields of SqlScriptGeneratorVisitor without any thread synchronization. If the same SqlScriptGenerator instance is used concurrently from multiple threads (which is not a typical pattern but is possible), these fields could be corrupted. Consider documenting that SqlScriptGenerator instances are not thread-safe and should not be shared across threads, or add appropriate thread-safety mechanisms.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@dzsquared dzsquared Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@llali curious your thoughts on thread safety for scriptgenerator. The overall overhead of a shared vs individual instances of the script generator across threads for common scenarios seems minimal so the justification doesn't seem to be there, but I'm potentially missing something.


/// <summary>
/// Tracks which comment tokens have already been emitted to avoid duplicates.
/// </summary>
private readonly HashSet<TSqlParserToken> _emittedComments = new HashSet<TSqlParserToken>();

/// <summary>
/// Tracks whether leading (file-level) comments have been emitted.
/// </summary>
private bool _leadingCommentsEmitted = false;

#endregion

#region Comment Preservation Methods

/// <summary>
/// Sets the token stream for comment tracking.
/// Call this before visiting the root node when PreserveComments is enabled.
/// </summary>
/// <param name="tokenStream">The token stream from the parsed script.</param>
protected void SetTokenStreamForComments(IList<TSqlParserToken> tokenStream)
{
_currentTokenStream = tokenStream;
_lastProcessedTokenIndex = -1;
_emittedComments.Clear();
_leadingCommentsEmitted = false;
}

/// <summary>
/// Emits comments that appear before the first fragment in the script (file-level leading comments).
/// Called once when generating the first fragment.
/// </summary>
/// <param name="fragment">The first fragment being generated.</param>
protected void EmitLeadingComments(TSqlFragment fragment)
{
if (!_options.PreserveComments || _currentTokenStream == null || fragment == null)
{
return;
}

if (fragment.FirstTokenIndex <= 0)
{
return;
}

for (int i = 0; i < fragment.FirstTokenIndex && i < _currentTokenStream.Count; i++)
{
var token = _currentTokenStream[i];
if (IsCommentToken(token) && !_emittedComments.Contains(token))
{
EmitCommentToken(token, isLeading: true);
_emittedComments.Add(token);
}
}
}

/// <summary>
/// Emits comments that appear in the gap between the last emitted token and the current fragment.
/// This captures comments embedded within sub-expressions.
/// </summary>
/// <param name="fragment">The fragment about to be generated.</param>
protected void EmitGapComments(TSqlFragment fragment)
{
if (!_options.PreserveComments || _currentTokenStream == null || fragment == null)
{
return;
}

int startIndex = _lastProcessedTokenIndex + 1;
int endIndex = fragment.FirstTokenIndex;

if (endIndex <= startIndex)
{
return;
}

for (int i = startIndex; i < endIndex && i < _currentTokenStream.Count; i++)
{
var token = _currentTokenStream[i];
if (IsCommentToken(token) && !_emittedComments.Contains(token))
{
EmitCommentToken(token, isLeading: true);
_emittedComments.Add(token);
_lastProcessedTokenIndex = i;
}
}
}

/// <summary>
/// Emits trailing comments that appear immediately after the fragment.
/// </summary>
/// <param name="fragment">The fragment that was just generated.</param>
protected void EmitTrailingComments(TSqlFragment fragment)
{
if (!_options.PreserveComments || _currentTokenStream == null || fragment == null)
{
return;
}

int lastTokenIndex = fragment.LastTokenIndex;
if (lastTokenIndex < 0 || lastTokenIndex >= _currentTokenStream.Count)
{
return;
}

// Scan for comments immediately following the fragment
for (int i = lastTokenIndex + 1; i < _currentTokenStream.Count; i++)
{
var token = _currentTokenStream[i];

if (IsCommentToken(token) && !_emittedComments.Contains(token))
{
EmitCommentToken(token, isLeading: false);
_emittedComments.Add(token);
_lastProcessedTokenIndex = i;
}
else if (token.TokenType != TSqlTokenType.WhiteSpace)
{
// Stop at next non-whitespace, non-comment token
break;
}
}
}

/// <summary>
/// Updates tracking after generating a fragment.
/// </summary>
/// <param name="fragment">The fragment that was just generated.</param>
protected void UpdateLastProcessedIndex(TSqlFragment fragment)
{
if (fragment != null && fragment.LastTokenIndex > _lastProcessedTokenIndex)
{
_lastProcessedTokenIndex = fragment.LastTokenIndex;
}
}

/// <summary>
/// Called from GenerateFragmentIfNotNull to handle comments before generating a fragment.
/// This is the key integration point that enables comments within sub-expressions.
/// </summary>
/// <param name="fragment">The fragment about to be generated.</param>
protected void BeforeGenerateFragment(TSqlFragment fragment)
{
if (!_options.PreserveComments || _currentTokenStream == null || fragment == null)
{
return;
}

// Emit file-level leading comments once
if (!_leadingCommentsEmitted)
{
EmitLeadingComments(fragment);
_leadingCommentsEmitted = true;
}

// Emit any comments in the gap between last processed token and this fragment
EmitGapComments(fragment);
}

/// <summary>
/// Called from GenerateFragmentIfNotNull to handle comments after generating a fragment.
/// </summary>
/// <param name="fragment">The fragment that was just generated.</param>
protected void AfterGenerateFragment(TSqlFragment fragment)
{
if (!_options.PreserveComments || _currentTokenStream == null || fragment == null)
{
return;
}

// Emit trailing comments and update tracking
EmitTrailingComments(fragment);
UpdateLastProcessedIndex(fragment);
}

/// <summary>
/// Emits a comment token to the output.
/// </summary>
/// <param name="token">The comment token.</param>
/// <param name="isLeading">True if this is a leading comment, false for trailing.</param>
private void EmitCommentToken(TSqlParserToken token, bool isLeading)
{
if (token == null)
{
return;
}

if (token.TokenType == TSqlTokenType.SingleLineComment)
{
if (!isLeading)
{
// Trailing: add space before comment
_writer.AddToken(ScriptGeneratorSupporter.CreateWhitespaceToken(1));
}

_writer.AddToken(new TSqlParserToken(TSqlTokenType.SingleLineComment, token.Text));

if (isLeading)
{
// After a leading comment, add newline
_writer.NewLine();
}
}
else if (token.TokenType == TSqlTokenType.MultilineComment)
{
if (!isLeading)
{
// Trailing: add space before comment
_writer.AddToken(ScriptGeneratorSupporter.CreateWhitespaceToken(1));
}

_writer.AddToken(new TSqlParserToken(TSqlTokenType.MultilineComment, token.Text));

if (isLeading)
{
// After a leading multi-line comment, add newline
_writer.NewLine();
}
}
}

/// <summary>
/// Emits any remaining comments at the end of the token stream.
/// Call this after visiting the root fragment to capture comments that appear
/// after the last statement (end-of-script comments).
/// </summary>
protected void EmitRemainingComments()
{
if (!_options.PreserveComments || _currentTokenStream == null)
{
return;
}

// Scan from the last processed token to the end of the token stream
for (int i = _lastProcessedTokenIndex + 1; i < _currentTokenStream.Count; i++)
{
var token = _currentTokenStream[i];
if (IsCommentToken(token) && !_emittedComments.Contains(token))
{
// End-of-script comments: add newline before, emit comment
_writer.NewLine();
_writer.AddToken(new TSqlParserToken(token.TokenType, token.Text));
_emittedComments.Add(token);
}
}
}

/// <summary>
/// Checks if a token is a comment token.
/// </summary>
private static bool IsCommentToken(TSqlParserToken token)
{
return token != null &&
(token.TokenType == TSqlTokenType.SingleLineComment ||
token.TokenType == TSqlTokenType.MultilineComment);
}

#endregion
}
}
9 changes: 9 additions & 0 deletions SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.TSqlScript.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ partial class SqlScriptGeneratorVisitor
{
public override void ExplicitVisit(TSqlScript node)
{
// Initialize token stream for comment preservation
if (_options.PreserveComments && node.ScriptTokenStream != null)
{
SetTokenStreamForComments(node.ScriptTokenStream);
}

Boolean firstItem = true;
foreach (var item in node.Batches)
{
Expand All @@ -28,6 +34,9 @@ public override void ExplicitVisit(TSqlScript node)

GenerateFragmentIfNotNull(item);
}

// Emit any remaining comments at end of script (after the last statement)
EmitRemainingComments();
}
}
}
7 changes: 7 additions & 0 deletions SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.Utils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,14 @@ protected void GenerateFragmentIfNotNull(TSqlFragment fragment)
{
if (fragment != null)
{
// Handle comments before generating the fragment
// This is the key integration point that enables comments within sub-expressions
BeforeGenerateFragment(fragment);

fragment.Accept(this);

// Handle comments after generating the fragment
AfterGenerateFragment(fragment);
}
}

Expand Down
3 changes: 3 additions & 0 deletions SqlScriptDom/ScriptDom/SqlServer/Settings/SqlScriptGeneratorOptions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@
<Setting name="NumNewlinesAfterStatement" type="int" default="1" min="0">
<Summary>Gets or sets the number of newlines to include after each statement</Summary>
</Setting>
<Setting name="PreserveComments" type="bool" default="false">
<Summary>Gets or sets a boolean indicating if comments from the original script should be preserved in the generated output</Summary>
</Setting>
</SettingGroup>
<SettingGroup name="CREATE TABLE">
<Setting name="AlignColumnDefinitionFields" type="bool" default="true">
Expand Down
9 changes: 9 additions & 0 deletions Test/SqlDom/Baselines170/MultiLineCommentTests170.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
SELECT 1;

SELECT 2;

SELECT 3;

SELECT 4;

SELECT 5;
10 changes: 10 additions & 0 deletions Test/SqlDom/Baselines170/SingleLineCommentTests170.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
SELECT 1;

SELECT *
FROM dbo.MyTable;

SELECT 2;

SELECT 3;

SELECT 4;
5 changes: 4 additions & 1 deletion Test/SqlDom/Only170SyntaxTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ public partial class SqlDomTests
new ParserTest170("OptimizedLockingTests170.sql", nErrors80: 2, nErrors90: 2, nErrors100: 2, nErrors110: 2, nErrors120: 2, nErrors130: 2, nErrors140: 2, nErrors150: 2, nErrors160: 2),
new ParserTest170("CreateEventSessionNotLikePredicate.sql", nErrors80: 2, nErrors90: 1, nErrors100: 1, nErrors110: 1, nErrors120: 1, nErrors130: 0, nErrors140: 0, nErrors150: 0, nErrors160: 0),
// Complex query with VECTOR types - parses syntactically in all versions (optimization fix), but VECTOR type only valid in TSql170
new ParserTest170("ComplexQueryTests170.sql")
new ParserTest170("ComplexQueryTests170.sql"),
// Comment preservation tests - basic SQL syntax works in all versions
new ParserTest170("SingleLineCommentTests170.sql"),
new ParserTest170("MultiLineCommentTests170.sql")
};

private static readonly ParserTest[] SqlAzure170_TestInfos =
Expand Down
Loading
Loading