golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.08k stars 17.68k forks source link

x/tools/gopls: zero-config gopls workspaces #57979

Closed findleyr closed 10 months ago

findleyr commented 1 year ago

Zero-config gopls workspaces

This issue describes a change to gopls' internal data model that will allow it to "Do The Right Thing" when the user opens a Go file. By decoupling the relationship between builds and workspace folders, we can eliminate complexity related to configuring the workspace (hence "zero-config"), and lay the groundwork for later improvements such as better support for working on multiple sets of build tags simultaneously (#29202).

After this change, users can work on multiple modules inside of a workspace regardless of whether they are related by a go.work file or explicitly open as separate workspace folders.

Background

Right now, gopls determines a unique build (called a View) for each workspace folder. When a workspace folder is opened, gopls performs the following steps:

  1. Request configuration for each workspace folder using the workspace/configuration request with scopeUri set to the folder URI.
  2. Using this configuration (which may affect the Go environment), resolve a root directory for this folder: a. Check go env GOWORK. b. Else, look for go.mod in a parent directory (recursively). c. Else, look for go.mod in a nested directory, if there is only one such nested directory. This was done to support polyglot workspaces where the Go project is in a nested directory, but is a source of both confusion and unpredictable startup time. d. Else, use the folder as root.
  3. Load package metadata for the workspace by calling go/packages.Load.
  4. Type check packages. We "fully" type-check packages that are inside a workspace module, and attempt to type-check only the exported symbols of packages in dependencies outside the workspace.

Problems

There are several problems with this model:

New Model

We can address these problems by decoupling Views from workspace folders. The set of views will be dynamic, depending on both the set of open folders and the set of open files, and will be chosen to cover all open files.

Specifically, define new View and Folder types approximately as follows:

type Session struct {
    views   []*View
    folders []*Folder

    // other per-session fields
}

type View struct {
    viewType ViewType  // workspace (go.work), module (go.mod), GOPATH, or adhoc
    source   URI       // go.work file, go.mod file, or directory
    modules  []URI     // set of modules contained in this View, if any
    options   *Options // options derived from either session options, or folder options

    // …per-view state, such as the latest snapshot
}

type ViewType int

const (
    workspace ViewType = iota // go.work
    module ViewType           // go.mod
    gopath ViewType           // GOPATH directory
    adhoc ViewType            // ad-hoc directory – see below
)

type Folder struct {
    dir     URI      // workspace folder
    options *Options // configuration scoped to the workspace folder
}

A Session consists of a set of View objects describing modules (go.mod files), workspaces (go.work files), GOPATH directories or ad-hoc packages that the user is working on. This set is determined by both the workspace folders specified by the editor and the set of open files.

View types

The set of Views

We define the set of Views to ensure that we have coverage for each open folder, and each open file.

  1. For each workspace folder, determine a View using the the following algorithm:
  1. For each open file, apply the following algorithm:

Match to an existing View

Initializing views

Initialize views using the following logic. This essentially matches gopls’ current behavior.

Type-check packages (and report their compiler diagnostics) as follows:

Resolving requests to Views

When a file-oriented request is handled by gopls (a request prefixed with textDocument/, such as textDocument/definition), gopls must usually resolve package metadata associated with the file.

