Odd quick fix ordering for misspelled identifiers

[DanielRosenwasser]

I believe that "misspelled identifier" quick fixes should precede the "remove unused declarations".

If anyone wants to take this on, this should just be a matter of

1. Making a test case for this, and
2. Shifting the order of which quick fix is registered first.

The ordering of fixes is currently determined by the load order of files, since they each immediately alter global state the moment they're loaded. I think we should change these files to export an object instead of calling registerCodeFix, make registerCodeFix private, and have codeFixProvider import from all code fixes and register them in an order it chooses.

[jrkong]

I'd like to give this one a shot if no one's working on it. This is my first issue so I would really appreciate it if someone could direct me to the files I should look at to fix this.

[RyanCavanaugh]

jrkong you just have to change the ordering of the codefix files in the various tsconfig.json files under src

[jrkong]

Ryan Cavanaugh (@RyanCavanaugh) thanks a lot! That really helps, I'll do some digging.

[jrkong]

So after spending a bit of time with the code I've discovered some odd inconsistencies with the behavior for this particular bug:

When I attempt to reproduce the bug with the given line in the issue this is what I see:

However if I added the lines:

export {};
function f() {}

the bug is reproduced.

Daniel Rosenwasser (@DanielRosenwasser) could you verify if I've reproduced the bug properly?

Here's where things got really odd for me:

Looking at the src/services/tsconfig.json the spelling code fixes are imported on line 54 and the code fixes for removing imports and unused dependencies are imported on line 62.

So in theory Change spelling to 'assertNever' should be loaded before Remove import from './Utilities' and Delete all unused declarations. What's more, the tests seem to corroborate that the source code is loading the spelling codefixes first:
Here's a screenshot of the test I've been working on and how it currently fails.

I deliberately tried testing against the incorrect output to verify the behavior of the bug before I made any further changes but the tests seem to imply that Change spelling to 'assertNever' should already. So does anyone have any idea what's going on?

I looked at the tsserver logs and it looks like the editor is making two separate calls:

Info 83   [17:4:3.734] request:
    {"seq":12,"type":"request","command":"getCodeFixes","arguments":{"file":"/home/andy/sample/ts/src/a.ts","startLine":1,"startOffset":10,"endLine":1,"endOffset":16,"errorCodes":[2724]}}
Perf 84   [17:4:3.793] 12::getCodeFixes: elapsed time (in milliseconds) 58.5188
Info 85   [17:4:3.793] response:
    {"seq":0,"type":"response","command":"getCodeFixes","request_seq":12,"success":true,"body":[{"fixName":"spelling","description":"Change spelling to 'fooBar'","changes":[{"fileName":"/home/andy/sample/ts/src/a.ts","textChanges":[{"start":{"line":1,"offset":10},"end":{"line":1,"offset":16},"newText":"fooBar"}]}],"fixId":"fixSpelling","fixAllDescription":"Fix all detected spelling errors"}]}
Info 86   [17:4:3.794] request:
    {"seq":13,"type":"request","command":"getCodeFixes","arguments":{"file":"/home/andy/sample/ts/src/a.ts","startLine":1,"startOffset":1,"endLine":1,"endOffset":30,"errorCodes":[6133]}}
Perf 87   [17:4:3.795] 13::getCodeFixes: elapsed time (in milliseconds) 1.0016
Info 88   [17:4:3.795] response:
    {"seq":0,"type":"response","command":"getCodeFixes","request_seq":13,"success":true,"body":[{"fixName":"unusedIdentifier","description":"Remove import from './b'","changes":[{"fileName":"/home/andy/sample/ts/src/a.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":2,"offset":1},"newText":""}]}],"fixId":"unusedIdentifier_delete","fixAllDescription":"Delete all unused declarations"}]}

So the ordering of our results is irrelevant because we're only returning one result at a time.
Our API is designed to take a range and a list of error codes, so perhaps it should be the editor's responsibility to make one combined call when ranges overlap, and then we could order the results in tsserver. Another option would to be return fixes with some kind of priority, but that would be irrelevant if they aren't actually overlapping. Matt Bierner (@mjbvz)