mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp: change `go mod vendor` warning into a diagnostic 

This CL uses the approach of a source.ErrorList for the `go mod vendor`
error--rather than a ShowMessageRequest. The suggested fix is a command
that runs `go mod vendor`, so I added a CommandFix to the source.Error
type.

I'm not sure if a diagnostic is better than a ShowMessageRequest--
perhaps it should be both?

Fixes golang/go#41819
Fixes golang/go#42152

Change-Id: I4ba2d7af0233f13583395e871166caed83454d6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/261737
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This commit is contained in:
Rebecca Stambler 2020-10-13 00:50:48 -04:00
parent 582c62ec74
commit 1f28ee6820
10 changed files with 148 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
gopls/internal/regtest/vendor_test.go
View File

@ -4,6 +4,8 @@ import (
"testing"
"golang.org/x/tools/internal/lsp"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/testenv"
)
@ -20,6 +22,7 @@ var Goodbye error
func TestInconsistentVendoring(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
const pkgThatUsesVendoring = `
-- go.mod --
module mod.com
@ -52,15 +55,21 @@ func _() {
// will be executed.
OnceMet(
CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1),
ShowMessageRequest("go mod vendor"),
env.DiagnosticAtRegexp("go.mod", "module mod.com"),
),
)
// Apply the quickfix associated with the diagnostic.
d := &protocol.PublishDiagnosticsParams{}
env.Await(ReadDiagnostics("go.mod", d))
env.ApplyQuickFixes("go.mod", d.Diagnostics)
// Check for file changes when the command completes.
env.Await(CompletedWork(source.CommandVendor.Title, 1))
env.CheckForFileChanges()
// Confirm that there is no longer any inconsistent vendoring.
env.Await(
OnceMet(
CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidChangeWatchedFiles), 1),
DiagnosticAt("a/a1.go", 6, 5),
),
DiagnosticAt("a/a1.go", 6, 5),
)
})
}

7
internal/lsp/cache/load.go vendored
View File

@ -92,8 +92,6 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
ctx, done := event.Start(ctx, "cache.view.load", tag.Query.Of(query))
defer done()
cleanup := func() {}
_, inv, cleanup, err := s.goCommandInvocation(ctx, source.ForTypeChecking, &gocommand.Invocation{
WorkingDir: s.view.rootURI.Filename(),
})
@ -111,11 +109,6 @@ func (s *snapshot) load(ctx context.Context, scopes ...interface{}) error {
return ctx.Err()
}
if err != nil {
// Match on common error messages. This is really hacky, but I'm not sure
// of any better way. This can be removed when golang/go#39164 is resolved.
if strings.Contains(err.Error(), "inconsistent vendoring") {
return source.InconsistentVendoring
}
event.Error(ctx, "go/packages.Load", err, tag.Snapshot.Of(s.ID()), tag.Directory.Of(cfg.Dir), tag.Query.Of(query), tag.PackageCount.Of(len(pkgs)))
} else {
event.Log(ctx, "go/packages.Load", tag.Snapshot.Of(s.ID()), tag.Directory.Of(cfg.Dir), tag.Query.Of(query), tag.PackageCount.Of(len(pkgs)))

47
internal/lsp/cache/mod_tidy.go vendored
View File

@ -69,6 +69,9 @@ func (s *snapshot) ModTidy(ctx context.Context, fh source.FileHandle) (*source.T
}
workspacePkgs, err := s.WorkspacePackages(ctx)
if err != nil {
if tm, ok := s.parseModErrors(ctx, fh, err); ok {
return tm, nil
}
return nil, err
}
importHash, err := hashImports(ctx, workspacePkgs)
@ -160,6 +163,50 @@ func (s *snapshot) ModTidy(ctx context.Context, fh source.FileHandle) (*source.T
return mth.tidy(ctx, s)
}
func (s *snapshot) parseModErrors(ctx context.Context, fh source.FileHandle, err error) (*source.TidiedModule, bool) {
if err == nil {
return nil, false
}
switch {
// Match on common error messages. This is really hacky, but I'm not sure
// of any better way. This can be removed when golang/go#39164 is resolved.
case strings.Contains(err.Error(), "inconsistent vendoring"):
pmf, err := s.ParseMod(ctx, fh)
if err != nil {
return nil, false
}
if pmf.File.Module == nil || pmf.File.Module.Syntax == nil {
return nil, false
}
rng, err := rangeFromPositions(pmf.Mapper, pmf.File.Module.Syntax.Start, pmf.File.Module.Syntax.End)
if err != nil {
return nil, false
}
args, err := source.MarshalArgs(protocol.URIFromSpanURI(fh.URI()))
if err != nil {
return nil, false
}
return &source.TidiedModule{
Parsed: pmf,
Errors: []source.Error{{
URI: fh.URI(),
Range: rng,
Kind: source.ListError,
Message: `Inconsistent vendoring detected. Please re-run "go mod vendor".
See https://github.com/golang/go/issues/39164 for more detail on this issue.`,
SuggestedFixes: []source.SuggestedFix{{
Command: &protocol.Command{
Command: source.CommandVendor.ID(),
Title: source.CommandVendor.Title,
Arguments: args,
},
}},
}},
}, true
}
return nil, false
}
func hashImports(ctx context.Context, wsPackages []source.Package) (string, error) {
results := make(map[string]bool)
var imports []string

25
internal/lsp/cache/snapshot.go vendored
View File

@ -946,6 +946,11 @@ func (s *snapshot) orphanedFileScopes() []interface{} {
if !contains(s.view.session.viewsOf(uri), s.view) {
continue
}
// If the file is not open and is in a vendor directory, don't treat it
// like a workspace package.
if _, ok := fh.(*overlay); !ok && inVendor(uri) {
continue
}
// Don't reload metadata for files we've already deemed unloadable.
if _, ok := s.unloadableFiles[uri]; ok {
continue
@ -970,6 +975,20 @@ func contains(views []*View, view *View) bool {
return false
}
func inVendor(uri span.URI) bool {
toSlash := filepath.ToSlash(uri.Filename())
if !strings.Contains(toSlash, "/vendor/") {
return false
}
// Only packages in _subdirectories_ of /vendor/ are considered vendored
// (/vendor/a/foo.go is vendored, /vendor/foo.go is not).
split := strings.Split(toSlash, "/vendor/")
if len(split) < 2 {
return false
}
return strings.Contains(split[1], "/")
}
func generationName(v *View, snapshotID uint64) string {
return fmt.Sprintf("v%v/%v", v.id, snapshotID)
}
@ -1069,6 +1088,12 @@ func (s *snapshot) clone(ctx context.Context, changes map[span.URI]*fileChange,
}
for uri, change := range changes {
// Maybe reinitialize the view if we see a change in the vendor
// directory.
if inVendor(uri) {
reinitialize = maybeReinit
}
// The original FileHandle for this URI is cached on the snapshot.
originalFH := s.files[uri]

24
internal/lsp/cache/view_test.go vendored
View File

@ -94,3 +94,27 @@ module de
}
}
}
func TestInVendor(t *testing.T) {
for _, tt := range []struct {
path string
inVendor bool
}{
{
path: "foo/vendor/x.go",
inVendor: false,
},
{
path: "foo/vendor/x/x.go",
inVendor: true,
},
{
path: "foo/x.go",
inVendor: false,
},
} {
if got := inVendor(span.URIFromPath(tt.path)); got != tt.inVendor {
t.Errorf("expected %s inVendor %v, got %v", tt.path, tt.inVendor, got)
}
}
}

12
internal/lsp/code_action.go
View File

@ -14,6 +14,7 @@ import (
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/mod"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/span"
@ -486,12 +487,12 @@ func moduleQuickFixes(ctx context.Context, snapshot source.Snapshot, fh source.V
return nil, err
}
}
tidied, err := snapshot.ModTidy(ctx, modFH)
errors, err := mod.ErrorsForMod(ctx, snapshot, modFH)
if err != nil {
return nil, err
}
var quickFixes []protocol.CodeAction
for _, e := range tidied.Errors {
for _, e := range errors {
var diag *protocol.Diagnostic
for _, d := range diagnostics {
if sameDiagnostic(d, e) {
@ -524,6 +525,13 @@ func moduleQuickFixes(ctx context.Context, snapshot source.Snapshot, fh source.V
Edits: edits,
})
}
if fix.Command != nil {
action.Command = &protocol.Command{
Command: fix.Command.Command,
Title: fix.Command.Title,
Arguments: fix.Command.Arguments,
}
}
quickFixes = append(quickFixes, action)
}
}

9
internal/lsp/command.go
View File

@ -55,8 +55,13 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom
}
if unsaved {
switch params.Command {
case source.CommandTest.ID(), source.CommandGenerate.ID(), source.CommandToggleDetails.ID(),
source.CommandAddDependency.ID(), source.CommandUpgradeDependency.ID(), source.CommandRemoveDependency.ID():
case source.CommandTest.ID(),
source.CommandGenerate.ID(),
source.CommandToggleDetails.ID(),
source.CommandAddDependency.ID(),
source.CommandUpgradeDependency.ID(),
source.CommandRemoveDependency.ID(),
source.CommandVendor.ID():
// TODO(PJW): for Toggle, not an error if it is being disabled
err := errors.New("all files must be saved first")
s.showCommandError(ctx, command.Title, err)

54
internal/lsp/diagnostics.go
View File

@ -344,7 +344,7 @@ func errorsToDiagnostic(ctx context.Context, snapshot source.Snapshot, errors []
if err != nil {
return err
}
reports.add(fh.VersionedFileIdentity(), false, diagnostic)
reports.add(fh.VersionedFileIdentity(), true, diagnostic)
}
return nil
}
@ -469,55 +469,5 @@ func (s *Server) handleFatalErrors(ctx context.Context, snapshot source.Snapshot
hasGo = true
return errors.New("done")
})
if !hasGo {
return true
}
// All other workarounds are for errors associated with modules.
if len(snapshot.ModFiles()) == 0 {
return false
}
switch loadErr {
case source.InconsistentVendoring:
item, err := s.client.ShowMessageRequest(ctx, &protocol.ShowMessageRequestParams{
Type: protocol.Error,
Message: `Inconsistent vendoring detected. Please re-run "go mod vendor".
See https://github.com/golang/go/issues/39164 for more detail on this issue.`,
Actions: []protocol.MessageActionItem{
{Title: "go mod vendor"},
},
})
// If the user closes the pop-up, don't show them further errors.
if item == nil {
return true
}
if err != nil {
event.Error(ctx, "go mod vendor ShowMessageRequest failed", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
return true
}
// Right now, we don't have a good way of mapping the error message
// to a specific module, so this will re-run `go mod vendor` in every
// known module with a vendor directory.
// TODO(rstambler): Only re-run `go mod vendor` in the relevant module.
for _, uri := range snapshot.ModFiles() {
// Check that there is a vendor directory in the module before
// running `go mod vendor`.
if info, _ := os.Stat(filepath.Join(filepath.Dir(uri.Filename()), "vendor")); info == nil {
continue
}
if err := s.directGoModCommand(ctx, protocol.URIFromSpanURI(uri), "mod", []string{"vendor"}...); err != nil {
if err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
Type: protocol.Error,
Message: fmt.Sprintf(`"go mod vendor" failed with %v`, err),
}); err != nil {
if err != nil {
event.Error(ctx, "go mod vendor ShowMessage failed", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
}
}
}
}
return true
}
return false
return !hasGo
}

