Const enum inlining and synthetic comment emit support #1599

weswigham · 2025-08-19T00:38:46Z

The synthetic comment support should unblock a handful of TODOs in the node builder for services logic. There might be some helpers missing that are used elsewhere, but all the core printer support is in, since that's most of our const enum emit logic that isn't checker-driven.

Despite appearances, this works differently from how we did const enum inlining in strada - there, we enabled substitutions in the ts transform and did const enum inlining late during the substitution pass (a super late pass invoked by the printer just before printing the node), which is also how transforms used to handle renaming exports and a few other things. Like other substitution uses, that's had to change. Now it's a transform executed at the end of the very end of the js transform pipeline instead, so you'll actually see the changed nodes in the AST. Unfortunately, since it's still checker-based, only nodes derived from a checked node get inlined. (So, for example, if a transform manifests an A.B from nothing into the result tree, and A.B refers to a const enum, the inliner transform doesn't know to inline it since it only inlines nodes related to parsed nodes - in practice, this doesn't seem to be a problem, since transforms don't generally manufacture new references from nothing to const enums - only to globals and helpers and such.)

Fixes #1216

Copilot

Pull Request Overview

This PR implements const enum inlining and synthetic comment emit support for the TypeScript Go port. The implementation enables the replacement of const enum property accesses with their literal values, along with optional synthetic comments to preserve the original const enum reference for debugging purposes.

Key changes include:

Adds const enum inlining transformer that replaces const enum accesses with literal values
Implements synthetic comment support in the printer for preserving original enum references
Extends the emit resolver interface to support constant value resolution

Reviewed Changes

Copilot reviewed 101 out of 101 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/transformers/inliners/constenum.go	New const enum inlining transformer that replaces enum property accesses with their literal values
internal/printer/printer.go	Enhanced printer with synthetic comment support and refactored comment handling
internal/printer/emitresolver.go	Extended emit resolver interface to include GetConstantValue method
internal/printer/emitcontext.go	Added synthetic comment structures and management methods
internal/compiler/emitter.go	Integrated const enum inlining transformer into compilation pipeline
internal/checker/emitresolver.go	Implemented GetConstantValue method in emit resolver
internal/checker/checker.go	Fixed symbol resolution for const enum inlining support
testdata/baselines/reference/*	Updated test baselines showing const enum inlining behavior

internal/transformers/inliners/constenum.go

jakebailey · 2025-08-19T00:40:13Z

Does this fix #1216?

weswigham · 2025-08-19T00:41:18Z

internal/checker/checker.go

@@ -15087,7 +15087,7 @@ func (c *Checker) resolveEntityName(name *ast.Node, meaning ast.SymbolFlags, ign
 				name.Parent != nil && name.Parent.Kind == ast.KindJSExportAssignment) {
 			c.markSymbolOfAliasDeclarationIfTypeOnly(getAliasDeclarationFromName(name), symbol, nil /*finalTarget*/, true /*overwriteEmpty*/, nil, "")
 		}
