From f4fcf867e710f011999090fe3bef367096314b48 Mon Sep 17 00:00:00 2001 From: Rohan Challa Date: Mon, 16 Mar 2020 14:46:49 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- gopls/doc/analyzers.md | 16 ++++ internal/analysisinternal/analysis.go | 1 + .../analysis/noresultvalues/noresultvalues.go | 85 +++++++++++++++++++ .../noresultvalues/noresultvalues_test.go | 17 ++++ .../noresultvalues/testdata/src/a/a.go | 9 ++ .../noresultvalues/testdata/src/a/a.go.golden | 9 ++ internal/lsp/source/options.go | 2 + .../primarymod/typeerrors/noresultvalues.go | 5 ++ .../typeerrors/noresultvalues.go.golden | 14 +++ internal/lsp/testdata/lsp/summary.txt.golden | 2 +- 10 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 internal/lsp/analysis/noresultvalues/noresultvalues.go create mode 100644 internal/lsp/analysis/noresultvalues/noresultvalues_test.go create mode 100644 internal/lsp/analysis/noresultvalues/testdata/src/a/a.go create mode 100644 internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden create mode 100644 internal/lsp/testdata/lsp/primarymod/typeerrors/noresultvalues.go create mode 100644 internal/lsp/testdata/lsp/primarymod/typeerrors/noresultvalues.go.golden diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 4b6753c913..c2d84982e7 100644 --- a/gopls/doc/analyzers.md +++ b/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 diff --git a/internal/analysisinternal/analysis.go b/internal/analysisinternal/analysis.go index efd7b393ab..1599702fc5 100644 --- a/internal/analysisinternal/analysis.go +++ b/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" ) diff --git a/internal/lsp/analysis/noresultvalues/noresultvalues.go b/internal/lsp/analysis/noresultvalues/noresultvalues.go new file mode 100644 index 0000000000..0259691ed4 --- /dev/null +++ b/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 +} diff --git a/internal/lsp/analysis/noresultvalues/noresultvalues_test.go b/internal/lsp/analysis/noresultvalues/noresultvalues_test.go new file mode 100644 index 0000000000..6b9451bf2c --- /dev/null +++ b/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") +} diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go new file mode 100644 index 0000000000..30265a42f2 --- /dev/null +++ b/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" diff --git a/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden b/internal/lsp/analysis/noresultvalues/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..6b29cefa36 --- /dev/null +++ b/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" diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index aa23f36400..57288e18e1 100644 --- a/internal/lsp/source/options.go +++ b/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}, } } diff --git a/internal/lsp/testdata/lsp/primarymod/typeerrors/noresultvalues.go b/internal/lsp/testdata/lsp/primarymod/typeerrors/noresultvalues.go new file mode 100644 index 0000000000..84234c4b93 --- /dev/null +++ b/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") diff --git a/internal/lsp/testdata/lsp/primarymod/typeerrors/noresultvalues.go.golden b/internal/lsp/testdata/lsp/primarymod/typeerrors/noresultvalues.go.golden new file mode 100644 index 0000000000..07c54d4455 --- /dev/null +++ b/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") + diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden index b74c61ddb8..bea9c712e2 100644 --- a/internal/lsp/testdata/lsp/summary.txt.golden +++ b/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