Format options + fourslash testing + fix formatting bugs #2370

iisaduan · 2025-12-12T22:03:27Z

Adds formatting options and links up the server's FormatCodeOptions with the formatting functions. Also adds in basic structure for js, javascript, ts, and typescript scoped settings.

Moves formatCodeSettings to lsutil

Formatting is now being tested in fourslash, although not all the commands are implemented due to non-formatting tests crashing on insertion. I've added a fourslashTest property to skip the check for those specific crashes, so that the rest of the test can proceed as normal. The flag(s) can easily be commented out if investigating these crashes.

Also fixes a formatting bug that was adding extra spaces after newlines.

…o formatOptions

Copilot

Pull request overview

This PR adds formatting options support and integrates it with the language server, enables fourslash testing for formatting functionality, and fixes a formatting bug related to extra spaces after newlines.

Key changes:

Adds FormatCodeSettings structure with comprehensive formatting options
Refactors configuration management to support both TypeScript and JavaScript scoped settings
Fixes scanner bug that was calculating end-of-line positions incorrectly
Updates fourslash test infrastructure to support formatting tests
Reduces baseline diffs showing improved formatting convergence with TypeScript

Reviewed changes

Copilot reviewed 283 out of 283 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`internal/scanner/scanner.go`	Fixed off-by-one error in `GetECMAEndLinePosition` to correctly calculate line endings
`internal/project/configuration.go`	New configuration structure supporting separate TypeScript/JavaScript preferences
`internal/project/snapshot.go`	Refactored to use pointer-based Config and removed embedded format options
`internal/project/session.go`	Updated to manage Config objects instead of bare UserPreferences
`internal/ls/lsutil/formatcodeoptions.go`	New comprehensive formatting options with parsing and default values
`internal/ls/lsutil/userpreferences.go`	Integrated FormatCodeSettings into UserPreferences structure
`internal/ls/format.go`	Updated to use new format options from UserPreferences
`internal/lsp/server.go`	Enhanced configuration request to support multiple scopes (typescript/ts/javascript/js)
Test files	Multiple fourslash tests updated to use new verification methods
Baseline files	Reduced diff files showing formatting convergence improvements

internal/fourslash/fourslash.go

jakebailey · 2025-12-12T22:15:08Z

internal/fourslash/fourslash.go

+func (f *FourslashTest) SetFormatOption(t *testing.T, optionName string, value any) {
+	t.Helper()
+	newPreferences := f.userPreferences.Copy()
+	newPreferences.Set(optionName, value)


I don't particularly setting these as strings; can we generate these differently so we can stay static? My options refactor will remove any sort of string-based setting

jakebailey · 2025-12-12T22:15:22Z

internal/fourslash/fourslash.go

+			End:   currentCaretPosition,
+			Start: currentCaretPosition,


Seems like a random change

jakebailey · 2025-12-12T22:15:47Z

internal/fourslash/fourslash.go


 // Inserts the text currently at the caret position character by character, as if the user typed it.
 func (f *FourslashTest) typeText(t *testing.T, text string) {
+	// temprorary -- this disables tests failing if format crashes; this unblocks unrelated tests such as codefixes


Hm, I see; how much does this matter?

there's currently ~10 passing tests that this unblocks, and some more (~10?) in failingtests.txt that fail for other reasons besides the formatting crashes. There's more unparsed tests (they have commands like format.Range for example) that I didn't include due to trying to get this pr out faster, that also crash. These numbers are a lot lower after the formatting bugs that I've fixed+included in this PR. I'm very pro-keeping this until either all of autoimports and completions tests are passing or formatting bugs are more ironed out. It'll be a lot less annoying for anyone working on completions or autoimports to work on their features without the formatter doing unrelated things.

more info:
The current disabling only disables reporting crashes when calling format on keystroke, which in strada fourslash was called after every character passed into edit.insert. However, completions and autoimport/codefix tests don't usually use edit.insert to simulate keystrokes-- they use edit.insert to simulate the file result after accepting a completion or codefixaction, which in the real world, returns already formatted text, and doesn't format on keystroke. We also don't really lose any formatting test coverage because if the formatter crashes, the check on formatted text will be wrong

jakebailey · 2025-12-12T22:18:23Z

internal/lsp/server.go

 				Section: ptrTo("typescript"),
 			},
+			{
+				Section: ptrTo("ts"),


The actual section is ts/js, but we should just ignore those for now since we don't pull inferred project compiler options out of it yet.

jakebailey · 2025-12-12T22:26:24Z

internal/lsp/server.go

+	} else if settings, ok := params.Settings.([]any); ok {
+		s.session.Configure(project.ParseConfiguration(settings))
+	} else if settings, ok := params.Settings.(map[string]any); ok {
+		s.session.Configure(project.ParseConfiguration([]any{settings["typescript"], settings["ts"], settings["javascript"], settings["js"]}))


If we want to respond to this, we're going to have to also register these in handleInitialized

jakebailey · 2025-12-12T22:27:04Z

internal/project/configuration.go

+}
+
+// any non-nil field in b is copied into a
+func (a *Config) CopyInto(b *Config) *Config {


Do we need mutability for any of the prefs like this? I had thought the change I made before to make it all immutable worked pretty well

jakebailey · 2025-12-12T22:27:41Z

internal/project/session.go

 	s.configRWMu.Lock()
 	defer s.configRWMu.Unlock()
-	return s.userPreferences.Copy()
+	return s.workspaceConfig.Ts.Copy()


What is the plan for returning the "right" one?

If we aren't handling that yet, can we just skip doing the JS stuff for the time being?

jakebailey · 2025-12-12T22:28:59Z

internal/scanner/scanner.go

 		ch, size := utf8.DecodeRuneInString(sourceFile.Text()[pos:])
 		if size == 0 || stringutil.IsLineBreak(ch) {
-			return pos
+			return pos - 1


I feel suspicious of this; some compiler baselines have changed

I think the error message was using the wrong function. I will fix that

yup, the compiler error function was ported wrong, probably because this function was ported wrong

iisaduan added 7 commits December 2, 2025 12:40

format options implementation

ecddfc4

Merge branch 'main' of https://github.com/microsoft/typescript-go int…

1e5c2ea

…o formatOptions

merge

48bdad2

Merge branch 'main' of https://github.com/microsoft/typescript-go int…

4f60eeb

…o formatOptions

formatting

ca78105

Merge branch 'main' of https://github.com/microsoft/typescript-go int…

ea1ac02

…o formatOptions

updatefailing

c105023

iisaduan requested review from DanielRosenwasser, Copilot, gabritto and jakebailey December 12, 2025 22:03

Copilot started reviewing on behalf of iisaduan December 12, 2025 22:07 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

jakebailey reviewed Dec 12, 2025

View reviewed changes

fix broken error message and nil formatcodesettings pointer

ae84521

Format options + fourslash testing + fix formatting bugs #2370

Are you sure you want to change the base?

Format options + fourslash testing + fix formatting bugs #2370

Uh oh!

Conversation

iisaduan commented Dec 12, 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

iisaduan commented Dec 12, 2025 •

edited

Loading