internal/lsp/analysis: add pass for unused parameters
This change adds a pass that checks for unused parameters inside of a function. It is disabled by default.
Updates golang/go#36602
Change-Id: I9e8de3368f16f27e7816ec4ddb16935e1a05584e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222817
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 77bc4a8..eec9ed8 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -373,4 +373,19 @@
}
```
-Default value: `true`.
\ No newline at end of file
+Default value: `true`.
+
+### **unusedparams**
+
+check for unused parameters of functions
+
+The unusedparams analyzer checks functions to see if there are
+any parameters that are not being used.
+
+To reduce false positives it ignores:
+- methods
+- parameters that do not have a name or are underscored
+- functions in test files
+- functions with empty bodies or those with just a return stmt
+
+Default value: `false`.
diff --git a/internal/lsp/analysis/unusedparams/testdata/src/a/a.go b/internal/lsp/analysis/unusedparams/testdata/src/a/a.go
new file mode 100644
index 0000000..248ecfc
--- /dev/null
+++ b/internal/lsp/analysis/unusedparams/testdata/src/a/a.go
@@ -0,0 +1,55 @@
+// 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 a
+
+import (
+ "bytes"
+ "fmt"
+ "net/http"
+)
+
+type parent interface {
+ n(f bool)
+}
+
+type yuh struct {
+ a int
+}
+
+func (y *yuh) n(f bool) {
+ for i := 0; i < 10; i++ {
+ fmt.Println(i)
+ }
+}
+
+func a(i1 int, i2 int, i3 int) int { // want "potentially unused parameter: 'i2'"
+ i3 += i1
+ _ = func(z int) int { // want "potentially unused parameter: 'z'"
+ _ = 1
+ return 1
+ }
+ return i3
+}
+
+func b(c bytes.Buffer) { // want "potentially unused parameter: 'c'"
+ _ = 1
+}
+
+func z(h http.ResponseWriter, _ *http.Request) { // want "potentially unused parameter: 'h'"
+ fmt.Println("Before")
+}
+
+func l(h http.Handler) http.Handler {
+ return http.HandlerFunc(z)
+}
+
+func mult(a, b int) int { // want "potentially unused parameter: 'b'"
+ a += 1
+ return a
+}
+
+func y(a int) {
+ panic("yo")
+}
diff --git a/internal/lsp/analysis/unusedparams/unusedparams.go b/internal/lsp/analysis/unusedparams/unusedparams.go
new file mode 100644
index 0000000..3b3a605
--- /dev/null
+++ b/internal/lsp/analysis/unusedparams/unusedparams.go
@@ -0,0 +1,146 @@
+// 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 unusedparams defines an analyzer that checks for unused
+// parameters of functions.
+package unusedparams
+
+import (
+ "fmt"
+ "go/ast"
+ "go/types"
+ "strings"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/inspect"
+ "golang.org/x/tools/go/ast/inspector"
+)
+
+const Doc = `check for unused parameters of functions
+
+The unusedparams analyzer checks functions to see if there are
+any parameters that are not being used.
+
+To reduce false positives it ignores:
+- methods
+- parameters that do not have a name or are underscored
+- functions in test files
+- functions with empty bodies or those with just a return stmt`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "unusedparams",
+ Doc: Doc,
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: run,
+}
+
+type paramData struct {
+ field *ast.Field
+ ident *ast.Ident
+ typObj types.Object
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+ inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+ nodeFilter := []ast.Node{
+ (*ast.FuncDecl)(nil),
+ (*ast.FuncLit)(nil),
+ }
+
+ inspect.Preorder(nodeFilter, func(n ast.Node) {
+ var fieldList *ast.FieldList
+ var body *ast.BlockStmt
+
+ // Get the fieldList and body from the function node.
+ switch f := n.(type) {
+ case *ast.FuncDecl:
+ fieldList, body = f.Type.Params, f.Body
+ // TODO(golang/go#36602): add better handling for methods, if we enable methods
+ // we will get false positives if a struct is potentially implementing
+ // an interface.
+ if f.Recv != nil {
+ return
+ }
+ // Ignore functions in _test.go files to reduce false positives.
+ if file := pass.Fset.File(n.Pos()); file != nil && strings.HasSuffix(file.Name(), "_test.go") {
+ return
+ }
+ case *ast.FuncLit:
+ fieldList, body = f.Type.Params, f.Body
+ }
+ // If there are no arguments or the function is empty, then return.
+ if fieldList.NumFields() == 0 || len(body.List) == 0 {
+ return
+ }
+
+ switch expr := body.List[0].(type) {
+ case *ast.ReturnStmt:
+ // Ignore functions that only contain a return statement to reduce false positives.
+ return
+ case *ast.ExprStmt:
+ callExpr, ok := expr.X.(*ast.CallExpr)
+ if !ok || len(body.List) > 1 {
+ break
+ }
+ // Ignore functions that only contain a panic statement to reduce false positives.
+ if fun, ok := callExpr.Fun.(*ast.Ident); ok && fun.Name == "panic" {
+ return
+ }
+ }
+
+ // Get the useful data from each field.
+ params := make(map[string]*paramData)
+ unused := make(map[*paramData]bool)
+ for _, f := range fieldList.List {
+ for _, i := range f.Names {
+ if i.Name == "_" {
+ continue
+ }
+ params[i.Name] = ¶mData{
+ field: f,
+ ident: i,
+ typObj: pass.TypesInfo.ObjectOf(i),
+ }
+ unused[params[i.Name]] = true
+ }
+ }
+
+ // Traverse through the body of the function and
+ // check to see which parameters are unused.
+ ast.Inspect(body, func(node ast.Node) bool {
+ n, ok := node.(*ast.Ident)
+ if !ok {
+ return true
+ }
+ param, ok := params[n.Name]
+ if !ok {
+ return false
+ }
+ if nObj := pass.TypesInfo.ObjectOf(n); nObj != param.typObj {
+ return false
+ }
+ if _, ok := unused[param]; ok {
+ delete(unused, param)
+ }
+ return false
+ })
+
+ // Create the reports for the unused parameters.
+ for u, _ := range unused {
+ start, end := u.field.Pos(), u.field.End()
+ if len(u.field.Names) > 1 {
+ start, end = u.ident.Pos(), u.ident.End()
+ }
+ // TODO(golang/go#36602): add suggested fixes to automatically remove the unused parameter,
+ // to start, just remove it from the function declaration,
+ // later, remove from every use of this function
+ pass.Report(analysis.Diagnostic{
+ Pos: start,
+ End: end,
+ Message: fmt.Sprintf("potentially unused parameter: '%s'", u.ident.Name),
+ })
+ }
+ })
+ return nil, nil
+}
diff --git a/internal/lsp/analysis/unusedparams/unusedparams_test.go b/internal/lsp/analysis/unusedparams/unusedparams_test.go
new file mode 100644
index 0000000..907f71c
--- /dev/null
+++ b/internal/lsp/analysis/unusedparams/unusedparams_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 unusedparams_test
+
+import (
+ "testing"
+
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/internal/lsp/analysis/unusedparams"
+)
+
+func Test(t *testing.T) {
+ testdata := analysistest.TestData()
+ analysistest.Run(t, testdata, unusedparams.Analyzer, "a")
+}
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 363f426..d839313 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -37,6 +37,7 @@
"golang.org/x/tools/go/analysis/passes/unsafeptr"
"golang.org/x/tools/go/analysis/passes/unusedresult"
"golang.org/x/tools/internal/lsp/analysis/simplifyrange"
+ "golang.org/x/tools/internal/lsp/analysis/unusedparams"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/diff/myers"
@@ -510,6 +511,7 @@
deepequalerrors.Analyzer.Name: {Analyzer: deepequalerrors.Analyzer, Enabled: true},
sortslice.Analyzer.Name: {Analyzer: sortslice.Analyzer, Enabled: true},
testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true},
+ unusedparams.Analyzer.Name: {Analyzer: unusedparams.Analyzer, Enabled: false},
// gofmt -s suite:
simplifyrange.Analyzer.Name: {Analyzer: simplifyrange.Analyzer, Enabled: true, HighConfidence: true},
