Apply same single-line style to copied nodes as manufactured ones

[weswigham]

I brought this up before, but not applying these emit styles to copied nodes in the node builder makes reusing more input nodes a fairly noisy activity in the test baselines (which might itself be a good thing, but not intentional), and, moreover, makes the types print poorly in error messages (with lots of tabs - .types baselines use basically the same print settings errors do). I'd like to take this chance to correct that and apply the node builder's "house style" to any nodes we copy (which is to say: SingleLine everything - except object literal types when the node builder flag to print them multiline is set).

[RyanCavanaugh]

Just commenting on the ones that aren't whitespace only for visiblity

[weswigham]

I think this is because our parenthesizer when we clone the node is stricter than the syntax allows. I'll have to check.

[weswigham]

Yeah, now that we make a new node to apply SingleLine to the {}, the node goes through the parenthesizer, and the relevant rule is

    // UnionType[Extends] :
    //     `|`? IntersectionType[?Extends]
    //     UnionType[?Extends] `|` IntersectionType[?Extends]
    //
    // - A union type constituent has the same precedence as the check type of a conditional type
    function parenthesizeConstituentTypeOfUnionType(type: TypeNode) {
        switch (type.kind) {
            case SyntaxKind.UnionType: // Not strictly necessary, but a union containing a union should have been flattened
            case SyntaxKind.IntersectionType: // Not strictly necessary, but makes generated output more readable and avoids breaks in DT tests
                return factory.createParenthesizedType(type);
        }
        return parenthesizeCheckTypeOfConditionalType(type);
    }

which acknowledges it's extraneous.

[jakebailey]

I do wonder if there's a still a reason to do it; the union case seems like overkill, and I recently changed ExpectType to normalize unions and could probably just make it always strip or add the parens.

[jakebailey]

I sent #57900 which shows the affect of this; I do sorta think we should stop doing this as I don't think that the extra parens are that much of an improvement?

But, on its own I think this particular change is alright.

[DanielRosenwasser]

Personally I found it a bit easier to read with the parens, so consider that a soft vote for keeping the parens.

I do wonder if there's a still a reason to do it; the union case seems like overkill

Are there cases of non-normalization to consider?

[jakebailey]

These are purely syntactic things we modify when synthetically creating nodes, so, no.

[jakebailey]

I feel like I want this, but the fact that we don't do this at the moment means that we can actually see node reuse changes in other PRs, so I'm sorta thinking that it'd be nice to see what a future reuse indicator might be... But I'm not sure what that will be yet.

[weswigham]

But I'm not sure what that will be yet.

The whitespace only worked as an indicator object literal types - if we want a similar restriction, we can instrument the printer to print {| ... |} and [| ... |] for object and tuple literal nodes that have non-synthetic positions and .original pointers, which'd be somewhat similar to the spans used in fourslash baselines. Another option would be... nearly doubling the size of the .types baselines to underline every preserved node? Or completely synthetic ones, whichever direction is preferred.

[weswigham]

@jakebailey I have a draft of a followup PR that explicitly underlines type nodes without positional mappings in the .types baselines, like so

var a14: (x: { a: string; b: number }) => Object;
>a14 : (x: { a: string; b: number; }) => Object
>    : ^^^^                         ^^^^^^^^^^^

that should do much nicer to actually track the desired information, without being too overbearing. The extra linebreaks, IMO, also make the type baselines a smidge more readable.

[weswigham]

+600k line diff, though 😄