In most cases, gopls currently chooses an existing view that best applies to the file (cache.bestViewForURI), but this is already problematic, because it can lead to path-dependency and incomplete results (c.f. #57558). For example: when finding references from a package imported from multiple views, gopls currently only shows references in one view.

Wherever possible, gopls should multiplex queries across all Views and merge their results. This would lead to consistent behavior of cross references. In a future where gopls has better build-tag support, this could also lead to multiple locations for jump-to-definition results.

In some cases (for example hover or signatureHelp), we must pick one view. In these cases we can apply some heuristic, but it should be of secondary significance (any hover or signatureHelp result is better than none).

Updating Views

Based on the algorithms used to determine Views above, the following notifications may affect the set of Views:

Following these changes, gopls will re-run the algorithm above to determine a new set of Views. It will re-use existing Views that have not changed.

Whenever new Views are created, they are reinitialized as above.

Differences from the current model

The algorithms described above are not vastly divergent from gopls’ current behavior. The significant differences may be summarized as follows:

Downsides

While this change will make gopls “do the right thing” in more cases, there are a several notable downsides:

Future extension to build tags

By decoupling Views from workspace folders, it becomes possible for gopls to support working on multiple sets of build tags simultaneously. One can imagine that the algorithm above to compute views based on open files could be extended to GOOS and GOARCH: if an open file is not included in an existing view because of its GOOS or GOARCH build constraints, create a new view with updated environment.

The downsides above apply: potentially increased memory, and potentially confusing UX as the behavior of certain workspace-wide queries (such as references or workspace symbols) depends on the set of open files. We can work to mitigate these downsides, and in my opinion they do not outweigh the upsides, as these queries simply don't work in the current model.

Task List

Here's an approximate plan of attack for implementing this feature, which I'm aiming to complete by the end of the year. (migrated from https://github.com/golang/go/issues/57979#issuecomment-1787445478).

This is inside baseball, but may be interesting to @adonovan and @hyangah.

Phase 1: making Views immutable:

At this point, gopls should still behave identically, but Views will have immutable options and main modules. There may be a bit more churn when configuration or go.work files change, but such events should be very infrequent, and it seems reasonable to reload the workspace when they occur.

Phase 2: supporting multiple views per folder

Phase 3: support for multiple GOOS/GOARCH combinations

gopherbot commented 1 year ago

Change https://go.dev/cl/495256 mentions this issue: gopls/internal/lsp/cache: fix race in adhoc reloading

gopherbot commented 1 year ago

Change https://go.dev/cl/496880 mentions this issue: gopls/internal/lsp/cache: limit module scan to 100K files

gopherbot commented 1 year ago

Change https://go.dev/cl/526160 mentions this issue: gopls/internal/lsp/cache: move workingDir to workspaceInformation

gopherbot commented 1 year ago

Change https://go.dev/cl/526417 mentions this issue: gopls/internal/lsp/cache: move Option management to the Server

findleyr commented 1 year ago

Task list migrated to issue description

gopherbot commented 1 year ago

Change https://go.dev/cl/538798 mentions this issue: gopls/internal/regtest/marker: port builtin/keyword completion tests

gopherbot commented 1 year ago

Change https://go.dev/cl/538797 mentions this issue: gopls/internal/regtest/marker: port five arbitrary completion tests

gopherbot commented 1 year ago

Change https://go.dev/cl/538799 mentions this issue: gopls/internal/regtest/marker: port rank and func_rank tests

gopherbot commented 1 year ago

Change https://go.dev/cl/538800 mentions this issue: gopls/internal/regtest/marker: port remaining completion tests

gopherbot commented 1 year ago

Change https://go.dev/cl/538801 mentions this issue: gopls/internal/regtest/marker: port remaining rank and snippet tests

gopherbot commented 1 year ago

Change https://go.dev/cl/538796 mentions this issue: gopls/internal/lsp/cache: make options immutable on the view

gopherbot commented 1 year ago

Change https://go.dev/cl/538766 mentions this issue: gopls/internal/lsp/cache: move 'contains' from snapshot to view

gopherbot commented 1 year ago

Change https://go.dev/cl/538803 mentions this issue: gopls/internal/lsp/cache: isolate getWorkspaceInformation from Session

gopherbot commented 1 year ago

Change https://go.dev/cl/539658 mentions this issue: gopls/internal/lsp/cache: remove forceReloadMetadata from clone

gopherbot commented 1 year ago

Change https://go.dev/cl/539657 mentions this issue: gopls/internal/lsp/cache: pass workspace information into createView

gopherbot commented 1 year ago

Change https://go.dev/cl/539676 mentions this issue: gopls/internal/lsp/cache: remove baseCtx from the View

gopherbot commented 1 year ago

Change https://go.dev/cl/540296 mentions this issue: gopls/internal/lsp/cache: exclusively use persistent data in snapshot

gopherbot commented 1 year ago

Change https://go.dev/cl/542639 mentions this issue: gopls/internal/lsp/cache: export Snapshot

gopherbot commented 1 year ago

Change https://go.dev/cl/542638 mentions this issue: gopls/internal/lsp/source: remove View.Snapshot

gopherbot commented 1 year ago

Change https://go.dev/cl/542642 mentions this issue: gopls: remove the "tempModFile" setting

gopherbot commented 1 year ago

Change https://go.dev/cl/542622 mentions this issue: gopls/internal/lsp/cache: remove workspaceMode

gopherbot commented 1 year ago

Change https://go.dev/cl/540479 mentions this issue: gopls/internal/lsp/cache: move upgrades and vulns onto the snapshot

gopherbot commented 1 year ago

Change https://go.dev/cl/542619 mentions this issue: gopls/internal/lsp/cache: move go.work parsing into the view

gopherbot commented 1 year ago

Change https://go.dev/cl/542640 mentions this issue: gopls: simplify the Snapshot and View interfaces

gopherbot commented 11 months ago

Change https://go.dev/cl/546017 mentions this issue: gopls/internal/server: simplify snapshot diagnostics

gopherbot commented 11 months ago

Change https://go.dev/cl/546415 mentions this issue: gopls/internal/server: rewrite the server diagnostic tracking

hyangah commented 11 months ago

The zero-config gopls work is going to generalize and improve the adhoc package support. The development work is still in progress. I am adding some findings from testing ad-hoc package supports (without go.mod). cc @findleyr @adonovan

1) drop the package load error report that recommends use of go.mod, go.work, GOPATH when operating in ad-hock package mode. IMO this is somewhat obsolete after the zero-config gopls work.

2) consider to drop the hanging progress bar that reports package load error ("Error loading workspace: gopls was not able to find modules in your workspace"). Related: https://github.com/golang/go/issues/50885

