TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

QuickFix: `Fix all detected spelling errors` not working

Open justschen opened this issue 2 years ago • 1 comments

TS Template added by @mjbvz

TypeScript Version: 4.8.0-dev.20220809

Search Terms

  • getCombinedCodeFix
  • code action
  • quick fix

Does this issue occur when all extensions are disabled?: Yes/No

  • VS Code Version:
  • OS Version:

Steps to Reproduce:

  1. Some kind of spelling error that hasn't been corrected before (for example, on startup of VS Code with the error)
  2. Hover over and click Quick Fix -> Fix all detected spelling errors
  3. May have to do with https://github.com/microsoft/vscode/issues/157119

Recording 2022-08-09 at 11 57 06

From the debug console: image 24

justschen avatar Aug 09 '22 18:08 justschen

Simple repo:

export function newFunction() {}

newFunctions(); // trigger here

Here's the relevant request:

[Trace  - 23:24:46.136] <semantic> Sending request: getCombinedCodeFix (97). Response expected: yes. Current queue length: 0
Arguments: {
    "scope": {
        "type": "file",
        "args": {
            "file": "/Users/matb/projects/san/y.ts"
        }
    },
    "fixId": "fixSpelling"
}
[Trace  - 23:24:46.138] <semantic> Response received: getCombinedCodeFix (97). Request took 2 ms. Success: false . Message: Error processing request. Debug Failure. False expression: Changes overlap
Verbose Debug Information: {"pos":35,"end":47} and {"pos":35,"end":47}
Error: Debug Failure. False expression: Changes overlap
Verbose Debug Information: {"pos":35,"end":47} and {"pos":35,"end":47}
    at _loop_10 (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:151056:34)
    at /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:151062:25
    at Object.mapDefined (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:622:30)
    at Object.getTextChangesFromChanges (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:151050:27)
    at ChangeTracker.getChanges (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:150989:45)
    at ChangeTracker.with (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:150409:32)
    at Object.codeFixAll (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:151674:60)
    at Object.getAllCodeActions (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:155886:68)
    at Object.getAllFixes (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:151661:81)
    at Object.getCombinedCodeFix (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:167298:31)
    at Session.getCombinedCodeFix (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:179538:56)
    at Session.handlers.ts.Map.ts.getEntries._a.<computed> (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:177904:61)
    at /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:179831:96
    at Session.executeWithRequestId (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:179822:28)
    at Session.executeCommand (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:179831:41)
    at Session.onMessage (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:179859:35)
    at process.<anonymous> (/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/extensions/node_modules/typescript/lib/tsserver.js:184025:31)
    at process.emit (node:events:526:28)
    at emit (node:internal/child_process:938:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:84:21)

mjbvz avatar Aug 09 '22 23:08 mjbvz

To me the problem (you can see it in the GIF) is that there are duplicate errors. Because there are multiple error spans, we will try to do the same change twice (which is nonsensical).

image

@andrewbranch, were you investigating something related to duplicate errors caused by related spans?

DanielRosenwasser avatar Aug 15 '22 18:08 DanielRosenwasser

Yes, that must be the same problem 👀

I have no idea how we didn’t notice this sooner. I guess we need to change how we do related spans and/or deduplication. It kind of feels like the related span shouldn’t contribute to the equality of diagnostics in deduplication anyway—I can’t think of any time we would intentionally issue multiple errors at the same location with different related spans. Those would still feel like duplicates to me.

andrewbranch avatar Aug 15 '22 22:08 andrewbranch

Here’s an example that’s sort of valid. It doesn’t convince me that the duplication is desirable:

image

andrewbranch avatar Aug 15 '22 22:08 andrewbranch

Deeper root-cause analysis of the original bug here https://github.com/microsoft/TypeScript/issues/49437#issuecomment-1216028660

DanielRosenwasser avatar Aug 16 '22 01:08 DanielRosenwasser