internal/lsp/analysis: add quickfix for "no result values expected"

This change adds a quick fix for type errors of the type "no result values expected". It will replace the return statment with an empty return statement.

Updates golang/go#34644

Change-Id: I3885748dfc69a2d19f8e7a2e81f36f6d0a20d25b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223666
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

gopls/doc/analyzers.md

@@ -321,6 +321,22 @@ errors is discouraged.
 
 Default value: `true`.
 
+### **noresultvalues**
+
+suggested fixes for "no result values expected"
+
+This checker provides suggested fixes for type errors of the
+type "no result values expected". For example:
+```go
+func z() { return nil }
+```
+will turn into
+```go
+func z() { return }
+```
+
+Default value: `true`.
+
 ### **simplifycompositelit**
 
 check for composite literal simplifications

internal/analysisinternal/analysis.go

@@ -29,5 +29,6 @@ var SetTypeErrors = func(p interface{}, errors []types.Error) {}
 type TypeErrorPass string
 
 const (
+	NoResultValues TypeErrorPass = "noresultvalues"
 	UndeclaredName TypeErrorPass = "undeclaredname"
 )

internal/lsp/analysis/noresultvalues/noresultvalues.go

@@ -0,0 +1,85 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package noresultvalues defines an Analyzer that applies suggested fixes
+// to errors of the type "no result values expected".
+package noresultvalues
+
+import (
+	"bytes"
+	"go/ast"
+	"go/format"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/internal/analysisinternal"
+)
+
+const Doc = `suggested fixes for "no result values expected"
+
+This checker provides suggested fixes for type errors of the
+type "no result values expected". For example:
+	func z() { return nil }
+will turn into
+	func z() { return }
+`
+
+var Analyzer = &analysis.Analyzer{
+	Name:             string(analysisinternal.NoResultValues),
+	Doc:              Doc,
+	Requires:         []*analysis.Analyzer{inspect.Analyzer},
+	Run:              run,
+	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)
+
+	nodeFilter := []ast.Node{(*ast.ReturnStmt)(nil)}
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		retStmt, _ := n.(*ast.ReturnStmt)
+
+		var file *ast.File
+		for _, f := range pass.Files {
+			if f.Pos() <= retStmt.Pos() && retStmt.Pos() < f.End() {
+				file = f
+				break
+			}
+		}
+		if file == nil {
+			return
+		}
+
+		for _, err := range errors {
+			if err.Msg != noResultValuesMsg {
+				continue
+			}
+			if retStmt.Pos() >= err.Pos || err.Pos >= retStmt.End() {
+				continue
+			}
+			var buf bytes.Buffer
+			if err := format.Node(&buf, pass.Fset, file); err != nil {
+				continue
+			}
+			pass.Report(analysis.Diagnostic{
+				Pos:     err.Pos,
+				End:     analysisinternal.TypeErrorEndPos(pass.Fset, buf.Bytes(), err.Pos),
+				Message: err.Msg,
+				SuggestedFixes: []analysis.SuggestedFix{{
+					Message: "Delete return values",
+					TextEdits: []analysis.TextEdit{{
+						Pos:     retStmt.Pos(),
+						End:     retStmt.End(),
+						NewText: []byte("return"),
+					}},
+				}},
+			})
+		}
+	})
+	return nil, nil
+}

internal/lsp/analysis/noresultvalues/noresultvalues_test.go

@@ -0,0 +1,17 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package noresultvalues_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/internal/lsp/analysis/noresultvalues"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.RunWithSuggestedFixes(t, testdata, noresultvalues.Analyzer, "a")
+}

internal/lsp/analysis/noresultvalues/testdata/src/a/a.go

@@ -0,0 +1,9 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package noresultvalues
+
+func x() { return nil } // want "no result values expected"
+
+func y() { return nil, "hello" } // want "no result values expected"

internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden

@@ -0,0 +1,9 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package noresultvalues
+
+func x() { return } // want "no result values expected"
+
+func y() { return } // want "no result values expected"

internal/lsp/source/options.go

@@ -36,6 +36,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/unreachable"
 	"golang.org/x/tools/go/analysis/passes/unsafeptr"
 	"golang.org/x/tools/go/analysis/passes/unusedresult"
+	"golang.org/x/tools/internal/lsp/analysis/noresultvalues"
 	"golang.org/x/tools/internal/lsp/analysis/simplifycompositelit"
 	"golang.org/x/tools/internal/lsp/analysis/simplifyrange"
 	"golang.org/x/tools/internal/lsp/analysis/simplifyslice"
@@ -487,6 +488,7 @@ func (r *OptionResult) setBool(b *bool) {
 
 func typeErrorAnalyzers() map[string]Analyzer {
 	return map[string]Analyzer{
+		noresultvalues.Analyzer.Name: {Analyzer: noresultvalues.Analyzer, Enabled: true},
 		undeclaredname.Analyzer.Name: {Analyzer: undeclaredname.Analyzer, Enabled: true},
 	}
 }

internal/lsp/testdata/lsp/primarymod/typeerrors/noresultvalues.go

@@ -0,0 +1,5 @@
+package typeerrors
+
+func x() { return nil } //@suggestedfix("nil", "quickfix")
+
+func y() { return nil, "hello" } //@suggestedfix("nil", "quickfix")

internal/lsp/testdata/lsp/primarymod/typeerrors/noresultvalues.go.golden

@@ -0,0 +1,14 @@
+-- suggestedfix_noresultvalues_3_19 --
+package typeerrors
+
+func x() { return } //@suggestedfix("nil", "quickfix")
+
+func y() { return nil, "hello" } //@suggestedfix("nil", "quickfix")
+
+-- suggestedfix_noresultvalues_5_19 --
+package typeerrors
+
+func x() { return nil } //@suggestedfix("nil", "quickfix")
+
+func y() { return } //@suggestedfix("nil", "quickfix")
+

internal/lsp/testdata/lsp/summary.txt.golden

@@ -11,7 +11,7 @@ DiagnosticsCount = 43
 FoldingRangesCount = 2
 FormatCount = 6
 ImportCount = 7
-SuggestedFixCount = 4
+SuggestedFixCount = 6
 DefinitionsCount = 45
 TypeDefinitionsCount = 2
 HighlightsCount = 52