Skip to content

Commit

Permalink
gopls: manual tweaks after the lsp/ -> . rename
Browse files Browse the repository at this point in the history
Also, update docs on testing.

Change-Id: Ie0381a84452162fe8036ae5e0be0f89071bebfd7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/557742
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
  • Loading branch information
adonovan authored and gopherbot committed Jan 24, 2024
1 parent 95c6ac4 commit 238800b
Show file tree
Hide file tree
Showing 28 changed files with 71 additions and 74 deletions.
2 changes: 1 addition & 1 deletion copyright/copyright.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func checkFile(toolsDir, filename string) (bool, error) {
return shouldAddCopyright, nil
}

// Copied from golang.org/x/tools/gopls/internal/lsp/source/util.go.
// Copied from golang.org/x/tools/gopls/internal/golang/util.go.
// Matches cgo generated comment as well as the proposed standard:
//
// https://golang.org/s/generatedcode
Expand Down
68 changes: 35 additions & 33 deletions gopls/doc/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ claiming it.

## Getting started

Most of the `gopls` logic is in the `golang.org/x/tools/gopls/internal/lsp`
directory.
Most of the `gopls` logic is in the `golang.org/x/tools/gopls/internal`
directory. See [design/implementation.md] for an overview of the code organization.

## Build

Expand Down Expand Up @@ -94,41 +94,43 @@ Users are invited to share it if they are willing.

## Testing

To run tests for just `gopls/`, run,
The normal command you should use to run the tests after a change is:

```bash
cd /path/to/tools/gopls
go test ./...
```

But, much of the gopls work involves `internal/lsp` too, so you will want to
run both:

```bash
cd /path/to/tools
cd gopls && go test ./...
cd ..
go test ./internal/lsp/...
gopls$ go test -short ./...
```

There is additional information about the `internal/lsp` tests in the
[internal/lsp/tests `README`](https://github.com/golang/tools/blob/master/internal/lsp/tests/README.md).

### Integration tests

gopls has a suite of integration tests defined in the `./gopls/internal/test/integration`
directory. Each of these tests writes files to a temporary directory, starts a
separate gopls session, and scripts interactions using an editor-like API. As a
result of this overhead they can be quite slow, particularly on systems where
file operations are costly.

Due to the asynchronous nature of the LSP, integration tests assert
'expectations' that the editor state must achieve _eventually_. This can
make debugging the integration tests difficult. To aid with debugging, the tests
output their LSP logs on any failure. If your CL gets a test failure while
running the tests, please do take a look at the description of the error and
the LSP logs, but don't hesitate to [reach out](#getting-help) to the gopls
team if you need help.
(The `-short` flag skips some slow-running ones. The trybot builders
run the complete set, on a wide range of platforms.)

Gopls tests are a mix of two kinds.

- [Marker tests](../internal/test/marker) express each test scenario
in a standalone text file that contains the target .go, go.mod, and
go.work files, in which special annotations embedded in comments
drive the test. These tests are generally easy to write and fast
to iterate, but have limitations on what they can express.

- [Integration tests](../internal/test/integration) are regular Go
`func Test(*testing.T)` functions that make a series of calls to an
API for a fake LSP-enabled client editor. The API allows you to open
and edit a file, navigate to a definition, invoke other LSP
operations, and assert properties about the state.

Due to the asynchronous nature of the LSP, integration tests make
assertions about states that the editor must achieve eventually,
even when the program goes wrong quickly, it may take a while before
the error is reported as a failure to achieve the desired state
within several minutes. We recommend that you set
`GOPLS_INTEGRATION_TEST_TIMEOUT=10s` to reduce the timeout for
integration tests when debugging.

When they fail, the integration tests print the log of the LSP
session between client and server. Though verbose, they are very
helpful for debugging once you know how to read them.

Don't hesitate to [reach out](#getting-help) to the gopls team if you
need help.

### CI

Expand Down
11 changes: 5 additions & 6 deletions gopls/doc/design/implementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ referred to by their last segment, which is usually unambiguous.
The height of each blob corresponds loosely to its technical depth.
Some blocks are wide and shallow, such as [protocol], which declares
Go types for the entire LSP protocol. Others are deep, such as [cache]
and [source], as they contain a lot of dense logic and algorithms.
and [golang], as they contain a lot of dense logic and algorithms.

<!-- Source: https://docs.google.com/drawings/d/1CK6YSLt7G3svRoZf7skJI-lxRol2VI90YOxHcYS0DP4 -->
![Gopls architecture](architecture.svg)
Expand Down Expand Up @@ -106,7 +106,7 @@ particular language:
[mod] for go.mod files;
[work] for go.work files;
[template] for files in `text/template` syntax; and
[source], for files in Go itself.
[golang], for files in Go itself.
This package, by far the largest, provides the main features of gopls:
navigation, analysis, and refactoring of Go code.
As most users imagine it, this package _is_ gopls.
Expand All @@ -125,7 +125,7 @@ order the packages based on the sequence in which they are encountered
during processing of a particular request; in such a view, the bottom
layer would represent the "wire" (protocol and command), the next
layer up would hold the RPC-related packages (lsprpc and server), and
features (e.g. source, mod, work, template) would be at the top.
features (e.g. golang, mod, work, template) would be at the top.

<!--
A dynamic view would be an interesting topic for another article.
Expand All @@ -148,14 +148,13 @@ provided as a debugging aid (but see
[cache]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/cache
[cmd]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/cmd
[command]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/protocol/command
[debug]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/lsp/debug
[debug]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/debug
[file]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/file
[filecache]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/filecache
[go/analysis]: https://pkg.go.dev/golang.org/x/tools@master/go/analysis
[go/packages]: https://pkg.go.dev/golang.org/x/tools@master/go/packages
[gopls]: https://pkg.go.dev/golang.org/x/tools/gopls@master
[jsonrpc2]: https://pkg.go.dev/golang.org/x/tools@master/internal/jsonrpc2
[lsp]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/lsp
[lsprpc]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/lsprpc
[memoize]: https://github.com/golang/tools/tree/master/internal/memoize
[metadata]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/cache/metadata
Expand All @@ -165,7 +164,7 @@ provided as a debugging aid (but see
[protocol]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/protocol
[server]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/server
[settings]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/settings
[source]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/lsp/source
[golang]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/golang
[template]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/template
[typerefs]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/cache/typerefs
[work]: https://pkg.go.dev/golang.org/x/tools/gopls@master/internal/work
Expand Down
2 changes: 1 addition & 1 deletion gopls/doc/settings.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Settings

<!--TODO: Generate this file from the documentation in golang.org/x/tools/gopls/internal/lsp/source/options.go.-->
<!--TODO: Generate this file from the documentation in golang.org/x/tools/gopls/internal/golang/options.go.-->

This document describes the global settings for `gopls` inside the editor.
The settings block will be called `"gopls"` and contains a collection of
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/call_hierarchy.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import (
"path/filepath"

"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/safetoken"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/change_signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import (
"regexp"

"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/parsego"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/safetoken"
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/golang/code_lens.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import (
"regexp"
"strings"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/protocol/command"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/protocol/command"
)

type LensFunc func(context.Context, *cache.Snapshot, file.Handle) ([]protocol.CodeLens, error)
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/golang/codeaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import (

"golang.org/x/tools/go/ast/inspector"
"golang.org/x/tools/gopls/internal/analysis/fillstruct"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/parsego"
"golang.org/x/tools/gopls/internal/protocol/command"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/protocol/command"
"golang.org/x/tools/gopls/internal/settings"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/slices"
Expand Down
6 changes: 1 addition & 5 deletions gopls/internal/golang/comment_go119.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,7 @@ package golang
// is a new package (go/doc/comment) for processing them.
// As long as gopls has to compile under earlier versions, tests
// have to pass with both the old and new code, which produce
// slightly different results. (cmd/test/definition.go, source/comment_test.go,
// and source/source_test.go) Each of the test files checks the results
// with a function, tests.CheckSameMarkdown, that accepts both the old and the new
// results. (The old code escapes many characters the new code does not,
// and the new code sometimes adds a blank line.)
// slightly different results.

// When gopls no longer needs to compile with go1.18, the old comment.go should
// be replaced by this file, the golden test files should be updated.
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import (
"go/token"
"go/types"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
"golang.org/x/tools/gopls/internal/cache/parsego"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/internal/event"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (

"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/progress"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/settings"
"golang.org/x/tools/gopls/internal/util/maps"
)
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/golang/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import (
"golang.org/x/tools/gopls/internal/analysis/stubmethods"
"golang.org/x/tools/gopls/internal/analysis/undeclaredname"
"golang.org/x/tools/gopls/internal/analysis/unusedparams"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/parsego"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/internal/imports"
Expand Down Expand Up @@ -191,7 +191,7 @@ func suggestedFixToEdits(ctx context.Context, snapshot *cache.Snapshot, fset *to

// addEmbedImport adds a missing embed "embed" import with blank name.
func addEmbedImport(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, _, _ token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
// Like goalng.AddImport, but with _ as Name and using our pgf.
// Like golang.AddImport, but with _ as Name and using our pgf.
protoEdits, err := ComputeOneImportFixEdits(snapshot, pgf, &imports.ImportFix{
StmtInfo: imports.ImportInfo{
ImportPath: "embed",
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/folding_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (
"sort"
"strings"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/safetoken"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
"strings"
"text/scanner"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/safetoken"
"golang.org/x/tools/internal/diff"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/highlight.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
"go/types"

"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/typesutil"
"golang.org/x/tools/internal/event"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/hover.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ import (
"golang.org/x/text/unicode/runenames"
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/types/typeutil"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
"golang.org/x/tools/gopls/internal/cache/parsego"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/settings"
"golang.org/x/tools/gopls/internal/util/bug"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/implementation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import (
"sync"

"golang.org/x/sync/errgroup"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
"golang.org/x/tools/gopls/internal/cache/methodsets"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/safetoken"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/inlay_hint.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
"go/types"
"strings"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/typesutil"
"golang.org/x/tools/internal/event"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/known_packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import (
"sync"
"time"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
)
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/references.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ import (

"golang.org/x/sync/errgroup"
"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
"golang.org/x/tools/gopls/internal/cache/methodsets"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/safetoken"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ import (
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/go/types/typeutil"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
"golang.org/x/tools/gopls/internal/cache/parsego"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/gopls/internal/util/safetoken"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/signature_help.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
"strings"

"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/settings"
"golang.org/x/tools/gopls/internal/util/bug"
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/symbols.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (
"go/token"
"go/types"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/internal/event"
)
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/golang/type_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"fmt"
"go/token"

"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/internal/event"
Expand Down
4 changes: 2 additions & 2 deletions gopls/internal/lsprpc/lsprpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import (
"sync/atomic"
"time"

"golang.org/x/tools/gopls/internal/debug"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/protocol/command"
"golang.org/x/tools/gopls/internal/debug"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/protocol/command"
"golang.org/x/tools/gopls/internal/server"
"golang.org/x/tools/gopls/internal/settings"
"golang.org/x/tools/internal/event"
Expand Down
Loading

0 comments on commit 238800b

Please sign in to comment.