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.

Sign up for GitHub

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

Jump to bottom

Deprecate legacy language providers (aka "go.useLanguageServer": false setting) #2799

Closed
hyangah opened this issue Jun 5, 2023 · 11 comments
Closed

Deprecate legacy language providers (aka "go.useLanguageServer": false setting) #2799

hyangah opened this issue Jun 5, 2023 · 11 comments
Milestone
v0.40.0

Comments

@hyangah
Copy link
Contributor

hyangah commented Jun 5, 2023

In early 2021 (blog post), the Go extension switched to using gopls as the default language feature provider. Since then, the Go Tools team and Go project contributors have continuously worked hard to improve the stability, performance, and functionality of gopls. Nowadays, we rarely see users disabling gopls in our issue tracker, even as the estimated number of VS Code Go extension users continues to grow.

Although gopls reached feature parity a while ago and we believe it performs better than the legacy language provider in many aspects, we kept the option to use the legacy language provider until now just in case users need to work with older versions of Go for which gopls's test coverage may not be sufficient. However, most of the legacy tools supporting the legacy language provider are no longer actively maintained and do not work with recent Go versions that require Go modules. According to our user surveys, very few users use go 1.15 or older.

It's time to remove this old code path, simplify our codebase, and focus on enhancing the delivery of the gopls-based language features.

  • In the next release of the extension (v0.39.0), we will notify affected users and delete the legacy provider after the release in September 2023.
  • After September 2023, setting "go.useLanguageServer": false will turn off all language features such as code completion, go-to-definition, formatting, auto-import, diagnostics, etc.

We have been asking users when they choose to disable the language server (by setting "go.useLanguageServer") for some time. Thank you so much for providing feedback and sharing your thoughts on what to improve.

Over the last 6 months, approximately 260 users responded. Here are the findings and our plans to address the concerns raised by users:

cc @golang/tools-team
@gopherbot gopherbot added this to the Untriaged milestone Jun 5, 2023
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/501198 mentions this issue: src/goMain.ts: show notification about go.useLanguageServer

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/501206 mentions this issue: package.json: add deprecation note for legacy tools settings

gopherbot pushed a commit that referenced this issue Jun 6, 2023
@hyangah
package.json: add deprecation note for legacy tools settings
0b6803b 
By adding the deprecation notes, the current users of these settings
will notice the deprecation when they happen to edit their
settings.json. Moreover, VS Code will exclude them from the default
settings UI and autocompletion list, which will help users not getting
distracted by these old settings.

Updates #2799

Change-Id: I771dd5aeb6f826882d2c234e80862a8fb7d45670
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/501206
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
gopherbot pushed a commit that referenced this issue Jun 8, 2023
@hyangah
src/goMain.ts: show notification about go.useLanguageServer
97f0627 
The notification provides options to open the settings
and suppress the notification forever.

Also update package.json to change the description of
the go.useLanguageServer setting.

Updates #2799

Change-Id: Ib0e4e4414942ab083d1932abcb34f5871911d3da
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/501198
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@suzmue suzmue modified the milestones: Untriaged, vscode-go/later Jun 29, 2023
@hyangah hyangah mentioned this issue Jul 17, 2023
Closed
@gopherbot
Copy link
Collaborator

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

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/535095 mentions this issue: src/goModules: delete unused functions

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/535097 mentions this issue: package.json: remove deprecated settings

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/535098 mentions this issue: src/goToolsInformation: clean up the tools list

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/535096 mentions this issue: src/language/legacy: delete legacy language feature providers

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/535715 mentions this issue: src/goDocumentSymbols: call LSP document symbols feature directly

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/535315 mentions this issue: test/gopls/goTest: move text explorer tests to test/gopls

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/535255 mentions this issue: test/gopls/codelens: move codelense test to gopls-based test

gopherbot pushed a commit that referenced this issue Oct 24, 2023
@hyangah
test/gopls/codelens: move codelense test to gopls-based test
dcf5ecc 
Instead of using the legacy document symbol provider, this test
will use the gopls-based go document symbol provider to find
the test function code lens places.

goplsTestEnv.utils.ts is a collection of helpers that starts the gopls
and collects the logs. They have been used by tests in
test/gopls/extension.test.ts. Now we are moving to a separate file
so other tests can use them to interact with gopls.

For #2799
For #1020

