fix(45501): split insertSpaceAfterOpeningAndBeforeClosingEmptyBraces to insertSpaceInEmptyObjects/insertSpaceInEmptyBlocks

[a-tarasyuk]

Fixes #45501
Fixes #45682

[typescript-bot]

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary

[a-tarasyuk]

\cc @andrewbranch

[andrewbranch]

Perfect, thanks! I think I’d like to hold merging this until we have time to get corresponding VS and VS Code PRs up so it doesn’t slip through the cracks.

[a-tarasyuk]

@andrewbranch Sure. I've created a PR to clean up my TODO :).

[sandersn]

@andrewbranch I lost track of this. Did we ever get PRs out to VS and VS Code to match this?

[andrewbranch]

Not that I know of.

[andrewbranch]

Coming back to this, I’ve noticed some issues, and fixing them requires thinking about the nodes with curly braces that are neither strictly objects nor strictly blocks. I think I would propose a classification like this:

Block-like:

• Block
• ModuleBlock
• CaseBlock

Object-like: everything else

• object literals
• type literals
• interface bodies
• class bodies
• named imports/exports

What do you think?

[andrewbranch]

The current status quo is more like

• object type literals get no space (not configurable)
• object literals get no space (not configurable)
• every other pair of empty braces is configurable with the mega-setting, defaulting to having a space

???

[andrewbranch]

I think the underlying aesthetic rationale, insofar as there is one, is/was for things that look like they got pulled straight out of C# to be consistent in having a space.

// Neither SyntaxKind.Block nor very JS-objecty
enum E { }
class C { }
interface I { }
module M { } // SyntaxKind.ModuleBlock but not sure why
switch (true) { } // SyntaxKind.CaseBlock but again ??

// Actually SyntaxKind.Block
{ }
if (true) { }
function f() { }
const x = () => { }

// JS object and direct type equivalent?
// AFAICT these are the only things that don't get a space
// and can't be configured.
const o: {} = {}

// Binding patterns also feel like they should
// behave the same as object literals, this default
// may be a mistake, though the syntax is truly useless
const { } = o

// Unclear where these fit, but to me they feel similar
// to shorthand object literals and binding patterns. The
// defaults include a space though.
import { } from "typescript"
export { }

I can understand the desire to have the top 4 examples have the same default and be controlled by the same setting, though I think 4 and 5 are a closer fit with the second group.

So now I’m not sure how to group all these into a reasonable number of options or what to name those options.

[jakebailey]

Giving my +1 for the two categories of block versus object, where enums/class/etc are block-like.

As for binding patterns, I'd say that it's object-y. For import/export, they also seem object-y too (as they are more binding patterns, sort of).

[jakebailey]

I'd also make the assertion that if people are getting particular about the formatting of their code, it sounds like they should be using prettier, dprint, etc, rather than whatever the formatting is that a particular editor defaults to.

[andrewbranch]

Why do you consider enums and classes block-like? They declare actual object-like values and their bodies are lists of members, not statements. How about interfaces? Do you think it’s weird for interfaces to be grouped apart from object type literals?

[jakebailey]

For both, I can reduce it down to the notion that their contents look more like declarations or assignments (or potentially worded as "things that feel legal inside of a class body").

enum Foo {
    X,
    Y = 1234,
    Z
}

interface Bar {
    x: number;
    someMethod(): void;
}

Whereas, the things in my object-y category use colons to re-bind from one name to another, or no colons at all.

[jakebailey]

The outlier of course is object types, which I stuck into the object category even though they're more like interface without the keyword in contexts where we don't need to have the keyword interface, except that they are no longer their own declaration and wouldn't be placed directly in any scope with other declarations (so "feel" more like objects to me).

[sandersn]

@andrewbranch @jakebailey do you want to pick this back up? Is it worth hashing through the design or should we close it?

[jakebailey]

My feeling about the formatter in general hasn't really changed, in that I think that if people care about these details they probably should be using prettier, dprint, etc, which aim to be opinionated enough to enforce one style. It's not very clear to me that the demand is there for our formatter to do anything more than leave what the user wrote in this particular case; this is just more knobs to maintain.

It seems to me like there are still unanswered questions about where we should stick spaces, what we consider to be a block vs not (especially considering interfaces, enums, object literals which all look different), so I don't think this PR would be going in as-is, at least, but I'm sure it could be mended.

But, @andrewbranch may have a different opinion here.

[sandersn]

OK. I'll close it for now and @andrewbranch (or anybody on the team) can re-open if they want.