cmd/govim: fix index out of range bug when flipping back to diagnosti…

…cs (#840)

See the description in the linked issue.

Fixes #680

cmd/govim/diagnostics.go

@@ -86,7 +86,7 @@ func (v *vimstate) diagnostics() *[]types.Diagnostic {
 }
 
 func (v *vimstate) handleDiagnosticsChanged() error {
-	if err := v.updateQuickfix(false); err != nil {
+	if err := v.updateQuickfixWithDiagnostics(false, false); err != nil {
 		return err
 	}
 

cmd/govim/quickfix.go

@@ -23,11 +23,17 @@ func (q quickfixEntry) equalModuloBuffer(q2 quickfixEntry) bool {
 }
 
 func (v *vimstate) quickfixDiagnostics(flags govim.CommandFlags, args ...string) error {
+	wasNotDiagnostics := !v.quickfixIsDiagnostics
 	v.quickfixIsDiagnostics = true
-	return v.updateQuickfix(true)
+	return v.updateQuickfixWithDiagnostics(true, wasNotDiagnostics)
 }
 
-func (v *vimstate) updateQuickfix(force bool) error {
+// updateQuickfixWithDiagnostics updates Vim's quickfix window with the current
+// diagnostics(), respecting config settings that are overridden by force.  The
+// rather clumsily named wasPrevNotDiagnostics can be set to true if it is
+// known to the caller that the quickfix window was previously (to this call)
+// being used for something other than diagnostics, e.g. references.
+func (v *vimstate) updateQuickfixWithDiagnostics(force bool, wasPrevNotDiagnostics bool) error {
 	if !force && (v.config.QuickfixAutoDiagnostics == nil || !*v.config.QuickfixAutoDiagnostics) {
 		return nil
 	}
@@ -60,10 +66,22 @@ func (v *vimstate) updateQuickfix(force bool) error {
 	}
 
 	// Note: indexes are 1-based, hence 0 means "no index"
+	//
+	// If we were previously not showing diagnostics, we default to selection
+	// the first entry. In the future we might want to improve this logic
+	// by stashing the last selected diagnostic when we flip to, for example,
+	// references mode. But for now we keep it simple.
 	newIdx := 0
-	if len(v.lastQuickFixDiagnostics) > 0 {
+	if !wasPrevNotDiagnostics && len(v.lastQuickFixDiagnostics) > 0 {
 		var want qflistWant
 		v.Parse(v.ChannelExpr(`getqflist({"idx":0})`), &want)
+		if want.Idx == 0 {
+			goto NewIndexSet
+		}
+		wantIdx := want.Idx - 1
+		if len(v.lastQuickFixDiagnostics) <= wantIdx {
+			goto NewIndexSet
+		}
 		currFix := v.lastQuickFixDiagnostics[want.Idx-1]
 		for i, f := range fixes {
 			if currFix.equalModuloBuffer(f) {
@@ -72,6 +90,7 @@ func (v *vimstate) updateQuickfix(force bool) error {
 			}
 		}
 	}
+NewIndexSet:
 	v.lastQuickFixDiagnostics = fixes
 	v.BatchStart()
 	v.BatchChannelCall("setqflist", fixes, "r")

cmd/govim/testdata/scenario_bugs/bug_govim_680.txt

@@ -0,0 +1,47 @@
+# Test case that verifies we have a fix for github.com/govim/govim/issues/680
+
+# Open main.go and verify we have one error
+vim ex 'e main.go'
+vimexprwait errors.golden GOVIMTest_getqflist()
+
+# Find references to x then select the second instance
+vim ex 'call cursor(5,5)'
+vim ex 'GOVIMReferences'
+vim ex 'execute \"normal \\<Down>\\<Enter>\"'
+
+# Now flip back to diagnostics mode
+vim ex ':GOVIMQuickfixDiagnostics'
+
+# Assert that we have received no error (Type: 1) or warning (Type: 2) log messages
+# Disabled pending resolution to https://github.com/golang/go/issues/34103
+# errlogmatch -start -count=0 'LogMessage callback: &protocol\.LogMessageParams\{Type:(1|2), Message:".*'
+
+-- go.mod --
+module mod.com
+
+go 1.12
+-- main.go --
+package main
+
+import "fmt"
+
+var x int
+
+func main() {
+	fmt.Printf("%v, %v", x)
+}
+-- errors.golden --
+[
+  {
+    "bufname": "main.go",
+    "col": 2,
+    "lnum": 8,
+    "module": "",
+    "nr": 0,
+    "pattern": "",
+    "text": "Printf format %v reads arg #2, but call has 1 arg",
+    "type": "",
+    "valid": 1,
+    "vcol": 0
+  }
+]

cmd/govim/vimstate.go

@@ -88,7 +88,7 @@ func (v *vimstate) setConfig(args ...json.RawMessage) (interface{}, error) {
 			}
 		} else {
 			// QuickfixAutoDiagnostics is now on
-			if err := v.updateQuickfix(true); err != nil {
+			if err := v.updateQuickfixWithDiagnostics(true, false); err != nil {
 				return nil, fmt.Errorf("failed to update diagnostics: %v", err)
 			}
 		}