Change-Id: Ib3582073960db67e1f1ec5b284ab4945258cb62a
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/535255
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopherbot pushed a commit that referenced this issue Oct 24, 2023
@hyangah
test/gopls/goTest: move text explorer tests to test/gopls
ab8643c 
Test explorer depends on the document symbol provider backed by gopls.
Test them using gopls, instead of using the fallback, legacy document
symbol provider.

Some of the test explorer tests were stubbing WorkspaceConfiguration
objects' `get` property using sinon stub. That is read-only and restore
calls from moch tests teardown fail with "cannot assign to read only
property get of object" errors. I don't know why we didn't encounter
this issue before. Replace the stubbed objects with MockCfg.

Also, I observed that the stretchr test that runs after the codelens
tests failed because the gopls started for the stretchr test was trying
to load package for test/testdata/stretchrTestSuite/suite_test.go
from the directory test/testdata/codelens. I don't know why the gopls
instance decided to use the directory from the previous test run as
the workspace folder yet. Work around this issue by explicitly setting
the workspace directory when starting gopls.

For #2799
For #1020

Change-Id: I8311ea9a38d2ee109297418a4dd1cffc7bf6055b
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/535315
Auto-Submit: Suzy Mueller <suzmue@golang.org>
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopherbot pushed a commit that referenced this issue Oct 24, 2023
@hyangah
src/goDocumentSymbols: call LSP document symbols feature directly
c2bc62f 
Previously gopls document symbols provider invoked a registered
provider - assuming gopls's LSP implementation is wired with the
vscode. The client-side registrations are done asyncronously
https://github.com/microsoft/vscode-languageserver-node/blob/ade01596f886ae041a71cf2a1b8c86fc35ea9bfc/client/src/common/client.ts#L1283
so it's tricky to depend on during testing. Since we know the
client is running and ready to accept the feature, let's call
the document symbols feature directly.

For #2799

Change-Id: Iba574be7d70c9322cf354e1777e2b7f8b9688890
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/535715
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
gopherbot pushed a commit that referenced this issue Oct 25, 2023
@hyangah
src/language/legacy: delete legacy language feature providers
340f78b 
Except the legacy goFormat provider. It is still used for custom
formatter feature implementation.

For #2799

Change-Id: Ia4c04bc584110d88b728b7ee144a255645e458b6
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/535096
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopherbot pushed a commit that referenced this issue Oct 25, 2023
@hyangah
src/goModules: delete unused functions
5c1eda7 
promptToUpdateToolForModules is not being referenced. Delete it.

And delete the prompt that warns about goreturns.

For #2799

Change-Id: Ib89eeb0e73ac8e2466e8d87b7c1fcd60728dc7c6
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/535095
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
gopherbot pushed a commit that referenced this issue Oct 26, 2023
@hyangah
package.json: remove deprecated settings
a3d76a5 
and remove legacy tools installation code.

Deleted settings
  - Gocode related settings (go.gocodeFlags, go.gocodeAutoBuild,
    go.gocodePackageLookupMode)
  - Code suggestion settings (go.useCodeSnippetsOnFunctionSuggest,
    go.useCodeSnippetsOnFunctionSuggestWithoutType,
    go.autocompleteUnimportedPackages)
  - go.gotoSymbol.includeCoroot
  - go.docsTool
  - go.useGoProxyToCheckForToolUpdates (obsolete by go.toolsManagement
    flag in 2020. Delete all the transition code as well)
  - go.liveErrors

For #2799

Change-Id: Id949d2f4ea5963553170a9c168b08752cc2e825f
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/535097
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Oct 31, 2023
@hyangah
gopls/internal/lsp: add OnSave diagnostics
11828ff 
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>
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/538762 mentions this issue: src/goToolsInformation: delete legacy tools

@hyangah hyangah modified the milestones: vscode-go/backlog, v0.40.0 Nov 1, 2023
@hyangah hyangah mentioned this issue Nov 1, 2023
Closed
gopherbot pushed a commit that referenced this issue Nov 6, 2023
@hyangah
src/goToolsInformation: clean up the tools list
9c57ee2 
Remove legacy tools.
Update docs/tools.md and tools/installtools too.

And, unskip gotests feature tests.

For #2799

Change-Id: I99c6c2892864aac887a23a720c19c1218f61f7d6
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/535098
Commit-Queue: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.40.0
Development

No branches or pull requests
4 participants
@hyangah @suzmue @gopherbot and others