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
proposal: x/tools/go/analysis: add tags or codes to diagnostics #34508
Comments
/cc @matloob @ianthehat |
I'm a bit wary of adding this to the analysis framework because it's specific to the LSP protocol. "unneccessary" and "deprecated" only come from specific analyses right? If all those analyses are in tools, maybe they can populate a field of a type defined in internal/lsp so we don't need to introduce a new API? Would that work? |
I was thinking about this before (we need the same thing for severity) and my conclusion was that severity should be per analyzer, not per result, which means gopls can add the tag based on the analyzer that generated the diagnostic rather than having to add it to the analysis framework. I think all analyzers can be consistent about their severity level (and if they can't it is easy to split an analyzer into a core that produces facts and multiple analyzers that use those facts to produce varying severity levels of diagnostic), and I think the same probably holds for tags. |
Oh that's even better! Rebecca. What do you think? Is that reasonable? |
Sounds good to me - thanks! |
Okay, I'll close this issue in favor of that solution then. |
Could we document what Ian said in #34508 (comment)? I have a few tools which are already analyzers, and some others which I have to convert still, and this is very relevant for some of them. I vaguely remembered this conversation, but it was hard to find it (or its result) because the godoc makes no mention or hint about analyzer severity. |
You mean to add a line to the go/analysis package documentation saying as such, or do you mean documenting it somewhere else? |
Yeah, I did mean |
Change https://golang.org/cl/230312 mentions this issue: |
@mvdan Added a paragraph to the go/analysis docs on golang.org/cl/230312. Want to take a look and see if that works for you? |
…ities Add to the go/analysis docs the suggestion made by Ian Cottrell that diagnostic severities should be classified by analyzer. Updates golang/go#34508 Change-Id: I9a75fb1400269ece32c9ca52afbf6c7975d3e205 Reviewed-on: https://go-review.googlesource.com/c/tools/+/230312 Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Ian Cottrell <iancottrell@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
Version 3.15 of the Language Server Protocol will be adding tags to the Diagnostic. These will allow us to specify types of diagnostics, in particular unnecessary and deprecated code. To avoid our having to parse diagnostic messages for
gopls
,go/analysis
should return these details along with diagnostics.The text was updated successfully, but these errors were encountered: