Skip to content

Fix duplicate diagnostics caused by DidChange handler #1079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 31, 2019

Conversation

TylerLeonhardt
Copy link
Member

I discovered that duplicate diagnostics were being sent to vscode because we iterated through the ContentChanges and asked for diagnostics in that loop, even though, the changes pertained to the same file.

You can see the bug if you type really fast or use like the ## comment generator and stuff like that because it will include multiple ContentChanges to the same file in 1 textDocument/DidChange event.

This properly runs diagnostics after all code changes are applied.

Discovered this originally in #1078 (which has a similar change in it as well... thought it might affect us today and sure enough, it did!)

comes with a test too!

@TylerLeonhardt TylerLeonhardt changed the title Fix dup diagnostics Fix duplicate diagnostics caused by DidChange handler Oct 31, 2019
changedFile.ApplyChange(
GetFileChangeDetails(
textChange.Range,
textChange.Text));

changedFiles.Add(changedFile);
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wow 🙄

@@ -145,6 +145,43 @@ public async Task CanReceiveDiagnosticsFromFileOpen()
Assert.Equal("PSUseDeclaredVarsMoreThanAssignments", diagnostic.Code);
}

[Fact]
public async Task CanReceiveDiagnosticsFromFileChanged()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@rjmholt rjmholt merged commit 2a7e8b6 into PowerShell:master Oct 31, 2019
@TylerLeonhardt TylerLeonhardt deleted the fix-dup-diagnostics branch October 31, 2019 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants