internal/lsp: fix a few staticcheck suggestions, some cleanup
There were a few merge conflict-related issues in the GC optimization
details CL. Also fixed a few things I noticed after the fact, like
separating out a new mutex.
Staticcheck caught a few things, and I also fixed a bug I noticed
in the cache package.
Change-Id: I3fc519373253418586dca08fdec3114b30a247ea
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245399
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index 9476efa..099457f 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -351,6 +351,9 @@
dep, version := info[0], info[2]
// Make sure that the format matches our expectation.
+ if len(version) < 2 {
+ continue
+ }
if version[0] != '[' || version[len(version)-1] != ']' {
continue
}
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index ebe440e..66321fb 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -40,9 +40,11 @@
)
// Declare explicit types for files and directories to distinguish between the two.
-type fileURI span.URI
-type directoryURI span.URI
-type viewLoadScope span.URI
+type (
+ fileURI span.URI
+ directoryURI span.URI
+ viewLoadScope span.URI
+)
func (p *pkg) ID() string {
return string(p.m.id)
@@ -114,9 +116,9 @@
return nil, errors.Errorf("no imported package for %s", pkgPath)
}
-func (pkg *pkg) MissingDependencies() []string {
+func (p *pkg) MissingDependencies() []string {
var md []string
- for i := range pkg.m.missingDeps {
+ for i := range p.m.missingDeps {
md = append(md, string(i))
}
return md
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 5632398..2b51dd7 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -145,23 +145,21 @@
}
err := s.directGoModCommand(ctx, uri, "get", goCmdArgs...)
return nil, err
- default:
- return nil, fmt.Errorf("unknown command: %s", params.Command)
case source.CommandToggleDetails:
var fileURI span.URI
if err := source.UnmarshalArgs(params.Arguments, &fileURI); err != nil {
return nil, err
}
pkgDir := span.URI(path.Dir(fileURI.Filename()))
- s.deliveredMu.Lock()
- if s.gcOptimizatonDetails[pkgDir] {
+ s.gcOptimizationDetailsMu.Lock()
+ if _, ok := s.gcOptimizatonDetails[pkgDir]; ok {
delete(s.gcOptimizatonDetails, pkgDir)
} else {
- s.gcOptimizatonDetails[pkgDir] = true
+ s.gcOptimizatonDetails[pkgDir] = struct{}{}
}
+ s.gcOptimizationDetailsMu.Unlock()
event.Log(ctx, fmt.Sprintf("gc_details %s now %v %v", pkgDir, s.gcOptimizatonDetails[pkgDir],
s.gcOptimizatonDetails))
- s.deliveredMu.Unlock()
// need to recompute diagnostics.
// so find the snapshot
sv, err := s.session.ViewOf(fileURI)
@@ -170,6 +168,8 @@
}
s.diagnoseSnapshot(sv.Snapshot())
return nil, nil
+ default:
+ return nil, fmt.Errorf("unknown command: %s", params.Command)
}
return nil, nil
}
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 873413b..ef7aec6 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -117,32 +117,24 @@
go func(pkg source.Package) {
defer wg.Done()
- detailsDir := ""
- // Only run analyses for packages with open files.
- withAnalysis := alwaysAnalyze
+ withAnalysis := alwaysAnalyze // only run analyses for packages with open files
+ var gcDetailsDir span.URI // find the package's optimization details, if available
for _, pgf := range pkg.CompiledGoFiles() {
if snapshot.IsOpen(pgf.URI) {
withAnalysis = true
}
- if detailsDir == "" {
- dir := filepath.Dir(pgf.URI.Filename())
- if s.gcOptimizatonDetails[span.URI(dir)] {
- detailsDir = dir
+ if gcDetailsDir == "" {
+ dirURI := span.URIFromPath(filepath.Dir(pgf.URI.Filename()))
+ s.gcOptimizationDetailsMu.Lock()
+ _, ok := s.gcOptimizatonDetails[dirURI]
+ s.gcOptimizationDetailsMu.Unlock()
+ if ok {
+ gcDetailsDir = dirURI
}
}
}
pkgReports, warn, err := source.Diagnostics(ctx, snapshot, pkg, withAnalysis)
- if detailsDir != "" {
- var more map[source.FileIdentity][]*source.Diagnostic
- more, err = source.DoGcDetails(ctx, snapshot, detailsDir)
- if err != nil {
- event.Error(ctx, "warning: gcdetails", err, tag.Snapshot.Of(snapshot.ID()))
- }
- for k, v := range more {
- pkgReports[k] = append(pkgReports[k], v...)
- }
- }
// Check if might want to warn the user about their build configuration.
// Our caller decides whether to send the message.
@@ -171,6 +163,26 @@
reports[key][diagnosticKey(d)] = d
}
}
+ // If gc optimization details are available, add them to the
+ // diagnostic reports.
+ if gcDetailsDir != "" {
+ gcReports, err := source.GCOptimizationDetails(ctx, snapshot, gcDetailsDir)
+ if err != nil {
+ event.Error(ctx, "warning: gc details", err, tag.Snapshot.Of(snapshot.ID()))
+ }
+ for id, diags := range gcReports {
+ key := idWithAnalysis{
+ id: id,
+ withAnalysis: withAnalysis,
+ }
+ if _, ok := reports[key]; !ok {
+ reports[key] = map[string]*source.Diagnostic{}
+ }
+ for _, d := range diags {
+ reports[key][diagnosticKey(d)] = d
+ }
+ }
+ }
reportsMu.Unlock()
}(pkg)
}
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index a362c03..5fbb757 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -24,7 +24,7 @@
func NewServer(session source.Session, client protocol.Client) *Server {
return &Server{
delivered: make(map[span.URI]sentDiagnostics),
- gcOptimizatonDetails: make(map[span.URI]bool),
+ gcOptimizatonDetails: make(map[span.URI]struct{}),
session: session,
client: client,
diagnosticsSema: make(chan struct{}, concurrentAnalyses),
@@ -74,9 +74,11 @@
deliveredMu sync.Mutex
delivered map[span.URI]sentDiagnostics
- // gcOptimizationDetails describes which packages we want optimization details
- // included in the diagnostics. The key is the directory of the package.
- gcOptimizatonDetails map[span.URI]bool
+ // gcOptimizationDetails describes the packages for which we want
+ // optimization details to be included in the diagnostics. The key is the
+ // directory of the package.
+ gcOptimizationDetailsMu sync.Mutex
+ gcOptimizatonDetails map[span.URI]struct{}
// diagnosticsSema limits the concurrency of diagnostics runs, which can be expensive.
diagnosticsSema chan struct{}
diff --git a/internal/lsp/source/code_lens.go b/internal/lsp/source/code_lens.go
index b0f146f..5a633ee 100644
--- a/internal/lsp/source/code_lens.go
+++ b/internal/lsp/source/code_lens.go
@@ -225,6 +225,9 @@
func toggleDetailsCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle) ([]protocol.CodeLens, error) {
_, pgf, err := getParsedFile(ctx, snapshot, fh, WidestPackage)
+ if err != nil {
+ return nil, err
+ }
fset := snapshot.View().Session().Cache().FileSet()
rng, err := newMappedRange(fset, pgf.Mapper, pgf.File.Package, pgf.File.Package).Range()
if err != nil {
diff --git a/internal/lsp/source/gc_annotations.go b/internal/lsp/source/gc_annotations.go
index 4d6e028..3dd8be8 100644
--- a/internal/lsp/source/gc_annotations.go
+++ b/internal/lsp/source/gc_annotations.go
@@ -18,12 +18,12 @@
"golang.org/x/tools/internal/span"
)
-func DoGcDetails(ctx context.Context, snapshot Snapshot, pkgDir string) (map[FileIdentity][]*Diagnostic, error) {
+func GCOptimizationDetails(ctx context.Context, snapshot Snapshot, pkgDir span.URI) (map[FileIdentity][]*Diagnostic, error) {
outDir := filepath.Join(os.TempDir(), fmt.Sprintf("gopls-%d.details", os.Getpid()))
if err := os.MkdirAll(outDir, 0700); err != nil {
return nil, err
}
- args := []string{fmt.Sprintf("-gcflags=-json=0,%s", outDir), pkgDir}
+ args := []string{fmt.Sprintf("-gcflags=-json=0,%s", outDir), pkgDir.Filename()}
err := snapshot.RunGoCommandDirect(ctx, "build", args)
if err != nil {
return nil, err
