Revamp user preferences serialization/deserialization by jakebailey · Pull Request #2272 · microsoft/typescript-go

jakebailey · 2025-12-08T17:56:31Z

In working on improving the fourslash testing, I wanted to make user preferences serializable, such that we can directly go from JSON to prefs and back, and therefore also test out the "actual" JSON encoding.

Making that work sort of snowballed into a total refactor, which eliminates the error-prone switch case for decoding, replacing it with struct tags and reflect.

Prefs now look like this:

type CodeLensUserPreferences struct {
	ReferencesCodeLensEnabled                     bool `raw:"referencesCodeLensEnabled" config:"referencesCodeLens.enabled"`
	ImplementationsCodeLensEnabled                bool `raw:"implementationsCodeLensEnabled" config:"implementationsCodeLens.enabled"`
	ReferencesCodeLensShowOnAllFunctions          bool `raw:"referencesCodeLensShowOnAllFunctions" config:"referencesCodeLens.showOnAllFunctions"`
	ImplementationsCodeLensShowOnInterfaceMethods bool `raw:"implementationsCodeLensShowOnInterfaceMethods" config:"implementationsCodeLens.showOnInterfaceMethods"`
	ImplementationsCodeLensShowOnAllClassMethods  bool `raw:"implementationsCodeLensShowOnAllClassMethods" config:"implementationsCodeLens.showOnAllClassMethods"`
}

A struct tag gives the name for the JSON propery (used by Strada), and also the name over LSP. https://github.com/microsoft/vscode/blob/main/extensions/typescript-language-features/package.json.

All prefs can still be set by the unstable object (which happens first, based on the raw name).

Structs can be nested, e.g.:

type UserPreferences struct {
	// ...
	ModuleSpecifier ModuleSpecifierUserPreferences
	// ...
}

type ModuleSpecifierUserPreferences struct {
	ImportModuleSpecifierPreference modulespecifiers.ImportModuleSpecifierPreference `raw:"importModuleSpecifierPreference" config:"preferences.importModuleSpecifier"` // !!!
	// Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js"
	ImportModuleSpecifierEnding       modulespecifiers.ImportModuleSpecifierEndingPreference `raw:"importModuleSpecifierEnding" config:"preferences.importModuleSpecifierEnding"`             // !!!
	AutoImportSpecifierExcludeRegexes []string                                               `raw:"autoImportSpecifierExcludeRegexes" config:"preferences.autoImportSpecifierExcludeRegexes"` // !!!
}

And, for prefs that are actually defined elsewhere (like the above), conversion works so long as the structure is the same, making this less error prone:

func (p *UserPreferences) ModuleSpecifierPreferences() modulespecifiers.UserPreferences {
	return modulespecifiers.UserPreferences(p.ModuleSpecifier)
}

UserPreferences are also now considered to be immutable. They should never be mutated for any reason. Copy() can be used if you need to make a deep copy.

User prefs are also now value types; pointers to them are not passed around. (Slightly untrue given they contain slices, but, oh well).

Copilot

Pull request overview

This PR refactors the user preferences system to use reflection-based serialization/deserialization with struct tags, replacing the error-prone switch-case approach. The changes make preferences immutable, support direct JSON marshaling/unmarshaling, and simplify configuration by mapping TypeScript config paths to struct fields via pref tags.

Key changes:

Implements reflection-based parsing using pref struct tags to map VS Code config paths to fields
Adds custom type parsers and serializers for enum-like types (Tristate, OrganizeImportsCollation, etc.)
Makes preferences immutable by removing mutation operations and returning the same pointer where possible
Adds generic deepCopy implementation for creating copies when needed

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`internal/ls/lsutil/userpreferences.go`	Core refactor: adds struct tags, reflection-based parsing/serialization, type parsers/serializers, and immutable Copy() implementation
`internal/ls/lsutil/userpreferences_test.go`	New roundtrip test validating JSON marshaling/unmarshaling and parseWorker
`internal/project/snapshot.go`	Removes unnecessary Copy() call when assigning immutable preferences
`internal/project/session.go`	Removes Copy() calls and returns direct pointer to immutable preferences
`internal/lsp/server.go`	Updates to use ParseUserPreferences() function and DefaultUserPreferences; fixes typo
`internal/ls/autoimports.go`	Updates field access to use nested ModuleSpecifier struct
`internal/fourslash/fourslash.go`	Updates to use DefaultUserPreferences directly instead of calling constructor

…eference, etc

Copilot

Pull request overview

Copilot reviewed 106 out of 106 changed files in this pull request and generated 6 comments.

Copilot · 2026-04-03T23:14:31Z

+func ParseUserPreferences(items map[string]any) UserPreferences {
+	prefs := NewDefaultUserPreferences()
+	// Apply editor settings first (tabSize, indentSize, etc.) as raw-name defaults,
+	// then overlay js/ts settings which take precedence.
+	if editorItem, ok := items["editor"]; ok && editorItem != nil {
+		if editorSettings, ok := editorItem.(map[string]any); ok {
+			prefs = prefs.withConfig(map[string]any{"unstable": editorSettings})
+		}
+	}
+	if jsTsItem, ok := items["js/ts"]; ok && jsTsItem != nil {
+		switch jsTsSettings := jsTsItem.(type) {
+		case map[string]any:
+			prefs = prefs.withConfig(jsTsSettings)
+		case UserPreferences:
+			prefs = prefs.WithOverrides(jsTsSettings)
+		}
 	}
-	return false
+	return prefs


ParseUserPreferences only considers the editor and js/ts sections. Prior behavior accepted typescript and javascript sections as well; with this change, any client that still sends those sections (or only configures them) will silently drop preferences. Consider either merging typescript/javascript into js/ts for backward compatibility, or explicitly rejecting/logging when they’re present so misconfiguration is visible.

I did this on purpose, but could bring it back, I suppose.

Copilot · 2026-04-03T23:14:32Z

@@ -307,54 +307,40 @@ func (s *Server) RequestConfiguration(ctx context.Context) (*lsutil.UserConfig,
 				*s.initializeParams.InitializationOptions.UserPreferences,
 				*s.initializeParams.InitializationOptions.UserPreferences,
 			)
-			// Any options received via initializationOptions will be used for both `js` and `ts` options
 			if config, ok := (*s.initializeParams.InitializationOptions.UserPreferences).(map[string]any); ok {
-				return lsutil.NewUserConfig(lsutil.NewDefaultUserPreferences().ParseWorker(config)), nil
+				return lsutil.ParseUserPreferences(map[string]any{"js/ts": config}), nil
 			}
 		}