Note: we can track ad-hoc package support outside a module or GOPATH as a separate issue and aim for the future milestone after v0.15 is also an option if this is too tricky.

gopherbot commented 11 months ago

Change https://go.dev/cl/550075 mentions this issue: gopls/internal/lsp/cache: reconcile view modes and workspace packages

gopherbot commented 11 months ago

Change https://go.dev/cl/550378 mentions this issue: gopls/internal/lsp/cache: simplify view definitions

gopherbot commented 11 months ago

Change https://go.dev/cl/550815 mentions this issue: gopls/internal/lsp/cache: add the zero-config algorithm, and a unit test

gopherbot commented 11 months ago

Change https://go.dev/cl/550915 mentions this issue: gopls/internal/lsp/cache: add views for unused modules in selectViews

gopherbot commented 11 months ago

Change https://go.dev/cl/551295 mentions this issue: gopls/internal/lsp/cache: switch to new bestViewForURI logic

gopherbot commented 11 months ago

Change https://go.dev/cl/551896 mentions this issue: gopls/internal/lsp/cache: finish integrating the zero-config algorithm

gopherbot commented 11 months ago

Change https://go.dev/cl/551895 mentions this issue: gopls/internal/lsp/cache: don't preserve sequence IDs in updated views

gopherbot commented 10 months ago

Change https://go.dev/cl/552315 mentions this issue: gopls/internal/lsp/cache: associate env with folders, not views

gopherbot commented 10 months ago

Change https://go.dev/cl/552355 mentions this issue: gopls/internal/lsp/cache: add support for checking views from tests

gopherbot commented 10 months ago

Change https://go.dev/cl/551897 mentions this issue: gopls/internal/lsp/cache: add dynamic build tag support

gopherbot commented 10 months ago

Change https://go.dev/cl/553096 mentions this issue: gopls/internal/lsp/cache: don't scan for modules when defining a view

gopherbot commented 10 months ago

Change https://go.dev/cl/553095 mentions this issue: gopls/internal/lsp/cache: remove validBuildConfiguration

gopherbot commented 10 months ago

Change https://go.dev/cl/553097 mentions this issue: gopls/internal/lsp/cache: simplify critical errors

findleyr commented 10 months ago

Ok, remaining TODOs for this issue are tracked in #29202, #64887, and #64888.

The fundamental goal of this issue is achieved: gopls is zero-config with respect to workspace layout. Following https://go.dev/cl/551897, it will also be zero-config with respect to GOOS/GOARCH combinations (at which point I think we can close #29202). Closing this as complete!

gopherbot commented 10 months ago

Change https://go.dev/cl/555197 mentions this issue: internal/debug: show new View information in the debug page

gopherbot commented 9 months ago

Change https://go.dev/cl/557500 mentions this issue: gopls/internal/lsp/cache: sort workspace folders in selectViews

aarzilli commented 9 months ago

I think there is a problem with the automatic GOOS/GOARCH thing where it doesn't work if cgo is used. For example editing this file on any OS that isn't freebsd.

findleyr commented 9 months ago

@aarzilli you are right: setting GOOS and GOARCH will, by default, disable cgo.

I haven't yet tested, but I wonder if we can make this work by setting CCFOR${GOOS}_${GOARCH}? https://pkg.go.dev/cmd/cgo#hdr-Using_cgo_with_the_go_command

aarzilli commented 9 months ago

I don't know how much that would help. I, for example, do not have CC_FOR_anything_other_than_linux_amd64. I think probably what needs to happen is that FakeImportC needs to be passed to go/types where appropriate (or maybe go/types needs to do it automatically when it can't do better).

findleyr commented 9 months ago

I think probably what needs to happen is that FakeImportC needs to be passed to go/types where appropriate (or maybe go/types needs to do it automatically when it can't do better)

Indeed, I'll give that a try. Thanks.

gopherbot commented 9 months ago

Change https://go.dev/cl/560467 mentions this issue: gopls/internal/cache: remove findWorkspaceModFile

gopherbot commented 9 months ago

Change https://go.dev/cl/559636 mentions this issue: gopls/internal/cache: share goimports state for GOMODCACHE

cletter7 commented 9 months ago

Was this already released? How can I start using it?