Repository navigation
Copy synthetic comments in SetOriginal and emit them once in TS trasforms #64736
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
7363ac9
c74815f
c17251e
2c4d14a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package printer_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/TypeScript/tsc/internal/ast" | ||
| "github.com/microsoft/TypeScript/tsc/internal/printer" | ||
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| func syntheticCommentTexts(comments []printer.SynthesizedComment) []string { | ||
| texts := make([]string, len(comments)) | ||
| for i, comment := range comments { | ||
| texts[i] = comment.Text | ||
| } | ||
| return texts | ||
| } | ||
|
|
||
| func TestSetOriginalCopiesSyntheticComments(t *testing.T) { | ||
| t.Parallel() | ||
| emitContext := printer.NewEmitContext() | ||
| factory := emitContext.Factory | ||
|
|
||
| original := factory.NewIdentifier("a") | ||
| emitContext.AddSyntheticLeadingComment(original, ast.KindMultiLineCommentTrivia, "leading", false /*hasTrailingNewLine*/) | ||
| emitContext.AddSyntheticTrailingComment(original, ast.KindMultiLineCommentTrivia, "trailing", false /*hasTrailingNewLine*/) | ||
|
|
||
| node := factory.NewIdentifier("b") | ||
| emitContext.AddSyntheticLeadingComment(node, ast.KindMultiLineCommentTrivia, "existing", false /*hasTrailingNewLine*/) | ||
| emitContext.SetOriginal(node, original) | ||
|
|
||
| // The original node's comments are prepended to the ones the node already has. | ||
| assert.DeepEqual(t, syntheticCommentTexts(emitContext.GetSyntheticLeadingComments(node)), []string{"leading", "existing"}) | ||
| assert.DeepEqual(t, syntheticCommentTexts(emitContext.GetSyntheticTrailingComments(node)), []string{"trailing"}) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package estransforms_test | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/TypeScript/tsc/internal/ast" | ||
| "github.com/microsoft/TypeScript/tsc/internal/core" | ||
| "github.com/microsoft/TypeScript/tsc/internal/printer" | ||
| "github.com/microsoft/TypeScript/tsc/internal/testutil/emittestutil" | ||
| "github.com/microsoft/TypeScript/tsc/internal/testutil/parsetestutil" | ||
| "github.com/microsoft/TypeScript/tsc/internal/transformers" | ||
| "github.com/microsoft/TypeScript/tsc/internal/transformers/estransforms" | ||
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| // When a static property initializer is moved into a `static {}` block, SetOriginal copies the | ||
| // property's synthetic comments to both the block and the statement inside it, so the transform | ||
| // must keep them on the block only. | ||
| func TestStaticPropertySyntheticCommentsAreEmittedOnce(t *testing.T) { | ||
| t.Parallel() | ||
| file := parsetestutil.ParseTypeScript("class C { static x = 1; }", false /*jsx*/) | ||
| parsetestutil.CheckDiagnostics(t, file) | ||
| property := file.Statements.Nodes[0].Members()[0] | ||
| emitContext := printer.NewEmitContext() | ||
| emitContext.AddSyntheticLeadingComment(property, ast.KindMultiLineCommentTrivia, "leading", false /*hasTrailingNewLine*/) | ||
| emitContext.AddSyntheticTrailingComment(property, ast.KindMultiLineCommentTrivia, "trailing", false /*hasTrailingNewLine*/) | ||
| compilerOptions := &core.CompilerOptions{Target: core.ScriptTargetES2022, UseDefineForClassFields: core.TSFalse} | ||
| opts := &transformers.TransformOptions{CompilerOptions: compilerOptions, Context: emitContext} | ||
| file = estransforms.GetESTransformer(opts).TransformSourceFile(file) | ||
| output := emittestutil.Emit(t, emitContext, file) | ||
| assert.Assert(t, strings.Contains(output, "static {"), output) | ||
| assert.Equal(t, strings.Count(output, "/*leading*/"), 1, output) | ||
| assert.Equal(t, strings.Count(output, "/*trailing*/"), 1, output) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package tstransforms_test | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find this file a little weird. We typically don't do unit tests for these, and this duplicates a helper string, and tests multiple packages ad-hoc.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair points. I looked at doing this as a compiler baseline test instead, but I don't think one can see it: synthetic comments only reach these transforms when an API user adds them (that's how we hit it, via a custom transformer). In a normal emit, the only synthetic comments in JS output come from the const enum inliner, which runs after these transforms, so a baseline passes with or without the change. I trimmed the test down instead in 2c4d14a. It now just checks that each comment is emitted once rather than matching the whole output, so the duplicated
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we have to do something annoying here because this is all Go API related. Just not mega satisfied with these ad hoc tests. Perhaps it's fine. |
||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/TypeScript/tsc/internal/ast" | ||
| "github.com/microsoft/TypeScript/tsc/internal/checker" | ||
| "github.com/microsoft/TypeScript/tsc/internal/core" | ||
| "github.com/microsoft/TypeScript/tsc/internal/printer" | ||
| "github.com/microsoft/TypeScript/tsc/internal/testutil/emittestutil" | ||
| "github.com/microsoft/TypeScript/tsc/internal/testutil/parsetestutil" | ||
| "github.com/microsoft/TypeScript/tsc/internal/transformers" | ||
| "github.com/microsoft/TypeScript/tsc/internal/transformers/tstransforms" | ||
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| // SetOriginal copies the synthetic comments of a declaration to every node a transform creates from | ||
| // it, so each transform must keep them on exactly one of the emitted nodes. | ||
| // | ||
| // For an enum or namespace, both comments belong on the `var` statement, as in TypeScript 6. The | ||
| // trailing one is currently missing there because EFNoTrailingComments on that statement also | ||
| // suppresses synthetic comments (#64453). | ||
| func TestSyntheticCommentsAreEmittedOnce(t *testing.T) { | ||
| t.Parallel() | ||
| data := []struct { | ||
| title string | ||
| input string | ||
| newTx func(opts *transformers.TransformOptions) *transformers.Transformer | ||
| trailingCount int | ||
| }{ | ||
| {title: "EnumDeclaration", input: "enum E { A }", newTx: tstransforms.NewRuntimeSyntaxTransformer, trailingCount: 0}, | ||
| {title: "ModuleDeclaration", input: "namespace N { export const x = 1; }", newTx: tstransforms.NewRuntimeSyntaxTransformer, trailingCount: 0}, | ||
| {title: "DecoratedClass", input: "declare function dec(c: any): any;\n@dec class C {}", newTx: tstransforms.NewLegacyDecoratorsTransformer, trailingCount: 1}, | ||
| } | ||
|
|
||
| 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) | ||
| compilerOptions := &core.CompilerOptions{ExperimentalDecorators: core.TSTrue, Target: core.ScriptTargetESNext} | ||
| esnext := func(ast.HasFileName) core.ModuleKind { return core.ModuleKindESNext } | ||
| c, _ := checker.NewChecker(&fakeProgram{ | ||
| singleThreaded: true, | ||
| compilerOptions: compilerOptions, | ||
| files: []*ast.SourceFile{file}, | ||
| getEmitModuleFormatOfFile: esnext, | ||
| getImpliedNodeFormatForEmit: esnext, | ||
| }, nil) | ||
| emitContext := printer.NewEmitContext() | ||
| emitResolver := c.NewEmitResolver(emitContext) | ||
| declaration := file.Statements.Nodes[len(file.Statements.Nodes)-1] | ||
| emitContext.AddSyntheticLeadingComment(declaration, ast.KindMultiLineCommentTrivia, "leading", false /*hasTrailingNewLine*/) | ||
| emitContext.AddSyntheticTrailingComment(declaration, ast.KindMultiLineCommentTrivia, "trailing", false /*hasTrailingNewLine*/) | ||
| opts := &transformers.TransformOptions{CompilerOptions: compilerOptions, Context: emitContext, EmitResolver: emitResolver, Resolver: emitResolver} | ||
| file = tstransforms.NewTypeEraserTransformer(opts).TransformSourceFile(file) | ||
| file = rec.newTx(opts).TransformSourceFile(file) | ||
| output := emittestutil.Emit(t, emitContext, file) | ||
| assert.Equal(t, strings.Count(output, "/*leading*/"), 1, output) | ||
| assert.Equal(t, strings.Count(output, "/*trailing*/"), rec.trailingCount, output) | ||
| }) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.