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

gopls: support a setting to control the frequency of diagnostics #50

Open
stamblerre opened this issue May 15, 2020 · 31 comments
Open

gopls: support a setting to control the frequency of diagnostics #50

stamblerre opened this issue May 15, 2020 · 31 comments
Labels
gopls/analysis issues related to diagnostics and analysis done by gopls NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@stamblerre
Copy link
Contributor

stamblerre commented May 15, 2020

It's possible that some users might like to see diagnostics only when they save a file, rather than seeing them as they type.
It is also possible that some users may want to use gopls without seeing any diagnostics at all.

Depending on the popularity of these use cases, we should consider supporting these two configurations.

Please react with a 🚀 if you would like to see diagnostics only when you save a file, not as you type.
Please react with a ❤️ if you would like to disable diagnostics from gopls completely.

Original discussion on microsoft/vscode-go#2907.

@stamblerre stamblerre added the NeedsFix The path to resolution is known, but the work has not been done. label May 15, 2020
@hyangah hyangah added this to the Gopls by default milestone Aug 21, 2020
@hyangah hyangah added this to To Triage in vscode-go: gopls by default Sep 17, 2020
@stamblerre stamblerre moved this from Needs Triage to Non-critical in vscode-go: gopls by default Nov 10, 2020
@hyangah hyangah removed this from the Gopls by default milestone Nov 20, 2020
@stamblerre stamblerre changed the title gopls: support a setting to show diagnostics only on-save instead of on-type gopls: support a setting to control the frequency of diagnostics Jan 7, 2021
@stamblerre stamblerre removed this from Non-critical in vscode-go: gopls by default Jan 7, 2021
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/280601 mentions this issue: src/goLanguageServer: remove languageServerExperimentalFeatures

gopherbot pushed a commit that referenced this issue Jan 11, 2021
This setting is no longer necessary, especially since we're still
undecided about the future of diagnostic configurability. Remove the
configuration and notify the user that it's deprecated.

Updates #50

Change-Id: I273187ebaa9e582f02dc84dbeafa329c28e4fed7
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/280601
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
@tamayika
Copy link
Contributor

tamayika commented Jan 21, 2021

Is there any way to use custom go vet tool with gopls enabled in >= 0.21?
Or is there any way to embed custom analysis to gopls?

Here is my configuration.

{
  "go.vetFlags": [
    "--vettool=${workspaceRoot}/bin/my-vet-tool"
  ],
  "go.languageServerExperimentalFeatures": {
    "diagnostics": false, // deprecated and not work >= 0.21
  },
}

Current workaround is to rollback vscode-go to 0.20.x.

@hyangah
Copy link
Contributor

hyangah commented Jan 21, 2021

@tamayika Thanks for the report. Sorry that I didn't know users depend on this feature to use custom vet analysis. Currently gopls doesn't allow custom analysis. @stamblerre This affects the current language server users. I will prepare a partial rollback of cl/280601 that still leaves the diagnostics flag.

Turning completely off gopls's diagnostics because of the lack of custom analyzer support in gopls isn't ideal. I think we need a way to allow vet/buildOnSave when the language server is running (so filed #1109 for investigation).

Out of curiosity: @tamayika is there a reason that your analyzer can't run as a linter?

@tamayika
Copy link
Contributor

Thanks for the support.

go.lintTool is already used by other lint tool(revive) in my configuration.
So if go.lintTool supports multiple linters and go vet command with custom vet tool, it may solve the problem.

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/285253 mentions this issue: src/goLanguageServer: partial revert of cl/280601

gopherbot pushed a commit that referenced this issue Jan 21, 2021
This partially reverts commit 4c91c38.

In https://go-review.googlesource.com/c/vscode-go/+/280601,
we tried to remove the languageServerExperimentalFeatures setting
because gopls's diagnostics feature is no longer in its experimental
state and that was the only flag left in this setting.

However, we learned some users depend on this flag because the
extension turns off buildOnSave and vetOnSave features when gopls's
diagnostics is used and they need to run custom vet analyzers.
This is not ideal and the extension shouldn't prevent users from
running their custom analyzers. That needs more investigation and
experiment.

For now, we rollback the change, but place the deprecation notice.

Update #50
Fixes #1110

Change-Id: I376692b152d3011aaa8da7a1b5121ba33e2188b6
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/285253
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/285256 mentions this issue: [release] src/goLanguageServer: partial revert of cl/280601

gopherbot pushed a commit that referenced this issue Jan 21, 2021
This partially reverts commit 4c91c38.

In https://go-review.googlesource.com/c/vscode-go/+/280601,
we tried to remove the languageServerExperimentalFeatures setting
because gopls's diagnostics feature is no longer in its experimental
state and that was the only flag left in this setting.

However, we learned some users depend on this flag because the
extension turns off buildOnSave and vetOnSave features when gopls's
diagnostics is used and they need to run custom vet analyzers.
This is not ideal and the extension shouldn't prevent users from
running their custom analyzers. That needs more investigation and
experiment.

For now, we rollback the change, but place the deprecation notice.

Update #50
Fixes #1110

Change-Id: I376692b152d3011aaa8da7a1b5121ba33e2188b6
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/285253
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
(cherry picked from commit fbd2fc4)
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/285256
@hyangah
Copy link
Contributor

hyangah commented Jan 21, 2021

@tamayika thanks for understanding. Released v0.21.1 and that will still accept the flag. We hope to address your use case in #1109, separate from this issue.

@tamayika
Copy link
Contributor

Thanks for the release. I confirmed it works now with my configuration.

@mrjrieke

This comment has been minimized.

@stamblerre

This comment has been minimized.

@mrjrieke

This comment has been minimized.

@hyangah
Copy link
Contributor

hyangah commented Feb 14, 2021

Discussion around textDocument/getDiagnostics looks relevant.
The proposed API for pullDiagnostics is currently available in vscode language client

@alok87

This comment has been minimized.

@alok87

This comment has been minimized.

@hyangah

This comment has been minimized.

@alok87

This comment has been minimized.

@alok87

This comment has been minimized.

@belak
Copy link

belak commented May 26, 2022

I ran across this in our Go setup doc a while back...

  "go.languageServerExperimentalFeatures": {
    // Disable diagnostics, because they'll run on the entire repo. With them
    // disabled, go vet will run on just the package.
    "diagnostics": false
  },

I assume if that comment is true that there will be a huge difference in performance with monorepos.

@hyangah
Copy link
Contributor

hyangah commented Jun 7, 2022

@belak For mono-repo, build.directoryFIlters can be used to make gopls scale. (not only diagnostics, but more), and if the mono-repo consists of multiple go modules, gopls won't diagnose modules in subdirectory unless users explicitly add them in go.work file or enables the experimental workspace module mode.

@hyangah hyangah added the gopls/analysis issues related to diagnostics and analysis done by gopls label Jun 7, 2022
@hyangah hyangah added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Jun 7, 2022
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/501199 mentions this issue: package.json: remove go.languageServerExperimentalFeatures setting

gopherbot pushed a commit that referenced this issue Jun 8, 2023
The deprecation note was present for over two years. No objection
was raised.

Updates #1109
Updates #50

Change-Id: Ia78b894a200f4120c76fb473ec84ae39578f215f
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/501199
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@yayiji
Copy link

yayiji commented Jun 26, 2023

Why did you remove this setting?
Are there any other options I can use to disable the diagnostics?

@hyangah
Copy link
Contributor

hyangah commented Jun 26, 2023

@yayiji Can you tell us more why you want to disable gopls's diagnostics? (which includes go build, vet error reporting)

Over the three years, we left this issue open to hear from users. We heard two reasons that made some users disable the default gopls diagnostics and we believe they are no longer a big issue.

  • Performance - over the last years, gopls performance/stability improved significantly. The latest gopls v0.12.x enables incremental build/diagnostics which I hope allows more scalable analysis as you type, etc, even when you work on a huge repos. There is also a directoryFilters option users narrow their working directories when working on a mono repo. If you turned off gopls diagnostics due to the performance concer, give it another try and report an issue if it doesn't meet your need.

  • Customization - some users wanted to run a custom linter through 'vet'. In check: allow use of custom vet when using the language server #1109 (comment) we discussed alternatives.

If there are other issues you wanted to work around by disabling gopls's diagnostics, please let us know.

@yayiji
Copy link

yayiji commented Jun 28, 2023

Hi @hyangah, I actually do diagnostics, but only when I need to.

When I am coding, I really want everything to be quiet so I can focus on coding and not have anything distracting me.

After that, if I need to check my code, I will run go lint or go vet for diagnostics.

@hyangah
Copy link
Contributor

hyangah commented Jun 28, 2023

@yayiji Do you manually run go vet and go build from terminal? We can have an option to turn off (or hide) all diagnostics. That will include compiler errors, too. Will this work for you?

@yayiji
Copy link

yayiji commented Jun 29, 2023

@hyangah I use go vet or go lint in vscode from the command palette. Other commands I prefer to use from the terminal.

I have the following configuration in my setting.json

  "go.lintOnSave" :"off",
  "go.vetOnSave":"off",
  "go.languageServerExperimentalFeatures": {
      "diagnostics": false
  },

The last setting worked perfectly before, but now it does not work.

@evan361425
Copy link

evan361425 commented Aug 2, 2023

Seams like it is default setting on vet(only diagnostic when save the file).

Screenshot 2023-08-02 at 4 30 53 PM

vet the current package on file saving

But it still diagnose my code every time I type and I try to add some flags on gopls server:

// vscode settings.json
{
  "gopls": {
    // (Advanced) diagnosticsDelay controls the amount of time that gopls waits
    // after the most recent file modification before computing deep diagnostics.
    // Simple diagnostics (parsing and type-checking) are always run immediately
    // on recently modified packages.
    // This option must be set to a valid duration string, for example "250ms".
    "ui.diagnostic.diagnosticsDelay": "10s"
  }
}

Not what I want that should diagnostics only after I save the file, but still some work around?

@metaleap
Copy link

There's a lot of talk about custom vet etc. in this thread but I got here because the original "some users might like to see diagnostics only when they save a file" — which I'm really keen on! And not just me, there's literally-dozens-of-us!

Seeing all sorts of live wiggly red lines bouncing about while typing the yet-incomplete line or block irritates me to no end. (In fact, if one could just tell VS another color than red/orange/anything-bright, it would also solve my hangup just as well.)

Having seen the mentions here of go.languageServerExperimentalFeatures I tried it out with "go.languageServerExperimentalFeatures": {"diagnostics": false} and it works a charm! Soon as I save, the diagnostics and wigglies do show up, which I then appreciate, having completed my current train of thought into code.

Now I notice that this very option is already depreciated and primed for removal.

Please leave it in and just rename the "experimental" naming! What's the hurt? It already "works" as far as the original comment goes. Some users do really treasure show-probs-on-save-only. Not constant "realtime nagging", because one just typed a var and haven't used it yet, or just typed a func body and isn't returning yet, etc etc...

@metaleap
Copy link

metaleap commented Oct 15, 2023

Just realized a subtlety with this, as it is it doesn't truly solve "diags-on-save-only" — when a change is fine for the .go file being saved but breaking other ones in other pkgs. Live diags show this, and so would diags-on-save ideally. But not currently with "diagnostics":false. So back to live diags for me for now I guess...

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/534861 mentions this issue: gopls/internal/lsp: add OnSave diagnostics

gopherbot pushed a commit to golang/tools that referenced this issue Oct 31, 2023
Gopls publishes diagnostics as soon as it observes issues, even
while the user is in the middle of typing resulting in transient
errors. Some users find this behavior rather distracting.
'diagnosticsDelay' may help avoid wasted work, but time-based
decision does not always match users' expectation on when they want
to see diagnostics updates.

Historically, vscode-go offered two additional ways to diagnose code.

* On save
* Manual trigger

They were implemented by running the go compiler and vet/lint tools
outside gopls. Now we are working to move all code analysis logic
into the language server (golang/vscode-go#2799). We need replacement
for these features (golang/vscode-go#50).

This change introduces a new gopls setting 'diagnosticsTrigger'.
The default is 'Edit'. The additional option is 'Save', which
is implemented by preventing file modification events from triggering
diagnosis. This helps migrating users of the legacy "Build/Vet On Save"
mode. For users of the manual trigger mode, we can consider to add the
"Manual" mode and expose a custom LSP command that triggers
diagnosis when we see the demand.

Alternatives I explored:

* Pull Model Diagnostics - LSP 3.17 introduced client-initiated diagnostics
supporta The idea fits well with 'on save' or 'manual trigger'
features, but I am afraid this requires non-trivial amount of work in
gopls side.

  https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_pullDiagnostics

Moreover, the state of client side implementations is
unclear. For example, VS Code does not seem to support all features
listed in the DiagnosticClientCapability yet. The interaction between
DocumentDiagnostics and WorkspaceDiagnostics, and how mature the vscode
implementation is unclear to me at this moment.

* Emulate from Client-side - I attempted to buffer diagnostics reports
in the LSP client middleware layer, and make them visible only when
files are saved. That adds a non-trivial amount of TS/JS code on the
extension side which defeats the purpose of our deprecation work.
Moreover, that causes the diagnostics diff to be computed in one more
place (in addition to VSCode side and Gopls side), and adds
complexities. I also think some users in other editors want this
feature.

For golang/vscode-go#50

Change-Id: If07d3446bee7bed90851ad2272d82d163ae586cd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/534861
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@hyangah
Copy link
Contributor

hyangah commented Oct 31, 2023

I just merged cl/534861 that implements aka "surface problems on file save only" feature. The change adds a new setting
ui.diagnostic.diagnosticsTrigger. To test this feature:

  1. Build gopls from the source.
git clone https://go.googlesource.com/tools
cd tools/gopls
go install
  1. Use this setting: diagnosticsTrigger
    "gopls": {
        "ui.diagnostic.diagnosticsTrigger": "Save", 
     },
  1. Restart gopls.

Please test it and provide feedback. I hope we can get this included in gopls v0.15.

@metaleap
Copy link

metaleap commented Nov 1, 2023

@hyangah wowzers, works flawlessly! I'm so looking forward to this being merged into mainline gopls asap 👍 tranquil typing of code without the overhasty red lines, I'll take it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/analysis issues related to diagnostics and analysis done by gopls NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

10 participants