Skip to content

Commit

Permalink
deps: update x/tools and gopls to 9ee5ef7a (#851)
Browse files Browse the repository at this point in the history
* internal/jsonrpc2: remove Direction 9ee5ef7a
* internal/telemetry: a faster logging exporter 888a00fb
* internal/lsp: make tag iteration allocation-free 6a751267
* internal/lsp/cache: add concurrency error check for go cmds 46bd65c8
* internal/lsp: add type error fixes to existing diagnostics 4d14fc9c
* gopls/docs: adding nvim-lsp option in gopls README file cd5a53e0
  • Loading branch information
myitcv committed Apr 8, 2020
1 parent d035992 commit dd56e8b
Show file tree
Hide file tree
Showing 38 changed files with 418 additions and 378 deletions.
69 changes: 61 additions & 8 deletions cmd/govim/internal/golang_org_x_tools/gocommand/invoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,70 @@ import (
"io"
"os"
"os/exec"
"regexp"
"strings"
"sync"
"time"

"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/telemetry/event"
)

// An Runner will run go command invocations and serialize
// them if it sees a concurrency error.
type Runner struct {
// LoadMu guards packages.Load calls and associated state.
loadMu sync.Mutex
serializeLoads int
}

// 1.13: go: updates to go.mod needed, but contents have changed
// 1.14: go: updating go.mod: existing contents have changed since last read
var modConcurrencyError = regexp.MustCompile(`go:.*go.mod.*contents have changed`)

// Run calls Runner.RunRaw, serializing requests if they fight over
// go.mod changes.
func (runner *Runner) Run(ctx context.Context, inv Invocation) (*bytes.Buffer, error) {
stdout, _, friendly, _ := runner.RunRaw(ctx, inv)
return stdout, friendly
}

// Run calls Innvocation.RunRaw, serializing requests if they fight over
// go.mod changes.
func (runner *Runner) RunRaw(ctx context.Context, inv Invocation) (*bytes.Buffer, *bytes.Buffer, error, error) {
// We want to run invocations concurrently as much as possible. However,
// if go.mod updates are needed, only one can make them and the others will
// fail. We need to retry in those cases, but we don't want to thrash so
// badly we never recover. To avoid that, once we've seen one concurrency
// error, start serializing everything until the backlog has cleared out.
runner.loadMu.Lock()
var locked bool // If true, we hold the mutex and have incremented.
if runner.serializeLoads == 0 {
runner.loadMu.Unlock()
} else {
locked = true
runner.serializeLoads++
}
defer func() {
if locked {
runner.serializeLoads--
runner.loadMu.Unlock()
}
}()

for {
stdout, stderr, friendlyErr, err := inv.runRaw(ctx)
if friendlyErr == nil || !modConcurrencyError.MatchString(friendlyErr.Error()) {
return stdout, stderr, friendlyErr, err
}
event.Error(ctx, "Load concurrency error, will retry serially", err)
if !locked {
runner.loadMu.Lock()
runner.serializeLoads++
locked = true
}
}
}

// An Invocation represents a call to the go command.
type Invocation struct {
Verb string
Expand All @@ -26,16 +86,9 @@ type Invocation struct {
Logf func(format string, args ...interface{})
}

// Run runs the invocation, returning its stdout and an error suitable for
// human consumption, including stderr.
func (i *Invocation) Run(ctx context.Context) (*bytes.Buffer, error) {
stdout, _, friendly, _ := i.RunRaw(ctx)
return stdout, friendly
}

// RunRaw is like RunPiped, but also returns the raw stderr and error for callers
// that want to do low-level error handling/recovery.
func (i *Invocation) RunRaw(ctx context.Context) (stdout *bytes.Buffer, stderr *bytes.Buffer, friendlyError error, rawError error) {
func (i *Invocation) runRaw(ctx context.Context) (stdout *bytes.Buffer, stderr *bytes.Buffer, friendlyError error, rawError error) {
stdout = &bytes.Buffer{}
stderr = &bytes.Buffer{}
rawError = i.RunPiped(ctx, stdout, stderr)
Expand Down
4 changes: 3 additions & 1 deletion cmd/govim/internal/golang_org_x_tools/imports/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,8 @@ func getPackageExports(ctx context.Context, wrapped func(PackageExport), searchP
type ProcessEnv struct {
LocalPrefix string

GocmdRunner *gocommand.Runner

BuildFlags []string

// If non-empty, these will be used instead of the
Expand Down Expand Up @@ -830,7 +832,7 @@ func (e *ProcessEnv) invokeGo(ctx context.Context, verb string, args ...string)
Logf: e.Logf,
WorkingDir: e.WorkingDir,
}
return inv.Run(ctx)
return e.GocmdRunner.Run(ctx, inv)
}

func addStdlibCandidates(pass *pass, refs references) {
Expand Down
5 changes: 5 additions & 0 deletions cmd/govim/internal/golang_org_x_tools/imports/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"strings"

"golang.org/x/tools/go/ast/astutil"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/gocommand"
)

// Options is golang.org/x/tools/imports.Options with extra internal-only options.
Expand Down Expand Up @@ -154,6 +155,10 @@ func initialize(filename string, src []byte, opt *Options) ([]byte, *Options, er
GOSUMDB: os.Getenv("GOSUMDB"),
}
}
// Set the gocmdRunner if the user has not provided it.
if opt.Env.GocmdRunner == nil {
opt.Env.GocmdRunner = &gocommand.Runner{}
}
if src == nil {
b, err := ioutil.ReadFile(filename)
if err != nil {
Expand Down
22 changes: 0 additions & 22 deletions cmd/govim/internal/golang_org_x_tools/jsonrpc2/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,6 @@ import (
// The handler should return ErrNotHandled if it could not handle the request.
type Handler func(context.Context, *Request) error

// Direction is used to indicate to a logger whether the logged message was being
// sent or received.
type Direction bool

const (
// Send indicates the message is outgoing.
Send = Direction(true)
// Receive indicates the message is incoming.
Receive = Direction(false)
)

func (d Direction) String() string {
switch d {
case Send:
return "send"
case Receive:
return "receive"
default:
panic("unreachable")
}
}

// MethodNotFound is a Handler that replies to all call requests with the
// standard method not found response.
// This should normally be the final handler in a chain.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
errors := analysisinternal.GetTypeErrors(pass)
// Filter out the errors that are not relevant to this analyzer.
for _, typeErr := range errors {
matches := wrongReturnNumRegex.FindStringSubmatch(strings.TrimSpace(typeErr.Msg))
if len(matches) < 3 {
if !FixesError(typeErr.Msg) {
continue
}
wantNum, err := strconv.Atoi(matches[1])
if err != nil {
continue
}
gotNum, err := strconv.Atoi(matches[2])
if err != nil {
continue
}
// Logic for handling more return values than expected is hard.
if wantNum < gotNum {
continue
}

var file *ast.File
for _, f := range pass.Files {
if f.Pos() <= typeErr.Pos && typeErr.Pos <= f.End() {
Expand Down Expand Up @@ -203,3 +189,20 @@ func equalTypes(t1, t2 types.Type) bool {
// TODO: Figure out if we want to check for types.AssignableTo(t1, t2) || types.ConvertibleTo(t1, t2)
return false
}

func FixesError(msg string) bool {
matches := wrongReturnNumRegex.FindStringSubmatch(strings.TrimSpace(msg))
if len(matches) < 3 {
return false
}
wantNum, err := strconv.Atoi(matches[1])
if err != nil {
return false
}
gotNum, err := strconv.Atoi(matches[2])
if err != nil {
return false
}
// Logic for handling more return values than expected is hard.
return wantNum >= gotNum
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ var Analyzer = &analysis.Analyzer{
RunDespiteErrors: true,
}

const noNewVarsMsg = "no new variables on left side of :="

func run(pass *analysis.Pass) (interface{}, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
errors := analysisinternal.GetTypeErrors(pass)
Expand All @@ -63,7 +61,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
}

for _, err := range errors {
if err.Msg != noNewVarsMsg {
if !FixesError(err.Msg) {
continue
}
if assignStmt.Pos() > err.Pos || err.Pos >= assignStmt.End() {
Expand All @@ -89,3 +87,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
})
return nil, nil
}

func FixesError(msg string) bool {
return msg == "no new variables on left side of :="
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ var Analyzer = &analysis.Analyzer{
RunDespiteErrors: true,
}

const noResultValuesMsg = "no result values expected"

func run(pass *analysis.Pass) (interface{}, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
errors := analysisinternal.GetTypeErrors(pass)
Expand All @@ -56,7 +54,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
}

for _, err := range errors {
if err.Msg != noResultValuesMsg {
if !FixesError(err.Msg) {
continue
}
if retStmt.Pos() >= err.Pos || err.Pos >= retStmt.End() {
Expand All @@ -83,3 +81,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
})
return nil, nil
}

func FixesError(msg string) bool {
return msg == "no result values expected"
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const undeclaredNamePrefix = "undeclared name: "

func run(pass *analysis.Pass) (interface{}, error) {
for _, err := range analysisinternal.GetTypeErrors(pass) {
if !strings.HasPrefix(err.Msg, undeclaredNamePrefix) {
if !FixesError(err.Msg) {
continue
}
name := strings.TrimPrefix(err.Msg, undeclaredNamePrefix)
Expand Down Expand Up @@ -184,3 +184,7 @@ func baseIfStmt(path []ast.Node, index int) ast.Stmt {
}
return stmt.(ast.Stmt)
}

func FixesError(msg string) bool {
return strings.HasPrefix(msg, undeclaredNamePrefix)
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
errors "golang.org/x/xerrors"
)

func (s *snapshot) Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*source.Error, error) {
func (s *snapshot) Analyze(ctx context.Context, id string, analyzers ...*analysis.Analyzer) ([]*source.Error, error) {
var roots []*actionHandle

for _, a := range analyzers {
Expand Down
2 changes: 1 addition & 1 deletion cmd/govim/internal/golang_org_x_tools/lsp/cache/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
defer done()

cfg := s.Config(ctx)
pkgs, err := s.view.loadPackages(cfg, query...)
pkgs, err := packages.Load(cfg, query...)

// If the context was canceled, return early. Otherwise, we might be
// type-checking an incomplete result. Check the context directly,
Expand Down
15 changes: 7 additions & 8 deletions cmd/govim/internal/golang_org_x_tools/lsp/cache/mod.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/lsp/protocol"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/lsp/source"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/memoize"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/packagesinternal"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/span"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/telemetry/event"
errors "golang.org/x/xerrors"
Expand Down Expand Up @@ -220,7 +221,7 @@ func goModWhy(ctx context.Context, cfg *packages.Config, folder string, data *mo
for _, req := range data.origParsedFile.Require {
inv.Args = append(inv.Args, req.Mod.Path)
}
stdout, err := inv.Run(ctx)
stdout, err := packagesinternal.GetGoCmdRunner(cfg).Run(ctx, inv)
if err != nil {
return err
}
Expand All @@ -247,7 +248,7 @@ func dependencyUpgrades(ctx context.Context, cfg *packages.Config, folder string
Env: cfg.Env,
WorkingDir: folder,
}
stdout, err := inv.Run(ctx)
stdout, err := packagesinternal.GetGoCmdRunner(cfg).Run(ctx, inv)
if err != nil {
return err
}
Expand Down Expand Up @@ -288,6 +289,7 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle)
cfg := s.Config(ctx)
options := s.View().Options()
folder := s.View().Folder().Filename()
gocmdRunner := s.view.gocmdRunner

wsPackages, err := s.WorkspacePackages(ctx)
if ctx.Err() != nil {
Expand Down Expand Up @@ -358,12 +360,9 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle)
Env: cfg.Env,
WorkingDir: folder,
}
if _, err := inv.Run(ctx); err != nil {
// Ignore concurrency errors here.
if !modConcurrencyError.MatchString(err.Error()) {
return &modData{
err: err,
}
if _, err := gocmdRunner.Run(ctx, inv); err != nil {
return &modData{
err: err,
}
}

Expand Down
45 changes: 0 additions & 45 deletions cmd/govim/internal/golang_org_x_tools/lsp/cache/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@
package cache

import (
"context"
"go/ast"
"go/types"

"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/lsp/protocol"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/lsp/source"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/packagesinternal"
"github.com/govim/govim/cmd/govim/internal/golang_org_x_tools/span"
Expand Down Expand Up @@ -125,46 +123,3 @@ func (p *pkg) Imports() []source.Package {
func (p *pkg) Module() *packagesinternal.Module {
return p.module
}

func (s *snapshot) FindAnalysisError(ctx context.Context, pkgID, analyzerName, msg string, rng protocol.Range) (*source.Error, *source.Analyzer, error) {
analyzer := findAnalyzer(s, analyzerName)
if analyzer.Analyzer == nil {
return nil, nil, errors.Errorf("unexpected analyzer: %s", analyzerName)
}
if !analyzer.Enabled(s) {
return nil, nil, errors.Errorf("disabled analyzer: %s", analyzerName)
}
act, err := s.actionHandle(ctx, packageID(pkgID), analyzer.Analyzer)
if err != nil {
return nil, nil, err
}
errs, _, err := act.analyze(ctx)
if err != nil {
return nil, nil, err
}
for _, err := range errs {
if err.Category != analyzer.Analyzer.Name {
continue
}
if err.Message != msg {
continue
}
if protocol.CompareRange(err.Range, rng) != 0 {
continue
}
return err, &analyzer, nil
}
return nil, nil, errors.Errorf("no matching diagnostic for %s:%v", pkgID, analyzerName)
}

func findAnalyzer(s *snapshot, analyzerName string) source.Analyzer {
checked := s.View().Options().DefaultAnalyzers
if a, ok := checked[analyzerName]; ok {
return a
}
checked = s.View().Options().TypeErrorAnalyzers
if a, ok := checked[analyzerName]; ok {
return a
}
return source.Analyzer{}
}

0 comments on commit dd56e8b

Please sign in to comment.