Skip to content

Keep formatting fragments out of source-file statement lists - #65491

Open
Jake Bailey (jakebailey) wants to merge 1 commit into
microsoft:mainfrom
jakebailey:compiler-fixes/synthetic-formatting-roots
Open

Jake Bailey (jakebailey) wants to merge 1 commit into
microsoft:mainfrom
jakebailey:compiler-fixes/synthetic-formatting-roots

Conversation

@jakebailey

Copy link
Copy Markdown
Member

The old way is technically violating the type annotations, though they are not currently checked.

Copilot AI balanced review requested due to automatic review settings November 13, 2026 23:02
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Nov 13, 2026
@typescript-automation typescript-automation Bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Nov 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The shortened formatting range excludes emitted trailing comments, causing semicolons to remain despite SemicolonPreferenceRemove; the full printed-text range must be preserved and covered by a regression test.

1 open issue
What changed in this PR

This PR keeps printed formatting fragments separate from source-file statement lists, preserving valid AST structure in the compiler and language server.

Changes:

  • Create synthetic source files as text and line-map contexts, with empty statement lists.
  • Update formatting callers and assign parent pointers to all positioned children.
  • Add formatting tests for non-statement fragments.
File Description
tsc/​internal/​printer/​syntheticfile.go Remove fragments from synthetic source-file statements.
tsc/​internal/​printer/​changetrackerwriter.go Set parent pointers for every child.
tsc/​internal/​ls/​completions.go Update snippet formatting to use the revised helper.
tsc/​internal/​ls/​change/​trackerimpl.go Format positioned fragments separately from source-file context.
tsc/​internal/​format/​api_test.go Test fragment formatting and valid source-file children.
tsc/​internal/​api/​session.go Update insertion formatting to use the revised helper.

Review effort: Balanced

Comment thread tsc/internal/ls/change/trackerimpl.go Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

With an empty statement list, indentation lookups repeatedly scan from the fragment’s start, potentially causing quadratic formatting time for large functions or classes with many multiline branches.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Restrict preceding-token scans to the enclosing if node

tsc/​internal/​printer/​syntheticfile.go:43

With an empty statement list, the FindPrecedingToken call in format/indent.go:502 scans from the start of the fragment for each multiline if/else whose indentation needs this lookup. Repeated branches in a large function or class can therefore cause quadratic token scanning during formatting. Keep the text-only source file, but change that lookup to astnav.FindPrecedingTokenEx(sourceFile, child.Pos(), parent, false) so it searches within the enclosing if node.

1 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The review reports no active issues with the changes separating formatting fragments from source-file statement lists.

No open issues 🎉

Review effort: Balanced

Formatting accepts roots that are not statements, including class
members, case clauses, types, and import specifiers. Putting these roots
in a source file violates its statement-child contract.

Use the source file for text and line-map context while traversing the
positioned fragment separately. Preserve the full printed-text range
for trailing comments, and bound indentation token lookups to their
enclosing nodes so large fragments do not incur repeated prefix scans.
@jakebailey
Jake Bailey (jakebailey) force-pushed the compiler-fixes/synthetic-formatting-roots branch from fdda39f to 4f09010 Compare November 14, 2026 03:12

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

2 participants