-		if symbol.Flags&meaning == 0 && !dontResolveAlias {
+		if symbol.Flags&meaning == 0 && !dontResolveAlias && symbol.Flags&ast.SymbolFlagsAlias != 0 {


In strada, checkExpressionCached on a.b.c.d caches resolved symbols on a.b.c and a.b and a. getConstantValue uses these cached symbols to shortcut its' logic. In corsa, it does not, and, in so doing, reveals that here we try to call resolveAlias on non-alias (module) symbols when we consider if we want to inline module.exports. This fixes that.

internal/compiler/emitter.go

jakebailey · 2025-08-19T00:44:39Z

internal/transformers/inliners/constenum.go

Are there more inliners than just constenum?

As of right now, no, but we've toyed with adding minifer-like emit passes in the past, so it's a logical place to group them.

I don't think that's going to happen, but 😄

jakebailey · 2025-08-19T00:45:19Z

internal/transformers/inliners/constenum.go

+	return tx.NewTransformer(tx.visit, emitContext)
+}
+
+func (tx *ConstEnumInliningTransformer) visit(node *ast.Node) *ast.Node {


Wow, I love how short this is

Yeah, I inlined some indirections from strada.

...ference/submodule/compiler/blockScopedEnumVariablesUseBeforeDef_verbatimModuleSyntax.js.diff

weswigham · 2025-08-19T00:49:15Z

...ference/submodule/compiler/blockScopedEnumVariablesUseBeforeDef_verbatimModuleSyntax.js.diff

+ }
+ const config = {
+-    a: AfterObject.A,
+    a: 2 /* AfterObject.A */,


I'm about to push the fix for this one (should be using the calculated options isolatedModules flag and not the raw one), but this test seems to raise an important issue in VMS, imo - const enums really just don't work under it under current rules, do they? Even within the same function...

The emit is bad, but IIRC this emits a checker error here anyway, so it's fair game.

weswigham · 2025-08-19T00:54:09Z

testdata/baselines/reference/submodule/conformance/jsxCheckJsxNoTypeArgumentsAllowed.js.diff

@@ -10,5 +10,5 @@
 -</>;
 +const component_1 = require("./component");
 +const React = require("react");
-+let x = (<component_1.MyComp />, <component_1.Prop> a={10} b="hi" />; // error, no type arguments in js
+let x = (<component_1.MyComp />, <Prop> a={10} b="hi" />; // error, no type arguments in js


This is the only diff that isn't a strict improvement to match strada, imo - seems like swapping from the binder-based import-only resolver to the full emit resolver has changed the behavior of the cjs transform on the Prop identifier here. Now, this is all syntactic gobbeldeygook because it's a jsx file with a jsx tag with a type argument, being generously reinterpreted as a comma expression, so the emit here doesn't actually matter, but might indicate a more important bug somewhere in the checker. It'd be trivial to swap the behavior back (without troubleshooting the root cause) by having both the import-only resolver and the checker-resolver on hand and using the import-only one for the cjs transform, though.

jakebailey · 2025-08-19T00:56:30Z

testdata/baselines/reference/submodule/compiler/constEnums.js.diff

@@ -37,9 +37,9 @@
 +    // correct cases: reference to the enum member from different enum declaration
 +    Enum1["W1"] = A0;
 +    if (typeof Enum1.W1 !== "string") Enum1[Enum1.W1] = "W1";
-+    Enum1["W2"] = Enum1.A0;
+    Enum1["W2"] = 100 /* Enum1.A0 */;


Not related to your PR, but this is an interesting case, since I assume this is due to us not doing enum merging properly or something

testdata/baselines/reference/submodule/conformance/jsxCheckJsxNoTypeArgumentsAllowed.js.diff

weswigham · 2025-08-19T00:59:41Z

And yea, this is logically the fix for #1216 since it reenables const enum inlining. I'll edit the OP to close it on merge.

weswigham · 2025-08-19T01:08:45Z

Looks like my one-liner change to resolveEntityName also fixed a bunch of fourslash tests - that's nice.

This reverts commit a4eb181.

weswigham · 2025-08-19T01:19:51Z

Or not? npm run updatefailing unskipped a bunch of tests like they pass now, but they all still crash...? What gives?

jakebailey · 2025-08-19T01:26:27Z

It looks like this panic is unrecovered, so the test suite crashed, leading all tests after it to appear passing.

SIGTRAP: trace trap
PC=0x486381 m=5 sigcode=128

goroutine 20581 gp=0xc0005156c0 m=5 mp=0xc0006bc008 [running]:
runtime.breakpoint()
	/opt/hostedtoolcache/go/1.25.0/x64/src/runtime/asm_amd64.s:388 +0x1 fp=0xc000c21a68 sp=0xc000c21a60 pc=0x486381
runtime.Breakpoint(...)
	/opt/hostedtoolcache/go/1.25.0/x64/src/runtime/proc.go:5431
github.com/microsoft/typescript-go/internal/debug.Fail(...)
	/home/runner/work/typescript-go/typescript-go/internal/debug/shared.go:14
github.com/microsoft/typescript-go/internal/checker.(*Checker).resolveAlias(0xc000963420?, 0x217ff68?)
	/home/runner/work/typescript-go/typescript-go/internal/checker/checker.go:15501 +0x36e fp=0xc000c21ad8 sp=0xc000c21a68 pc=0x115d20e
github.com/microsoft/typescript-go/internal/checker.(*Checker).ResolveAlias(...)
	/home/runner/work/typescript-go/typescript-go/internal/checker/checker.go:15495
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).getReferencedSymbolsForModuleIfDeclaredBySourceFile(0xc001c3e2e8, {0x217ff68, 0xc000ae3540}, 0xc000235c00?, 0xc001905a40, {0xc00029d720, 0x9, 0x9}, {0x0, 0x0, ...}, ...)
	/home/runner/work/typescript-go/typescript-go/internal/ls/findallreferences.go:631 +0x3c6 fp=0xc000c21bb0 sp=0xc000c21ad8 pc=0x1494aa6
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).getReferencedSymbolsForNode(0xc001c3e2e8, {0x217ff68, 0xc000ae3540}, 0x13?, 0xc00250ac60, 0xc001905a40, {0xc00029d720, 0x9, 0x9}, {0x0, ...}, ...)
	/home/runner/work/typescript-go/typescript-go/internal/ls/findallreferences.go:599 +0x899 fp=0xc000c21d88 sp=0xc000c21bb0 pc=0x14940f9
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).ProvideReferences(0xc001c3e2e8, {0x217ff68, 0xc000ae3540}, 0xc00133ced0)
	/home/runner/work/typescript-go/typescript-go/internal/ls/findallreferences.go:409 +0x195 fp=0xc000c21e60 sp=0xc000c21d88 pc=0x14921b5
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleReferences(0xbfa970495a0e337?, {0x217ff68, 0xc000ae3540}, 0xc00133ced0)
	/home/runner/work/typescript-go/typescript-go/internal/lsp/server.go:740 +0xa5 fp=0xc000c21eb8 sp=0xc000c21e60 pc=0x153b925
github.com/microsoft/typescript-go/internal/lsp.init.func1.registerRequestHandler[...].15({0x217ff68, 0xc000ae3540}, 0xc00133cf00)
	/home/runner/work/typescript-go/typescript-go/internal/lsp/server.go:535 +0x86 fp=0xc000c21ef8 sp=0xc000c21eb8 pc=0x1535346
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleRequestOrNotification(0xc001d963c0, {0x217ff68, 0xc000ae3540}, 0xc00133cf00)
	/home/runner/work/typescript-go/typescript-go/internal/lsp/server.go:471 +0x18c fp=0xc000c21f50 sp=0xc000c21ef8 pc=0x15399cc
github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop.func1()
	/home/runner/work/typescript-go/typescript-go/internal/lsp/server.go:376 +0x89 fp=0xc000c21fe0 sp=0xc000c21f50 pc=0x15385a9
runtime.goexit({})
	/opt/hostedtoolcache/go/1.25.0/x64/src/runtime/asm_amd64.s:1693 +0x1 fp=0xc000c21fe8 sp=0xc000c21fe0 pc=0x488301
created by github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop in goroutine 20499
	/home/runner/work/typescript-go/typescript-go/internal/lsp/server.go:396 +0x648

jakebailey · 2025-08-19T01:27:15Z

Wait, does Breakpoint fall over if no debugger is attached? Do we need to remove that line?

weswigham · 2025-08-19T01:33:32Z

Looks like it. I'll just remove the Debug.fail call from this PR to unblock it, since that was only a driveby swap to debug a test anyway.

jakebailey · 2025-08-19T01:37:19Z

Sent #1601 too.

jakebailey · 2025-08-19T02:08:24Z

internal/printer/emitcontext.go

@@ -917,3 +927,37 @@ func (c *EmitContext) VisitIterationBody(body *ast.Statement, visitor *ast.NodeV

 	return updated
 }
+
+func (c *EmitContext) SetSyntheticLeadingComments(node *ast.Node, comments []SynthesizedComment) *ast.Node {


Not this PR, but there are some callers of this I left commented out in the node builder (probably more than that though, just opining)

Sent #1607 before this task slipped out of my brain.

DoctorGester · 2025-08-20T09:19:06Z

And I just got rid of const enums in our code to switch to tsgo because I thought they are not coming back 😭

weswigham added 2 commits August 18, 2025 17:27

Add const enum inlining transform and synthetic comment emit support

3497c59

Accept baselines

eb5048e

weswigham requested review from jakebailey, gabritto and Copilot August 19, 2025 00:38

Copilot AI reviewed Aug 19, 2025

View reviewed changes

internal/transformers/inliners/constenum.go Show resolved Hide resolved

internal/transformers/inliners/constenum.go Show resolved Hide resolved

weswigham commented Aug 19, 2025

View reviewed changes

jakebailey reviewed Aug 19, 2025

View reviewed changes

internal/compiler/emitter.go Outdated Show resolved Hide resolved

jakebailey reviewed Aug 19, 2025

View reviewed changes

internal/compiler/emitter.go Outdated Show resolved Hide resolved

jakebailey reviewed Aug 19, 2025

View reviewed changes

...ference/submodule/compiler/blockScopedEnumVariablesUseBeforeDef_verbatimModuleSyntax.js.diff Outdated Show resolved Hide resolved

Use derived isolatedModules flag

0467926

weswigham commented Aug 19, 2025

View reviewed changes

jakebailey reviewed Aug 19, 2025

View reviewed changes

testdata/baselines/reference/submodule/conformance/jsxCheckJsxNoTypeArgumentsAllowed.js.diff Show resolved Hide resolved

Tons of fourslash changes?

a4eb181

Revert "Tons of fourslash changes?"

0f447ac

This reverts commit a4eb181.

Back to panic to fourslash new stuff is reported

48b1b76

weswigham enabled auto-merge August 19, 2025 02:03

weswigham requested a review from jakebailey August 19, 2025 02:03

jakebailey reviewed Aug 19, 2025

View reviewed changes

jakebailey approved these changes Aug 19, 2025

View reviewed changes

weswigham added this pull request to the merge queue Aug 19, 2025

Merged via the queue into microsoft:main with commit 809b2c0 Aug 19, 2025
22 checks passed

weswigham deleted the const-enum-inlining branch August 19, 2025 02:20

jakebailey mentioned this pull request Aug 19, 2025

Add missing calls to AddSyntheticLeadingComment in node builder #1607

Merged

This was referenced Aug 25, 2025

Const enums shouldn't be emitted after inlining #1632

Open

Update build TS versions microsoft/vscode#263304

Merged

Const enum inlining and synthetic comment emit support #1599

Const enum inlining and synthetic comment emit support #1599

Uh oh!

Conversation

weswigham commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

jakebailey commented Aug 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weswigham commented Aug 19, 2025

Uh oh!

weswigham commented Aug 19, 2025

Uh oh!

weswigham commented Aug 19, 2025

Uh oh!

jakebailey commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakebailey commented Aug 19, 2025

Uh oh!

weswigham commented Aug 19, 2025

Uh oh!

jakebailey commented Aug 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DoctorGester commented Aug 20, 2025

Uh oh!

Uh oh!

weswigham commented Aug 19, 2025 •

edited

Loading

jakebailey commented Aug 19, 2025 •

edited

Loading