Use getFileAndProject in session provideInlayHints to ensure language service updates are applied

[weswigham]

Fixes #45320

Credit to @DanielRosenwasser for the actual fix.

The explanation for why that fixes anything is a bit long winded, though. The issue, as described, appears like a heisenbug - you need to type really fast, and even then, sometimes everything still works fine. That's because, under normal circumstances, vscode does 3 things on document edit - request inlay hints, request encoded semantic classifications, and request diagnostics. The later two use getFileAndProject already. So what normally would happen is that you'd request inlay hints, see document updates, mark the LS as dirty, calculate inlay hints with the once-updated data, and then semantic classifications or diagnostics would see the dirty marker and flush the old LS and build a new one with the new data. The buggy behavior occurred when there was no semantic classification or diagnostic request to trigger the flush of the old LS - instead, if we got an inlay hint request followed by an update, followed by another inlay hint request, the second inlay hint request would not cause an LS synchronization to occur, and would instead reuse the LS from the first request verbatim. That then causes the positions returned by the LS to drift as the new lineMap we map positions with doesn't align with the text still in the old LS, causing the positions to drift until a LS synchronizing protocol request occurs. This all occurs very rarely because it's actually pretty hard to get a inlayHints -> updateOpen -> inlayHints sequence to occur - to cause the editor to issue such a command sequence you need to be typing fast enough that it bails on sending the diagnostics and semantic classifications requests, and also slow enough that it doesn't skip sending the inlayHints request. So there's an awkward sweet-spot of typing speed matched up against editor responsiveness that was required to trigger this bug.

[Kingwl]

Thanks for the explanation.

[DanielRosenwasser]

The buggy behavior occurred when there was no semantic classification or diagnostic request to trigger the flush of the old LS - instead, if we got an inlay hint request followed by an update, followed by another inlay hint request, the second inlay hint request would not cause an LS synchronization to occur, and would instead reuse the LS from the first request verbatim.

There's something I still don't totally understand. Given the scenario where the editor

1. requests inlay hints
2. sends updates
3. requests inlay hints
4. requests errors

I understand that why the inlay hints might get skewed, but the entire text state for errors was off too - in a way that would imply the whole file was corrupted. So:

1. Why didn't geterr immediately force the old LS out?
2. What is it about a subsequent updateOpen (and maybe some other semantic operations) that enabled us to recover?

Every time I think that I understand what's going wrong, there's other architectural questions I have. For example, maybe it's not that the document itself is corrupted, it's just the line maps. I can understand that, but if it's just the line maps, why does that affect the checker's view of the file content (as opposed to the ranges on which it's reporting)? I can imagine that the line maps are used to construct the document for the LS, but then arguably the document text must be inaccurate in a way that's hard to recover from.

Credit to @DanielRosenwasser for the actual fix.

It wasn't until @sheetalkamat left her comment over at #45320 (comment) that the problem jumped out at me. The credit really goes to her!

[DanielRosenwasser]

This looks great. The new test fails without the change and now passes. Thanks for digging into it!

[DanielRosenwasser]

@typescript-bot cherry-pick this to release-4.4

[typescript-bot]

Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into release-4.4 on this PR at 61dd78f. You can monitor the build here.

[typescript-bot]

Hey @DanielRosenwasser, I've opened #45396 for you.

[sandersn]

So, in this more likely in

• a really large file?
• a really large project?
• an edit that triggers a really large check?
• a really slow machine?

I'm trying to figure out why I was able to repro on my desktop but not laptop.

[weswigham]

It only happens when your typing rate almost precisely matches the rate at which the server processes the inlay hints command (on my machine, about 22ms thus around 45 characters per second). Larger documents and slower machines mean inlay hints takes longer, and thus slower typing will trigger the issue.

[orta]

@typescript-bot pack this

[typescript-bot]

Heya @orta, I've started to run the tarball bundle task on this PR at 61dd78f. You can monitor the build here.

[typescript-bot]

Hey @orta, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/108478/artifacts?artifactName=tgz&fileId=E787A91802A4F4E14210CDE62264ACA16857AC4EBDDE7A0F9531075E0351299D02&fileName=/typescript-4.5.0-insiders.20210810.tgz"
    }
}

and then running npm install.

---

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

[andreialecu]

Just wanted to add a confirmation that there's a huge improvement from this PR.

Here's a before:
https://user-images.githubusercontent.com/697707/128908402-d34c2c8d-1b60-49b0-904a-dbd802920085.mov

And after:
https://user-images.githubusercontent.com/697707/128908325-0c07ff58-f80d-4317-9272-499881f44f9e.mov

@weswigham you mentioned there would be no performance improvement (besides fixing the wrong positions they land on), but in the "after" video the squigglies stick to code and don't jump around as much. I wonder why that is.