cmd/govim: place signs for existing diagnostics in new buffers (#590)

If gopls report diagnostics for files that aren't opened in vim
they won't get any signs placed when they are opened.

A typical use case is when staticcheck report diagnostics for
a file that isn't opened, and the user uses quickfix to jump
directly to the issue (effectively opening a new buffer).

This change updates the internal diagnostics cache when a buffer
is read or closed so existing diagnostics will have signs placed.

cmd/govim/buffer_events.go

@@ -17,6 +17,15 @@ import (
 
 func (v *vimstate) bufReadPost(args ...json.RawMessage) error {
 	nb := v.currentBufferInfo(args[0])
+
+	// If we load a buffer that already had diagnostics reported by gopls, the buffer number must be
+	// updated to ensure that sign placement etc. works.
+	for i, d := range v.diagnosticsCache {
+		if d.Buf == -1 && d.Filename == nb.URI().Filename() {
+			v.diagnosticsCache[i].Buf = nb.Num
+		}
+	}
+
 	if cb, ok := v.buffers[nb.Num]; ok {
 		// reload of buffer, e.v. e!
 		cb.SetContents(nb.Contents())
@@ -36,6 +45,13 @@ func (v *vimstate) bufReadPost(args ...json.RawMessage) error {
 		nb.Version = 1
 	}
 	nb.Listener = v.ParseInt(v.ChannelCall("listener_add", v.Prefix()+string(config.FunctionEnrichDelta), nb.Num))
+
+	if v.placeSigns() {
+		if err := v.redefineSigns(v.diagnostics()); err != nil {
+			v.Logf("failed to update signs for buffer %d: %v", nb.Num, err)
+		}
+	}
+
 	return v.handleBufferEvent(nb)
 }
 
@@ -151,6 +167,17 @@ func (v *vimstate) deleteCurrentBuffer(args ...json.RawMessage) error {
 	if !ok {
 		return fmt.Errorf("tried to remove buffer %v; but we have no record of it", currBufNr)
 	}
+
+	// The diagnosticsCache is updated with -1 (unknown buffer) as bufnr.
+	// We don't want to remove the entries completely here since we want to show them in
+	// the quickfix window. And we don't need to remove existing signs or text properties
+	// either here since they are removed by vim automatically when a buffer is deleted.
+	for i := range v.diagnosticsCache {
+		if v.diagnosticsCache[i].Buf == currBufNr {
+			v.diagnosticsCache[i].Buf = -1
+		}
+	}
+
 	v.ChannelCall("listener_remove", cb.Listener)
 	delete(v.buffers, cb.Num)
 	params := &protocol.DidCloseTextDocumentParams{

cmd/govim/diagnostics.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"io/ioutil"
-	"path/filepath"
 	"sort"
 	"strings"
 
@@ -28,9 +27,6 @@ func (v *vimstate) diagnostics() []types.Diagnostic {
 	v.diagnosticsChanged = false
 	v.rawDiagnosticsLock.Unlock()
 
-	// TODO: this will become fragile at some point
-	cwd := v.ParseString(v.ChannelCall("getcwd"))
-
 	// must be non-nil
 	diags := []types.Diagnostic{}
 
@@ -51,12 +47,6 @@ func (v *vimstate) diagnostics() []types.Diagnostic {
 			// create a temp buffer
 			buf = types.NewBuffer(-1, fn, byts, false)
 		}
-		// make fn relative for reporting purposes
-		fn, err := filepath.Rel(cwd, fn)
-		if err != nil {
-			v.Logf("redefineDiagnostics: failed to call filepath.Rel(%q, %q): %v", cwd, fn, err)
-			continue
-		}
 		for _, d := range lspDiags {
 			s, err := types.PointFromPosition(buf, d.Range.Start)
 			if err != nil {

cmd/govim/main.go

@@ -226,6 +226,7 @@ func (g *govimplugin) Init(gg govim.Govim, errCh chan error) error {
 	g.vimstate.Driver.Govim = gg.Scheduled()
 	g.ChannelEx(`augroup govim`)
 	g.ChannelEx(`augroup END`)
+	g.vimstate.workingDirectory = g.ParseString(g.ChannelCall("getcwd", -1))
 	g.DefineFunction(string(config.FunctionBalloonExpr), []string{}, g.vimstate.balloonExpr)
 	g.DefineAutoCommand("", govim.Events{govim.EventBufUnload}, govim.Patterns{"*.go"}, false, g.vimstate.bufUnload, "eval(expand('<abuf>'))")
 	g.DefineAutoCommand("", govim.Events{govim.EventBufRead, govim.EventBufNewFile}, govim.Patterns{"*.go"}, false, g.vimstate.bufReadPost, exprAutocmdCurrBufInfo)
@@ -336,9 +337,8 @@ func (g *govimplugin) Init(gg govim.Govim, errCh chan error) error {
 		g: g,
 	}
 
-	wd := g.ParseString(g.ChannelCall("getcwd", -1))
 	initParams := &protocol.ParamInitia{}
-	initParams.RootURI = string(span.FileURI(wd))
+	initParams.RootURI = string(span.FileURI(g.vimstate.workingDirectory))
 	initParams.Capabilities.TextDocument.Hover = &protocol.HoverClientCapabilities{
 		ContentFormat: []protocol.MarkupKind{protocol.PlainText},
 	}
@@ -362,7 +362,7 @@ func (g *govimplugin) Init(gg govim.Govim, errCh chan error) error {
 
 	// Temporary fix for the fact that gopls does not yet support watching (via
 	// the client) changed files: https://github.com/golang/go/issues/31553
-	gomodpath, err := goModPath(wd)
+	gomodpath, err := goModPath(g.vimstate.workingDirectory)
 	if err != nil {
 		return fmt.Errorf("failed to derive go.mod path: %v", err)
 	}

cmd/govim/quickfix.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"path/filepath"
+
 	"github.com/govim/govim"
 	"github.com/govim/govim/cmd/govim/internal/types"
 )
@@ -24,8 +26,15 @@ func (v *vimstate) updateQuickfix(diags []types.Diagnostic) error {
 	fixes := []quickfixEntry{}
 
 	for _, d := range diags {
+		// make fn relative for reporting purposes
+		fn, err := filepath.Rel(v.workingDirectory, d.Filename)
+		if err != nil {
+			v.Logf("redefineDiagnostics: failed to call filepath.Rel(%q, %q): %v", v.workingDirectory, fn, err)
+			continue
+		}
+
 		fixes = append(fixes, quickfixEntry{
-			Filename: d.Filename,
+			Filename: fn,
 			Lnum:     d.Range.Start.Line(),
 			Col:      d.Range.Start.Col(),
 			Text:     d.Text,

cmd/govim/references.go

@@ -32,9 +32,6 @@ func (v *vimstate) references(flags govim.CommandFlags, args ...string) error {
 		},
 	}
 
-	// TODO this will become fragile at some point
-	cwd := v.ParseString(v.ChannelCall("getcwd"))
-
 	// must be non-nil
 	locs := []quickfixEntry{}
 
@@ -64,9 +61,9 @@ func (v *vimstate) references(flags govim.CommandFlags, args ...string) error {
 			buf = types.NewBuffer(-1, fn, byts, false)
 		}
 		// make fn relative for reporting purposes
-		fn, err := filepath.Rel(cwd, fn)
+		fn, err := filepath.Rel(v.workingDirectory, fn)
 		if err != nil {
-			v.Logf("references: failed to call filepath.Rel(%q, %q): %v", cwd, fn, err)
+			v.Logf("references: failed to call filepath.Rel(%q, %q): %v", v.workingDirectory, fn, err)
 			continue
 		}
 		p, err := types.PointFromPosition(buf, ref.Range.Start)

cmd/govim/testdata/scenario_default/signs.txt

@@ -65,7 +65,6 @@ cmp stdout placed_twowarnings.golden
 # Disabled pending resolution to https://github.com/golang/go/issues/34103
 # errlogmatch -count=0 'LogMessage callback: &protocol\.LogMessageParams\{Type:(1|2), Message:".*'
 
-
 -- go.mod --
 module mod.com
 

cmd/govim/testdata/scenario_default/signs_existing_diags.txt

@@ -0,0 +1,45 @@
+# Test that signs are placed when opening a file that already has diagnostics.
+vim ex 'e main.go'
+errlogmatch -wait 30s 'sendJSONMsg: .*\"call\",\"s:batchCall\",.*\"sign_placelist\"'
+vim ex 'e other.go'
+errlogmatch -wait 30s 'sendJSONMsg: .*\"call\",\"s:batchCall\",.*\"sign_placelist\"'
+vim -indent expr 'sign_getplaced(\"other.go\", {\"group\": \"*\"})'
+! stderr .+
+cmp stdout placed.golden
+# Disabled pending resolution to https://github.com/golang/go/issues/34103
+# errlogmatch -count=0 'LogMessage callback: &protocol\.LogMessageParams\{Type:(1|2), Message:".*'
+
+-- go.mod --
+module mod.com
+
+-- main.go --
+package main
+
+func main() {
+    var z int
+    z = z
+}
+-- other.go --
+package main
+
+import "fmt"
+
+func foo() {
+    fmt.Printf("%v")
+}
+
+-- placed.golden --
+[
+  {
+    "bufnr": 2,
+    "signs": [
+      {
+        "group": "govim",
+        "id": 1,
+        "lnum": 6,
+        "name": "govimWarnSign",
+        "priority": 12
+      }
+    ]
+  }
+]

cmd/govim/vimstate.go

@@ -70,6 +70,10 @@ type vimstate struct {
 	// currently defined popups (both hidden and visible) and have a lifespan of single
 	// codeAction call.
 	suggestedFixesPopups map[int][]*protocol.WorkspaceEdit
+
+	// working directory (when govim was started)
+	// TODO: handle changes to current working directory during runtime
+	workingDirectory string
 }
 
 func (v *vimstate) setConfig(args ...json.RawMessage) (interface{}, error) {