"Add all missing imports" adds them all in one line

[mjbvz]

TypeScript Version: 3.9.0-dev.20200420

Search Terms:

• fix all
• code action
• quick fix

Code
With @types/node installed, open a file:

join();
homedir()

1. Now trigger quick fixes and select add all missing imports

Bug:
The two imports are added on the same line:

import { join } from "path";import { homedir } from "os";

join();
homedir()

Here's the edit that TS Server returns in this case

[Trace  - 23:52:04.985] <semantic> Response received: getCombinedCodeFix (72). Request took 13 ms. Success: true 
Result: {
    "changes": [
        {
            "fileName": "/Users/matb/projects/san/test.ts",
            "textChanges": [
                {
                    "start": {
                        "line": 1,
                        "offset": 1
                    },
                    "end": {
                        "line": 1,
                        "offset": 1
                    },
                    "newText": "import { join } from \"path\";"
                },
                {
                    "start": {
                        "line": 1,
                        "offset": 1
                    },
                    "end": {
                        "line": 1,
                        "offset": 1
                    },
                    "newText": "import { homedir } from \"os\";\n"
                }
            ]
        }
    ]
}

[DanielRosenwasser]

Likely a culprit of the new "preserved trivia" strategy - generated nodes don't have trivia, so we're not printing out new lines between statements. See some instances of this in #37814

[andrewbranch]

It would be helpful if I could get a review on #37846

[DanielRosenwasser]

I gave a reivew, but if I understand correctly I think that only solves problems for gathering newlines between nodes more appropriately - it doesn't fix the problem for nodes that have no newlines because they're generated.

[andrewbranch]

I think you’re probably right, but it will be useful nonetheless to avoid making simultaneous PRs to the same area. Thanks!

[andrewbranch]

@mjbvz, it doesn’t seem to repro for me—also using that nightly and Insiders. Now that I think about it, we definitely already have test coverage for fix-all imports that should catch this. My screen cap shows semicolons on both calls, but I tried it with mixed and semicolon-less styles too. Any tricks you can think of to reproduce this?

[mjbvz]

To repo for me, start with:

import fs = require('fs')

join();
homedir()

Delete the import. Then trigger the add all missing imports quick fix

[andrewbranch]

Thanks! The trick was having a newline at the start of the file. This turned out not to be related to the newline preservation changes after all.