Skip to content

[LSP] Switch to project version caching instead of sln crawler events pull diagnostics#57380

Merged
dibarbet merged 6 commits intodotnet:mainfrom
dibarbet:lsp_diagnostics_caching
Oct 28, 2021
Merged

[LSP] Switch to project version caching instead of sln crawler events pull diagnostics#57380
dibarbet merged 6 commits intodotnet:mainfrom
dibarbet:lsp_diagnostics_caching

Conversation

@dibarbet
Copy link
Copy Markdown
Member

This changes us from listening to diagnostic service events to using project versions to determine when to re-calculate diagnostics. This is helpful as we do not necessarily have access to the sln crawler outside of VS.

@ghost ghost added the Area-IDE label Oct 26, 2021
// Second doc should show up as unchanged.
Assert.Null(results2[1].Diagnostics);
Assert.Equal(results[1].ResultId, results2[1].ResultId);
// Second doc should be changed as the project has changed.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, diagnostic change events only told us changes to the actual diagnostics. So even if the diagnostics had to be recalculated, LSP wouldn't know unless the diagnostics actually changed. Now that we're looking at project version, we will return updated diagnostics even if the end result of the calculation is the same.

@dibarbet dibarbet changed the base branch from main to release/dev17.1 October 27, 2021 01:08
@dibarbet dibarbet changed the base branch from release/dev17.1 to main October 27, 2021 01:08
@dibarbet dibarbet changed the base branch from main to release/dev17.1 October 27, 2021 19:28
@dibarbet dibarbet changed the base branch from release/dev17.1 to main October 27, 2021 19:28
@dibarbet dibarbet force-pushed the lsp_diagnostics_caching branch from 1dadef2 to 6d9e48f Compare October 27, 2021 20:35
// Note that these checksums should only actually be calculated once, if the project is unchanged
// the same checksum will be returned.
var referencedProjectChecksum = await referencedProject.State.GetChecksumAsync(cancellationToken).ConfigureAwait(false);
tempChecksumArray.Add(referencedProjectChecksum);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we likely need to do this recursively right? or, if not, please document why not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, thinking now that definitely needs to happen. Will also add a test for that scenario

@dibarbet dibarbet marked this pull request as ready for review October 28, 2021 18:41
@dibarbet dibarbet requested a review from a team as a code owner October 28, 2021 18:41
@dibarbet dibarbet merged commit 7fe2842 into dotnet:main Oct 28, 2021
@ghost ghost added this to the Next milestone Oct 28, 2021
@dibarbet dibarbet deleted the lsp_diagnostics_caching branch October 28, 2021 22:00
@allisonchou allisonchou modified the milestones: Next, 17.1.P2 Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants