Skip to content

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

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions