Skip to content
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

Fix sticky diagnostics #1188

Merged
merged 5 commits into from
Jan 10, 2021
Merged

Fix sticky diagnostics #1188

merged 5 commits into from
Jan 10, 2021

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Jan 10, 2021

In haskell/ghcide#959 I tried to fix a problem with diagnostics:

  • PROBLEM: when early cut-off kicks in and a stage is skipped (e.g. TypeCheck) its diagnostics are lost
  • SOLUTION: when creating diagnostics for a new version of a document, start from the diagnostics for the previous version.

The assumption in the solution is that starting with the previous diagnostics is sound because they will get updated incrementally as the rules are re-run. If a rule is not re-run then it must have been cut-off and its diagnostics are still valid.

But hold on, the "build" graph is dynamic, so the rules to be run for a file can change at any time! So it's not always the case that if a rule is not re-run, its diagnostics are still valid. For instance, the graph for a non-FOI changes when it becomes a FOI, e.g. GetModIfaceFromDisk is not re-run, so its diagnostics will stick around until the file becomes a non-FOI again, and GetModIfaceFromDisk is re-run.

The right solution is to store the diagnostics in our state (the values ref):

  • when creating diagnostics for a new version, just start from scratch as we used to
  • when early cut-off kicks in, reuse the diagnostics stored during the previous run of the rule
  • graph changes are not a problem, since only the diagnostics from the rules that actually run are considered

Fixes #1171

@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Jan 10, 2021
@mergify mergify bot merged commit 63218d1 into master Jan 10, 2021
fendor pushed a commit to fendor/haskell-language-server that referenced this pull request Jan 11, 2021
* Add a test for haskell#1171

* Refactor: extract more types from Core.Shake

* Store diagnostics in values ref (fixes haskell#1171)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sticky diagnostics
2 participants