27
internal/lsp/mod/diagnostics.go
View File

@ -26,30 +26,39 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.Vers
return nil, err
}
reports[fh.VersionedFileIdentity()] = []*source.Diagnostic{}
tidied, err := snapshot.ModTidy(ctx, fh)
if err == source.ErrTmpModfileUnsupported {
return nil, nil
}
errors, err := ErrorsForMod(ctx, snapshot, fh)
if err != nil {
return nil, err
}
for _, e := range tidied.Errors {
diag := &source.Diagnostic{
for _, e := range errors {
d := &source.Diagnostic{
Message: e.Message,
Range: e.Range,
Source: e.Category,
}
if e.Category == "syntax" {
diag.Severity = protocol.SeverityError
d.Severity = protocol.SeverityError
} else {
diag.Severity = protocol.SeverityWarning
d.Severity = protocol.SeverityWarning
}
fh, err := snapshot.GetVersionedFile(ctx, e.URI)
if err != nil {
return nil, err
}
reports[fh.VersionedFileIdentity()] = append(reports[fh.VersionedFileIdentity()], diag)
reports[fh.VersionedFileIdentity()] = append(reports[fh.VersionedFileIdentity()], d)
}
}
return reports, nil
}
func ErrorsForMod(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) ([]source.Error, error) {
tidied, err := snapshot.ModTidy(ctx, fh)
if source.IsNonFatalGoModError(err) {
return nil, nil
}
if err != nil {
return nil, err
}
return tidied.Errors, nil
}

3
internal/lsp/source/view.go
View File

@ -576,8 +576,7 @@ func (e *Error) Error() string {
}
var (
InconsistentVendoring = errors.New("inconsistent vendoring")
PackagesLoadError = errors.New("packages.Load error")
PackagesLoadError = errors.New("packages.Load error")
)
// WorkspaceModuleVersion is the nonexistent pseudoversion suffix used in the