-		// if no configuration request capability, return default config
-		return lsutil.NewUserConfig(nil), nil
+		return lsutil.NewDefaultUserPreferences(), nil
 	}
 	configs, err := sendClientRequest(ctx, s, lsproto.WorkspaceConfigurationInfo, &lsproto.ConfigurationParams{
 		Items: []*lsproto.ConfigurationItem{
 			{
 				Section: new("js/ts"),
 			},
-			{
-				Section: new("typescript"),
-			},
-			{
-				Section: new("javascript"),
-			},
 			{
 				Section: new("editor"),
 			},
 		},


RequestConfiguration no longer requests the typescript and javascript sections, and the didChangeConfiguration registration also drops them. If any clients/extensions rely on those sections (or on different JS vs TS preferences), this becomes a breaking behavior change. If the intent is to drop support, consider a compatibility layer in ParseUserPreferences or a clear migration note / log when those sections are received.

Copilot · 2026-04-03T23:14:32Z

+// WithOverrides returns a copy of p with non-zero fields from overrides applied on top.
+// This is safe because all preference fields use types where zero = "not set":
+// Tristate (TSUnknown=0), int (0), string (""), slice (nil).
+func (p UserPreferences) WithOverrides(overrides UserPreferences) UserPreferences {
+	mergeNonZeroFields(reflect.ValueOf(&p).Elem(), reflect.ValueOf(&overrides).Elem())
+	return p
 }


There’s no longer a Copy()/deep-clone helper on UserPreferences, but multiple call sites and the PR description rely on preferences being treated as immutable despite containing slices. Without a deep-copy helper, it’s hard to safely snapshot or expose preferences without risking shared-slice mutation. Consider adding an explicit deep-copy method (at least cloning slice fields) and using it where a defensive copy is expected (e.g. Session.Config).

Yes, though I don't think it matters; I don't think anyone should be mutating the slices themselves.

Yes, once parsed, there shouldn't be any mutating. The one potential case is if you are parsing into an existing (non default) userpreferences object-- in that case you'd need to parse into a copy

# Conflicts: # internal/ls/lsutil/userpreferences.go # testdata/baselines/reference/fourslash/state/codeLensAcrossProjects.baseline

jakebailey · 2026-04-08T21:37:44Z

Any opinions on whether or not the typescript/javascript fallback is preserved? Not hard to stick back in.

jakebailey · 2026-04-08T21:45:58Z

I'm going to restore it, it wasn't really the main point of this change but something I did in the middle of the refactor.

jakebailey added 12 commits December 5, 2025 11:01

Make user prefs serializable

c4286f5

Very redo

bca6d8f

Deep copy

1deb813

Another struct

1763af6

Simpler

86b1d94

Simpler

bf21cb2

Remove copying

88a928f

more unused copies

67db3e9

Restore comments

18be102

Merge branch 'main' into jabaile/user-prefs-json

820c516

Default just pointer

7ff185e

Remove

0c95a82

jakebailey requested review from DanielRosenwasser, andrewbranch, Copilot and iisaduan December 8, 2025 17:56

Copilot started reviewing on behalf of jakebailey December 8, 2025 17:57 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

Cleanup

8b08250

DanielRosenwasser reviewed Dec 9, 2025

View reviewed changes

jakebailey added 8 commits December 8, 2025 19:54

remove old dead test code

46cf5f1

Just return

711f302

Delete alias support

6c703eb

Explict unstable

4728fba

Merge branch 'main' into jabaile/user-prefs-json

9c38c4e

test, what

8ae6651

Move stuff around but I don't know how to improve quoteStyle, quotePr…

74c815b

…eference, etc

Try and deal with old vs new

2b4b1f6

jakebailey mentioned this pull request Dec 9, 2025

Fix getReferencedSymbolsForSymbol, implement getReferencesAtExportSpecifier #2304

Merged

DanielRosenwasser reviewed Dec 9, 2025

View reviewed changes

Comment thread internal/ls/lsutil/userpreferences.go

jakebailey added 13 commits March 30, 2026 21:00

Remove UserConfig

f834683

WithOverrides

a73091e

No pointer

4508850

WithEditorSettings

e8e0f3e

ParseUserPreferences fix

e4c7727

More value-ing

e100c0a

unexport

b18591d

cleanup

f3327e7

modify things further

e64ce36

Further value it

8ccf6c2

Values for formatting

730375d

Delete default global

2bd4cbd

Fix GetFormatCodeSettingsFromContext

c95df25

jakebailey requested review from DanielRosenwasser and Copilot April 3, 2026 23:07

Copilot started reviewing on behalf of jakebailey April 3, 2026 23:07 View session

jakebailey marked this pull request as ready for review April 3, 2026 23:12

Copilot AI reviewed Apr 3, 2026

View reviewed changes

jakebailey added 2 commits April 3, 2026 16:46

PR feedback

b80bc96

Merge branch 'main' into jabaile/user-prefs-json

e30efce

# Conflicts: # internal/ls/lsutil/userpreferences.go # testdata/baselines/reference/fourslash/state/codeLensAcrossProjects.baseline

andrewbranch approved these changes Apr 8, 2026

View reviewed changes

Restore typescript/javascript reading

1ebd161

jakebailey requested a review from andrewbranch April 8, 2026 21:46

andrewbranch approved these changes Apr 9, 2026

View reviewed changes

jakebailey added this pull request to the merge queue Apr 9, 2026

Merged via the queue into main with commit 2050f96 Apr 9, 2026
21 checks passed

jakebailey deleted the jabaile/user-prefs-json branch April 9, 2026 17:13

Conversation

jakebailey commented Dec 8, 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

jakebailey Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

jakebailey Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

iisaduan Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

jakebailey commented Apr 8, 2026

Uh oh!

jakebailey commented Apr 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jakebailey commented Dec 8, 2025 •

edited

Loading