PreserveComments: trailing single-line comments are re-anchored to a different node, and output is not idempotent

Open
#225 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
48/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Quiet
Tech stack
csharp, sql
Domain
tooling

Research direction

Start by running the supplied C# three-pass reproduction with TSql170Parser, Sql170ScriptGenerator, and PreserveComments enabled. Trace comment handling during GenerateScript for multiple trailing comments on CASE WHEN branches; done means comments remain associated with their branches or statement clause and the second formatting pass produces no changes.

Written by the indexing model from the issue text.

Description

Describe the bug

With PreserveComments = true, a trailing -- comment attached to a WHEN branch of a CASE expression is emitted against a different node than the one it annotated. Formatting the generated output a second time moves the comment again — this time into the middle of the FROM clause. Output only becomes stable on the third pass, by which point the comment sits in a clause unrelated to the expression it documented.

No exception is thrown and the parser reports no errors on any pass — the only symptom is wrong output, which is what makes it easy to ship unnoticed.

Two problems, one root cause:

  1. Semantic relocation — a comment that documented WHEN a = 1 ends up annotating the whole select element, and a comment that documented WHEN a = 2 ends up inside FROM. For code where comments carry the rationale for individual CASE branches, the regenerated script is actively misleading: the text is preserved but the association is lost.
  2. Non-idempotencyFormat(Format(x)) != Format(x). This breaks the usual formatter contract and makes the generator unusable behind a format-then-verify-no-diff gate, which is how formatters are normally enforced in CI.
Input
SELECT CASE WHEN a = 1 THEN 1 -- one
            WHEN a = 2 THEN 2 -- two
            ELSE 3 END AS x
FROM t;
Actual output
--- pass 1 (changed: True) ---
SELECT CASE WHEN a = 1 THEN 1 WHEN a = 2 THEN 2 ELSE 3 END AS x -- one
 -- two
FROM   t;

--- pass 2 (changed: True) ---
SELECT CASE WHEN a = 1 THEN 1 WHEN a = 2 THEN 2 ELSE 3 END AS x -- one
FROM   -- two
       t;

--- pass 3 (changed: False) ---
SELECT CASE WHEN a = 1 THEN 1 WHEN a = 2 THEN 2 ELSE 3 END AS x -- one
FROM   -- two
       t;

Note also the stray leading space on the -- two line in pass 1.

A single WHEN branch with a trailing comment is stable — two or more branches are needed to reproduce.

Expected behaviour

Each trailing comment stays attached to the construct it followed in the source, and the second pass is a no-op. Something along these lines would be acceptable:

SELECT CASE WHEN a = 1 THEN 1 -- one
            WHEN a = 2 THEN 2 -- two
            ELSE 3 END AS x
FROM   t;

If per-branch anchoring inside a collapsed expression is not feasible, then keeping every comment within the statement clause it originated in — and guaranteeing idempotency — would still be a large improvement over the current behaviour.

Repro
using Microsoft.SqlServer.TransactSql.ScriptDom;

const string sql = """
    SELECT CASE WHEN a = 1 THEN 1 -- one
                WHEN a = 2 THEN 2 -- two
                ELSE 3 END AS x
    FROM t;
    """;

static string Format(string input)
{
    var parser = new TSql170Parser(true);
    var tree = parser.Parse(new StringReader(input), out var errors);
    if (errors.Count > 0) throw new Exception(errors[0].Message);

    var generator = new Sql170ScriptGenerator(new SqlScriptGeneratorOptions
    {
        PreserveComments = true
    });
    generator.GenerateScript(tree, out var output);
    return output;
}

var current = sql;
for (var pass = 1; pass <= 3; pass++)
{
    var next = Format(current);
    Console.WriteLine($"--- pass {pass} (changed: {next != current}) ---");
    Console.WriteLine(next);
    current = next;
}
Environment
  • Microsoft.SqlServer.TransactSql.ScriptDom 180.78.1 (assembly 18.0.0.0), net8.0
  • Reproduces identically with Sql160ScriptGenerator, Sql170ScriptGenerator, Sql180ScriptGenerator (and their matching parsers)
  • .NET 10, macOS
Impact / context

We evaluated the generator as a formatter for a T-SQL codebase in which comments routinely annotate individual CASE branches. In that setting the relocation is worse than comment loss would be: the text survives, so the output looks fine, but the comment now explains a different expression — plausible enough to pass review unnoticed. The non-idempotency is a separate blocker, since it rules out enforcing the formatter with a format-then-check-for-diff step. Happy to test a fix if that would help.

Related

Adjacent but distinct: #194 (leading newline ahead of multi-line comments) is fixed and covers block comments; #20 is the original PreserveComments request. I could not find an existing report covering trailing single-line comment re-anchoring or the resulting non-idempotency.

Dominant language
GAP
Stars
277
Forks
43
Avg merge
6d 17h
Merged PRs (30d)
3

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from microsoft/SqlScriptDOM

All issues in microsoft/SqlScriptDOM

Similar issues

More DevTools issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.