all: merge master (e8cdaf4) into gopls-release-branch.0.13
For golang/go#61583
Merge List:
+ 2023-07-26 e8cdaf46d gopls/internal/lsp/cache: fast-path for type-checking active packages
+ 2023-07-26 da5abd36a gopls/internal/lsp/cache: fix use of time.NewTimer instead of NewTicker
+ 2023-07-26 b38978515 gopls: fix tests that depend on log sequencing
+ 2023-07-26 2ffc4dc5c all: fix some typos
Change-Id: I7a3de6330a6b6d0dca51de6094603cde2cec1b6a
diff --git a/cmd/compilebench/main.go b/cmd/compilebench/main.go
index 754acdc..6900b25 100644
--- a/cmd/compilebench/main.go
+++ b/cmd/compilebench/main.go
@@ -567,10 +567,10 @@
return nil
}
-// genSymAbisFile runs the assembler on the target packge asm files
+// genSymAbisFile runs the assembler on the target package asm files
// with "-gensymabis" to produce a symabis file that will feed into
// the Go source compilation. This is fairly hacky in that if the
-// asm invocation convenion changes it will need to be updated
+// asm invocation convention changes it will need to be updated
// (hopefully that will not be needed too frequently).
func genSymAbisFile(pkg *Pkg, symAbisFile, incdir string) error {
args := []string{"-gensymabis", "-o", symAbisFile,
diff --git a/go/analysis/passes/buildssa/testdata/src/c/c.go b/go/analysis/passes/buildssa/testdata/src/c/c.go
index 387a3b0..d6ce8b8 100644
--- a/go/analysis/passes/buildssa/testdata/src/c/c.go
+++ b/go/analysis/passes/buildssa/testdata/src/c/c.go
@@ -19,6 +19,6 @@
m := b.G.Load()
f := b.Load(&b.G)
if f != m {
- panic("loads of b.G are expected to be indentical")
+ panic("loads of b.G are expected to be identical")
}
}
diff --git a/go/analysis/passes/errorsas/testdata/src/a/a.go b/go/analysis/passes/errorsas/testdata/src/a/a.go
index 7a9ae89..222b279 100644
--- a/go/analysis/passes/errorsas/testdata/src/a/a.go
+++ b/go/analysis/passes/errorsas/testdata/src/a/a.go
@@ -29,7 +29,7 @@
ei interface{}
)
errors.As(nil, &e) // want `second argument to errors.As should not be \*error`
- errors.As(nil, &m) // *T where T implemements error
+ errors.As(nil, &m) // *T where T implements error
errors.As(nil, &f) // *interface
errors.As(nil, perr()) // want `second argument to errors.As should not be \*error`
errors.As(nil, ei) // empty interface
diff --git a/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go b/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
index 4f7ae84..16a974c 100644
--- a/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
+++ b/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
@@ -26,7 +26,7 @@
tw twice[myError[int]]
)
errors.As(nil, &e)
- errors.As(nil, &m) // *T where T implemements error
+ errors.As(nil, &m) // *T where T implements error
errors.As(nil, &tw.t) // *T where T implements error
errors.As(nil, perr[error]()) // want `second argument to errors.As should not be \*error`
diff --git a/go/ssa/builder_go120_test.go b/go/ssa/builder_go120_test.go
index acdd182..2472a9d 100644
--- a/go/ssa/builder_go120_test.go
+++ b/go/ssa/builder_go120_test.go
@@ -36,7 +36,7 @@
// as []rune, pointers to rune arrays, rune arrays, or strings.
//
// Comments listed given the current emitted instructions [approximately].
- // If multiple conversions are needed, these are seperated by |.
+ // If multiple conversions are needed, these are separated by |.
// rune was selected as it leads to string casts (byte is similar).
// The length 2 is not significant.
// Multiple array lengths may occur in a cast in practice (including 0).
diff --git a/go/ssa/subst.go b/go/ssa/subst.go
index 89c41a8..23d19ae 100644
--- a/go/ssa/subst.go
+++ b/go/ssa/subst.go
@@ -388,7 +388,7 @@
// no type params to substitute
// (2)generic method and recv needs to be substituted.
- // Recievers can be either:
+ // Receivers can be either:
// named
// pointer to named
// interface
diff --git a/go/ssa/util.go b/go/ssa/util.go
index 7735dd8..68cc971 100644
--- a/go/ssa/util.go
+++ b/go/ssa/util.go
@@ -304,7 +304,7 @@
return T
}
-// A type for representating an canonized list of types.
+// A type for representing a canonized list of types.
type typeList []types.Type
func (l *typeList) identical(ts []types.Type) bool {
diff --git a/gopls/internal/lsp/analysis/stubmethods/stubmethods.go b/gopls/internal/lsp/analysis/stubmethods/stubmethods.go
index e24447d..930cb99 100644
--- a/gopls/internal/lsp/analysis/stubmethods/stubmethods.go
+++ b/gopls/internal/lsp/analysis/stubmethods/stubmethods.go
@@ -115,7 +115,7 @@
//
// TODO(adonovan): this function (and its following 5 helpers) tries
// to deduce a pair of (concrete, interface) types that are related by
-// an assignment, either explictly or through a return statement or
+// an assignment, either explicitly or through a return statement or
// function call. This is essentially what the refactor/satisfy does,
// more generally. Refactor to share logic, after auditing 'satisfy'
// for safety on ill-typed code.
diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go
index 5d36f5e..6852c74 100644
--- a/gopls/internal/lsp/cache/check.go
+++ b/gopls/internal/lsp/cache/check.go
@@ -93,10 +93,30 @@
// the type-checking operation.
func (s *snapshot) TypeCheck(ctx context.Context, ids ...PackageID) ([]source.Package, error) {
pkgs := make([]source.Package, len(ids))
- post := func(i int, pkg *Package) {
- pkgs[i] = pkg
+
+ var (
+ needIDs []PackageID // ids to type-check
+ indexes []int // original index of requested ids
+ )
+
+ // Check for existing active packages, as any package will do.
+ //
+ // This is also done inside forEachPackage, but doing it here avoids
+ // unnecessary set up for type checking (e.g. assembling the package handle
+ // graph).
+ for i, id := range ids {
+ if pkg := s.getActivePackage(id); pkg != nil {
+ pkgs[i] = pkg
+ } else {
+ needIDs = append(needIDs, id)
+ indexes = append(indexes, i)
+ }
}
- return pkgs, s.forEachPackage(ctx, ids, nil, post)
+
+ post := func(i int, pkg *Package) {
+ pkgs[indexes[i]] = pkg
+ }
+ return pkgs, s.forEachPackage(ctx, needIDs, nil, post)
}
// getImportGraph returns a shared import graph use for this snapshot, or nil.
diff --git a/gopls/internal/lsp/cache/parse.go b/gopls/internal/lsp/cache/parse.go
index 7afa5d9..42353df 100644
--- a/gopls/internal/lsp/cache/parse.go
+++ b/gopls/internal/lsp/cache/parse.go
@@ -398,7 +398,7 @@
//
// }
//
-// The resulting bool reports whether any fixing occured.
+// The resulting bool reports whether any fixing occurred.
func fixEmptySwitch(body *ast.BlockStmt, tok *token.File, src []byte) bool {
// We only care about empty switch statements.
if len(body.List) > 0 || !body.Rbrace.IsValid() {
@@ -475,7 +475,7 @@
//
// TODO(rfindley): should this constitute an ast 'fix'?
//
-// The resulting bool reports whether any fixing occured.
+// The resulting bool reports whether any fixing occurred.
func fixPhantomSelector(sel *ast.SelectorExpr, tf *token.File, src []byte) bool {
if !isPhantomUnderscore(sel.Sel, tf, src) {
return false
@@ -525,7 +525,7 @@
// parser is looking for the conditional expression. However, "i := 0"
// are not valid expressions, so we get a BadExpr.
//
-// The resulting bool reports whether any fixing occured.
+// The resulting bool reports whether any fixing occurred.
func fixInitStmt(bad *ast.BadExpr, parent ast.Node, tok *token.File, src []byte) bool {
if !bad.Pos().IsValid() || !bad.End().IsValid() {
return false
diff --git a/gopls/internal/lsp/cache/parse_cache.go b/gopls/internal/lsp/cache/parse_cache.go
index b1febce..438cc62 100644
--- a/gopls/internal/lsp/cache/parse_cache.go
+++ b/gopls/internal/lsp/cache/parse_cache.go
@@ -254,7 +254,7 @@
func (c *parseCache) gc() {
const period = 10 * time.Second // gc period
- timer := time.NewTimer(period)
+ timer := time.NewTicker(period)
defer timer.Stop()
for {
diff --git a/gopls/internal/lsp/fake/workdir.go b/gopls/internal/lsp/fake/workdir.go
index c064b28..d5e8eb2 100644
--- a/gopls/internal/lsp/fake/workdir.go
+++ b/gopls/internal/lsp/fake/workdir.go
@@ -120,7 +120,7 @@
// fileID identifies a file version on disk.
type fileID struct {
mtime time.Time
- hash string // empty if mtime is old enough to be reliabe; otherwise a file digest
+ hash string // empty if mtime is old enough to be reliable; otherwise a file digest
}
func hashFile(data []byte) string {
@@ -363,7 +363,7 @@
return nil
}
- // Opt: avoid reading the file if mtime is sufficently old to be reliable.
+ // Opt: avoid reading the file if mtime is sufficiently old to be reliable.
//
// If mtime is recent, it may not sufficiently identify the file contents:
// a subsequent write could result in the same mtime. For these cases, we
diff --git a/gopls/internal/lsp/regtest/expectation.go b/gopls/internal/lsp/regtest/expectation.go
index ece9aaa..0136870 100644
--- a/gopls/internal/lsp/regtest/expectation.go
+++ b/gopls/internal/lsp/regtest/expectation.go
@@ -212,20 +212,6 @@
}
}
-// ReadLogs is an expectation that may be used with AfterChange or OnceMet
-// conditions to copy logs into the provided slice.
-func ReadLogs(into *[]*protocol.LogMessageParams) Expectation {
- check := func(s State) Verdict {
- *into = make([]*protocol.LogMessageParams, len(s.logs))
- copy(*into, s.logs)
- return Met
- }
- return Expectation{
- Check: check,
- Description: "read logs",
- }
-}
-
// NoOutstandingWork asserts that there is no work initiated using the LSP
// $/progress API that has not completed.
func NoOutstandingWork() Expectation {
@@ -521,6 +507,10 @@
// The count argument specifies the expected number of matching logs. If
// atLeast is set, this is a lower bound, otherwise there must be exactly count
// matching logs.
+//
+// Logs are asynchronous to other LSP messages, so this expectation should not
+// be used with combinators such as OnceMet or AfterChange that assert on
+// ordering with respect to other operations.
func LogMatching(typ protocol.MessageType, re string, count int, atLeast bool) Expectation {
rec, err := regexp.Compile(re)
if err != nil {
@@ -537,6 +527,11 @@
if found == count || (found >= count && atLeast) {
return Met
}
+ // If we require an exact count, and have received more than expected, the
+ // expectation can never be met.
+ if found > count && !atLeast {
+ return Unmeetable
+ }
return Unmet
}
desc := fmt.Sprintf("log message matching %q expected %v times", re, count)
diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go
index 40810dd..02bdaed 100644
--- a/gopls/internal/lsp/source/rename.go
+++ b/gopls/internal/lsp/source/rename.go
@@ -1061,7 +1061,7 @@
}
// Find all identifiers in the package that define or use a
- // renamed object. We iterate over info as it is more efficent
+ // renamed object. We iterate over info as it is more efficient
// than calling ast.Inspect for each of r.pkg.CompiledGoFiles().
type item struct {
node ast.Node // Ident, ImportSpec (obj=PkgName), or CaseClause (obj=Var)
diff --git a/gopls/internal/regtest/bench/repo_test.go b/gopls/internal/regtest/bench/repo_test.go
index 18c6edf..c3b8b3b 100644
--- a/gopls/internal/regtest/bench/repo_test.go
+++ b/gopls/internal/regtest/bench/repo_test.go
@@ -115,7 +115,7 @@
tb.Fatalf("repo %s does not exist", name)
}
if !repo.short && testing.Short() {
- tb.Skipf("large repo %s does not run whith -short", repo.name)
+ tb.Skipf("large repo %s does not run with -short", repo.name)
}
return repo
}
diff --git a/gopls/internal/regtest/diagnostics/diagnostics_test.go b/gopls/internal/regtest/diagnostics/diagnostics_test.go
index 29a54f1..623cd72 100644
--- a/gopls/internal/regtest/diagnostics/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics/diagnostics_test.go
@@ -8,7 +8,6 @@
"context"
"fmt"
"os/exec"
- "strings"
"testing"
"golang.org/x/tools/gopls/internal/bug"
@@ -433,7 +432,11 @@
func TestMissingDependency(t *testing.T) {
Run(t, testPackageWithRequire, func(t *testing.T, env *Env) {
env.OpenFile("print.go")
- env.Await(LogMatching(protocol.Error, "initial workspace load failed", 1, false))
+ env.Await(
+ // Log messages are asynchronous to other events on the LSP stream, so we
+ // can't use OnceMet or AfterChange here.
+ LogMatching(protocol.Error, "initial workspace load failed", 1, false),
+ )
})
}
@@ -1303,42 +1306,18 @@
)
env.OpenFile("a/a_exclude.go")
- countLoads := func(logs []*protocol.LogMessageParams) int {
- count := 0
- for _, log := range logs {
- if strings.Contains(log.Message, `go/packages.Load`) {
- count++
- }
- }
- return count
- }
-
- var before []*protocol.LogMessageParams
- env.AfterChange(
- Diagnostics(env.AtRegexp("a/a_exclude.go", "package (a)")),
- ReadLogs(&before),
- )
+ loadOnce := LogMatching(protocol.Info, "query=.*file=.*a_exclude.go", 1, false)
+ env.Await(loadOnce) // can't use OnceMet or AfterChange as logs are async
// Check that orphaned files are not reloaded, by making a change in
// a.go file and confirming that the workspace diagnosis did not reload
// a_exclude.go.
//
- // Currently, typing in a_exclude.go does result in a reload, because we
- // aren't precise about which changes could result in an unloadable file
- // becoming loadable.
- loads0 := countLoads(before)
- if loads0 == 0 {
- t.Fatal("got 0 Load log messages, want at least 1", loads0)
- }
+ // This is racy (but fails open) because logs are asynchronous to other LSP
+ // operations. There's a chance gopls _did_ log, and we just haven't seen
+ // it yet.
env.RegexpReplace("a/a.go", "package a", "package a // arbitrary comment")
- var after []*protocol.LogMessageParams
- env.AfterChange(
- Diagnostics(env.AtRegexp("a/a_exclude.go", "package (a)")),
- ReadLogs(&after),
- )
- if loads := countLoads(after); loads != loads0 {
- t.Errorf("got %d Load log messages after change to orphaned file, want %d", loads, loads0)
- }
+ env.AfterChange(loadOnce)
})
}
@@ -1853,8 +1832,10 @@
`
Run(t, files, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
- env.AfterChange(
+ env.Await(
// Check that we have only loaded "<dir>/..." once.
+ // Log messages are asynchronous to other events on the LSP stream, so we
+ // can't use OnceMet or AfterChange here.
LogMatching(protocol.Info, `.*query=.*\.\.\..*`, 1, false),
)
})
diff --git a/gopls/internal/regtest/watch/watch_test.go b/gopls/internal/regtest/watch/watch_test.go
index f485b74..dccf869 100644
--- a/gopls/internal/regtest/watch/watch_test.go
+++ b/gopls/internal/regtest/watch/watch_test.go
@@ -383,7 +383,9 @@
).Run(t, pkg, func(t *testing.T, env *Env) {
env.OpenFile("a/a.go")
env.OpenFile("a/a_unneeded.go")
- env.AfterChange(
+ env.Await(
+ // Log messages are asynchronous to other events on the LSP stream, so we
+ // can't use OnceMet or AfterChange here.
LogMatching(protocol.Info, "a_unneeded.go", 1, false),
)
@@ -395,7 +397,7 @@
Diagnostics(env.AtRegexp("a/a.go", "fmt")),
)
env.SaveBuffer("a/a.go")
- env.AfterChange(
+ env.Await(
// There should only be one log message containing
// a_unneeded.go, from the initial workspace load, which we
// check for earlier. If there are more, there's a bug.
@@ -411,7 +413,7 @@
).Run(t, pkg, func(t *testing.T, env *Env) {
env.OpenFile("a/a.go")
env.OpenFile("a/a_unneeded.go")
- env.AfterChange(
+ env.Await(
LogMatching(protocol.Info, "a_unneeded.go", 1, false),
)
@@ -423,7 +425,7 @@
Diagnostics(env.AtRegexp("a/a.go", "fmt")),
)
env.SaveBuffer("a/a.go")
- env.AfterChange(
+ env.Await(
// There should only be one log message containing
// a_unneeded.go, from the initial workspace load, which we
// check for earlier. If there are more, there's a bug.
diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go
index 02e3a8c..fa04a41 100644
--- a/gopls/internal/regtest/workspace/workspace_test.go
+++ b/gopls/internal/regtest/workspace/workspace_test.go
@@ -177,7 +177,7 @@
replace random.org => %s
`, env.ReadWorkspaceFile("pkg/go.mod"), dir)
env.WriteWorkspaceFile("pkg/go.mod", goModWithReplace)
- env.AfterChange(
+ env.Await(
LogMatching(protocol.Info, `packages\.Load #\d+\n`, 2, false),
)
})
diff --git a/internal/persistent/map.go b/internal/persistent/map.go
index a5d3083..a9d878f 100644
--- a/internal/persistent/map.go
+++ b/internal/persistent/map.go
@@ -18,7 +18,7 @@
// * Each value is reference counted by nodes which hold it.
// * Each node is reference counted by its parent nodes.
// * Each map is considered a top-level parent node from reference counting perspective.
-// * Each change does always effectivelly produce a new top level node.
+// * Each change does always effectively produce a new top level node.
//
// Functions which operate directly with nodes do have a notation in form of
// `foo(arg1:+n1, arg2:+n2) (ret1:+n3)`.
