Skip to content

Fix synthesized comment emission with comment flags #64488

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

Draft
S.H Jeong (z0rimo) wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Fix synthesized comment emission with comment flags #64488

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
fb37cd2
Fix synthesized comment emission with comment flags
z0rimo Sep 28, 2026
5b728e8
Merge branch 'main' into fix/64453-synthetic-comments
z0rimo Sep 28, 2026
0e51356
Merge branch 'main' into fix/64453-synthetic-comments
z0rimo Sep 29, 2026
44153bf
Merge branch 'main' into fix/64453-synthetic-comments
z0rimo Sep 29, 2026
2f32369
Merge branch 'main' into fix/64453-synthetic-comments
z0rimo Sep 29, 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
6 changes: 0 additions & 6 deletions tsc/internal/printer/printer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5471,9 +5471,6 @@ func (p *Printer) emitTrailingCommentsOfNode(node *ast.Node, emitFlags EmitFlags
}

func (p *Printer) emitLeadingSyntheticCommentsOfNode(node *ast.Node, emitFlags EmitFlags) {
if emitFlags&EFNoLeadingComments != 0 {
return
}
synth := p.emitContext.GetSyntheticLeadingComments(node)
for _, c := range synth {
p.emitLeadingSynthesizedComment(c)
Expand All @@ -5493,9 +5490,6 @@ func (p *Printer) emitLeadingSynthesizedComment(comment SynthesizedComment) {
}

func (p *Printer) emitTrailingSyntheticCommentsOfNode(node *ast.Node, emitFlags EmitFlags) {
if emitFlags&EFNoTrailingComments != 0 {
return
}
synth := p.emitContext.GetSyntheticTrailingComments(node)
for _, c := range synth {
p.emitTrailingSynthesizedComment(c)
Expand Down
42 changes: 42 additions & 0 deletions tsc/internal/printer/printer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2622,3 +2622,45 @@ func TestOmitTrailingSemicolon(t *testing.T) {
t.Fatalf("omit EmitSourceFile(for) = %q, want %q", got, "for (;;) { }")
}
}

func TestSynthesizedCommentsIgnoreNoCommentsEmitFlags(t *testing.T) {

@jakebailey Jake Bailey (jakebailey) Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We only add unit tests when we really really have to. You should test this with a compiler test if possible.

@jakebailey Jake Bailey (jakebailey) Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Though, it was an API based report, so...

@z0rimo S.H Jeong (z0rimo) Sep 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, that was my concern as well. Since the behavior under test requires attaching synthesized comments through the emit/printer API, I couldn't find a source-only compiler test that exercises the same path.

I used the printer unit test here so it can explicitly combine a real source comment, the emit flag, and a synthesized replacement comment.

Happy to move this to a compiler test if there's an existing harness/path that can exercise that API behavior.

@jakebailey Jake Bailey (jakebailey) Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, there isn't something like that.

@jakebailey Jake Bailey (jakebailey) Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't even have a Go API at the moment. So it's a bit of an odd ask, sort of

t.Parallel()

// EFNoLeadingComments/EFNoTrailingComments only suppress comments from the source text.
// Synthesized comments should still be emitted, so a transformer can replace a source comment.
data := []struct {
title string
input string
flags printer.EmitFlags
leading bool
trailing bool
output string
}{
{title: "Leading/NoFlags", input: "/* original */ x;", leading: true, output: "/* original */ /* rewritten */ x;"},
{title: "Leading/NoLeadingComments", input: "/* original */ x;", flags: printer.EFNoLeadingComments, leading: true, output: "/* rewritten */ x;"},
{title: "Leading/NoComments", input: "/* original */ x;", flags: printer.EFNoComments, leading: true, output: "/* rewritten */ x;"},
{title: "Trailing/NoFlags", input: "x; /* original */", trailing: true, output: "x; /* rewritten */ /* original */"},
{title: "Trailing/NoTrailingComments", input: "x; /* original */", flags: printer.EFNoTrailingComments, trailing: true, output: "x; /* rewritten */"},
{title: "Trailing/NoComments", input: "x; /* original */", flags: printer.EFNoComments, trailing: true, output: "x; /* rewritten */"},
}

for _, rec := range data {
t.Run(rec.title, func(t *testing.T) {
t.Parallel()
file := parsetestutil.ParseTypeScript(rec.input, false /*jsx*/)
parsetestutil.CheckDiagnostics(t, file)

emitContext := printer.NewEmitContext()
statement := file.Statements.Nodes[0]
emitContext.SetEmitFlags(statement, rec.flags)
if rec.leading {
emitContext.AddSyntheticLeadingComment(statement, ast.KindMultiLineCommentTrivia, " rewritten ", false /*hasTrailingNewLine*/)
}
if rec.trailing {
emitContext.AddSyntheticTrailingComment(statement, ast.KindMultiLineCommentTrivia, " rewritten ", false /*hasTrailingNewLine*/)
}

emittestutil.CheckEmit(t, emitContext, file, rec.output)
})
}
}