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

Store diagnostics in memdb table #856

Closed
dbanck opened this issue Apr 7, 2022 · 4 comments
Closed

Store diagnostics in memdb table #856

dbanck opened this issue Apr 7, 2022 · 4 comments

Comments

@dbanck
Copy link
Member

dbanck commented Apr 7, 2022

This is part of hashicorp/vscode-terraform#635

Problem Statement

Currently we're sourcing and publishing diagnostics in two different places:

As result of parsing files

diags := diagnostics.NewDiagnostics()
diags.EmptyRootDiagnostic()
defer notifier.PublishHCLDiags(ctx, newMod.Path, diags)
if newMod != nil {
diags.Append("HCL", newMod.ModuleDiagnostics.AsMap())
diags.Append("HCL", newMod.VarsDiagnostics.AutoloadedOnly().AsMap())
}

terraform validate

diags := diagnostics.NewDiagnostics()
validateDiags := diagnostics.HCLDiagsFromJSON(jsonDiags)
diags.EmptyRootDiagnostic()
diags.Append("terraform validate", validateDiags)
diags.Append("HCL", mod.ModuleDiagnostics.AsMap())
diags.Append("HCL", mod.VarsDiagnostics.AutoloadedOnly().AsMap())

This can lead to stale or suddenly disappearing diagnostics, for example when validate is executed as an editor command.

Requirements Summary

  • Central storage for diagnostics
  • Different sources feed diagnostics for a single or multiple files
  • Publishing diagnostics will always return all available diagnostics
  • Diagnostics need to be refreshed, independent of each other, and not impact or clear other sources

Proposal

  • Create a new memdb table for diagnostics based on the following struct
type Diagnostic struct {
   Dir      string
   Filename string
   Range    hcl.Range
   Severity hcl.DiagnosticSeverity
 
   // The diagnostic's code, which might appear in the user interface
   // e.g. "terraform_deprecated_interpolation"
   Code            string
   // An optional property to describe the error code
   CodeDescription string
 
   // A human-readable string describing the source, e.g. "tflint"
   Source  string
   // The diagnostic's message.
   // e.g. "Interpolation-only expressions are deprecated"
   Message string
 
   // Optional data entry field
   Data interface{}
}
  • Store HCL diagnostics in the table
  • Store validate diagnostics in the table
  • Query diagnostics table when publishing diagnostics

Related LSP methods

Upcoming diagnostics sources

Open Questions

  • Could memdb "watchers" be used to publish diagnostics?
@radeksimko
Copy link
Member

@jpogran & @dbanck will take a look if we could avoid storing diagnostics in memdb entirely

James expects the LSP spec to differ from the reality in VS Code and/or other clients.

@radeksimko
Copy link
Member

I have not looked into gopls codebase for the implementation and current state, but at least last year in 2021 they stored diagnostics per files according to this microsoft/language-server-protocol#1217

@dbanck
Copy link
Member Author

dbanck commented Jul 12, 2024

It's no longer needed due to #1667 and the diagnostic source map we introduced as part of the enhanced validation work.

@dbanck dbanck closed this as completed Jul 12, 2024
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants