gopls/internal/analysis/unusedfunc: analyzer for unused funcs/methods

This CL defines a new gopls analyzer that reports unused functions
and methods using a local heuristic suitable for the analysis
framework, delivering some of the value of cmd/deadcode but with
the value of near real-time feedback and gopls integration.

Like unusedparams, it assumes that it is running within gopls'
analysis driver, which always chooses the "widest" package for
a given file. Without this assumption, the additional files
for an in-package test may invalidate the analyzer's findings.

Unfortunately a rather large number of marker tests define
throwaway functions called f that not trigger a diagnostic.
They have been updated to finesse the problem.

+ test, doc, relnote

Change-Id: I85ef593eee7a6940779ee27a2455d9090a3e8c7c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/639716
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

gopls/doc/analyzers.md

@@ -903,6 +903,37 @@ Default: on.
 
 Package documentation: [unsafeptr](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/unsafeptr)
 
+<a id='unusedfunc'></a>
+## `unusedfunc`: check for unused functions and methods
+
+
+The unusedfunc analyzer reports functions and methods that are
+never referenced outside of their own declaration.
+
+A function is considered unused if it is unexported and not
+referenced (except within its own declaration).
+
+A method is considered unused if it is unexported, not referenced
+(except within its own declaration), and its name does not match
+that of any method of an interface type declared within the same
+package.
+
+The tool may report a false positive for a declaration of an
+unexported function that is referenced from another package using
+the go:linkname mechanism, if the declaration's doc comment does
+not also have a go:linkname comment. (Such code is in any case
+strongly discouraged: linkname annotations, if they must be used at
+all, should be used on both the declaration and the alias.)
+
+The unusedfunc algorithm is not as precise as the
+golang.org/x/tools/cmd/deadcode tool, but it has the advantage that
+it runs within the modular analysis framework, enabling near
+real-time feedback within gopls.
+
+Default: on.
+
+Package documentation: [unusedfunc](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/unusedfunc)
+
 <a id='unusedparams'></a>
 ## `unusedparams`: check for unused parameters of functions
 

gopls/doc/release/v0.18.0.md

@@ -22,7 +22,7 @@ checks.
 
 ## New `modernize` analyzer
 
-Gopls will now report when code could be simplified or clarified by
+Gopls now reports when code could be simplified or clarified by
 using more modern features of Go, and provides a quick fix to apply
 the change.
 
@@ -31,6 +31,14 @@ Examples:
 - replacement of conditional assignment using an if/else statement by
   a call to the `min` or `max` built-in functions added in Go 1.18;
 
+## New `unusedfunc` analyzer
+
+Gopls now reports unused functions and methods, giving you near
+real-time feedback about dead code that may be safely deleted.
+Because the analysis is local to each package, only unexported
+functions and methods are candidates.
+(For a more precise analysis that may report unused exported
+functions too, use the `golang.org/x/tools/cmd/deadcode` command.)
 
 ## "Implementations" supports generics
 

gopls/internal/analysis/unusedfunc/doc.go

@@ -0,0 +1,34 @@
+// Copyright 2024 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 unusedfunc defines an analyzer that checks for unused
+// functions and methods
+//
+// # Analyzer unusedfunc
+//
+// unusedfunc: check for unused functions and methods
+//
+// The unusedfunc analyzer reports functions and methods that are
+// never referenced outside of their own declaration.
+//
+// A function is considered unused if it is unexported and not
+// referenced (except within its own declaration).
+//
+// A method is considered unused if it is unexported, not referenced
+// (except within its own declaration), and its name does not match
+// that of any method of an interface type declared within the same
+// package.
+//
+// The tool may report a false positive for a declaration of an
+// unexported function that is referenced from another package using
+// the go:linkname mechanism, if the declaration's doc comment does
+// not also have a go:linkname comment. (Such code is in any case
+// strongly discouraged: linkname annotations, if they must be used at
+// all, should be used on both the declaration and the alias.)
+//
+// The unusedfunc algorithm is not as precise as the
+// golang.org/x/tools/cmd/deadcode tool, but it has the advantage that
+// it runs within the modular analysis framework, enabling near
+// real-time feedback within gopls.
+package unusedfunc

gopls/internal/analysis/unusedfunc/main.go

@@ -0,0 +1,15 @@
+// Copyright 2024 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.
+
+//go:build ignore
+
+// The unusedfunc command runs the unusedfunc analyzer.
+package main
+
+import (
+	"golang.org/x/tools/go/analysis/singlechecker"
+	"golang.org/x/tools/gopls/internal/analysis/unusedfunc"
+)
+
+func main() { singlechecker.Main(unusedfunc.Analyzer) }

gopls/internal/analysis/unusedfunc/testdata/src/a/a.go

@@ -0,0 +1,41 @@
+package a
+
+func main() {
+	_ = live
+}
+
+// -- functions --
+
+func Exported() {}
+
+func dead() { // want `function "dead" is unused`
+}
+
+func deadRecursive() int { // want `function "deadRecursive" is unused`
+	return deadRecursive()
+}
+
+func live() {}
+
+//go:linkname foo
+func apparentlyDeadButHasPrecedingLinknameComment() {}
+
+// -- methods --
+
+type ExportedType int
+type unexportedType int
+
+func (ExportedType) Exported()   {}
+func (unexportedType) Exported() {}
+
+func (x ExportedType) dead() { // want `method "dead" is unused`
+	x.dead()
+}
+
+func (u unexportedType) dead() { // want `method "dead" is unused`
+	u.dead()
+}
+
+func (x ExportedType) dynamic() {} // matches name of interface method => live
+
+type _ interface{ dynamic() }

gopls/internal/analysis/unusedfunc/testdata/src/a/a.go.golden

@@ -0,0 +1,26 @@
+package a
+
+func main() {
+	_ = live
+}
+
+// -- functions --
+
+func Exported() {}
+
+func live() {}
+
+//go:linkname foo
+func apparentlyDeadButHasPrecedingLinknameComment() {}
+
+// -- methods --
+
+type ExportedType int
+type unexportedType int
+
+func (ExportedType) Exported()   {}
+func (unexportedType) Exported() {}
+
+func (x ExportedType) dynamic() {} // matches name of interface method => live
+
+type _ interface{ dynamic() }

gopls/internal/analysis/unusedfunc/unusedfunc.go

@@ -0,0 +1,183 @@
+// Copyright 2024 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 unusedfunc
+
+import (
+	_ "embed"
+	"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"
+	"golang.org/x/tools/gopls/internal/util/astutil"
+	"golang.org/x/tools/internal/analysisinternal"
+)
+
+// Assumptions
+//
+// Like unusedparams, this analyzer depends on the invariant of the
+// gopls analysis driver that only the "widest" package (the one with
+// the most files) for a given file is analyzed. This invariant allows
+// the algorithm to make "closed world" assumptions about the target
+// package. (In general, analysis of Go test packages cannot make that
+// assumption because in-package tests add new files to existing
+// packages, potentially invalidating results.) Consequently, running
+// this analyzer in, say, unitchecker or multichecker may produce
+// incorrect results.
+//
+// A function is unreferenced if it is never referenced except within
+// its own declaration, and it is unexported. (Exported functions must
+// be assumed to be referenced from other packages.)
+//
+// For methods, we assume that the receiver type is "live" (variables
+// of that type are created) and "address taken" (its rtype ends up in
+// an at least one interface value). This means exported methods may
+// be called via reflection or by interfaces defined in other
+// packages, so again we are concerned only with unexported methods.
+//
+// To discount the possibility of a method being called via an
+// interface, we must additionally ensure that no literal interface
+// type within the package has a method of the same name.
+// (Unexported methods cannot be called through interfaces declared
+// in other packages because each package has a private namespace
+// for unexported identifiers.)
+
+//go:embed doc.go
+var doc string
+
+var Analyzer = &analysis.Analyzer{
+	Name:     "unusedfunc",
+	Doc:      analysisinternal.MustExtractDoc(doc, "unusedfunc"),
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+	URL:      "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/unusedfunc",
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+	// Gather names of unexported interface methods declared in this package.
+	localIfaceMethods := make(map[string]bool)
+	nodeFilter := []ast.Node{(*ast.InterfaceType)(nil)}
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		iface := n.(*ast.InterfaceType)
+		for _, field := range iface.Methods.List {
+			if len(field.Names) > 0 {
+				id := field.Names[0]
+				if !id.IsExported() {
+					// TODO(adonovan): check not just name but signature too.
+					localIfaceMethods[id.Name] = true
+				}
+			}
+		}
+	})
+
+	// Map each function/method symbol to its declaration.
+	decls := make(map[*types.Func]*ast.FuncDecl)
+	for _, file := range pass.Files {
+		if ast.IsGenerated(file) {
+			continue // skip generated files
+		}
+
+		for _, decl := range file.Decls {
+			if decl, ok := decl.(*ast.FuncDecl); ok {
+				id := decl.Name
+				// Exported functions may be called from other packages.
+				if id.IsExported() {
+					continue
+				}
+
+				// Blank functions are exempt from diagnostics.
+				if id.Name == "_" {
+					continue
+				}
+
+				// An (unexported) method whose name matches an
+				// interface method declared in the same package
+				// may be dynamically called via that interface.
+				if decl.Recv != nil && localIfaceMethods[id.Name] {
+					continue
+				}
+
+				// main and init functions are implicitly always used
+				if decl.Recv == nil && (id.Name == "init" || id.Name == "main") {
+					continue
+				}
+
+				fn := pass.TypesInfo.Defs[id].(*types.Func)
+				decls[fn] = decl
+			}
+		}
+	}
+
+	// Scan for uses of each function symbol.
+	// (Ignore uses within the function's body.)
+	use := func(ref ast.Node, obj types.Object) {
+		if fn, ok := obj.(*types.Func); ok {
+			if fn := fn.Origin(); fn.Pkg() == pass.Pkg {
+				if decl, ok := decls[fn]; ok {
+					// Ignore uses within the function's body.
+					if decl.Body != nil && astutil.NodeContains(decl.Body, ref.Pos()) {
+						return
+					}
+					delete(decls, fn) // symbol is referenced
+				}
+			}
+		}
+	}
+	for id, obj := range pass.TypesInfo.Uses {
+		use(id, obj)
+	}
+	for sel, seln := range pass.TypesInfo.Selections {
+		use(sel, seln.Obj())
+	}
+
+	// Report the remaining unreferenced symbols.
+nextDecl:
+	for fn, decl := range decls {
+		noun := "function"
+		if decl.Recv != nil {
+			noun = "method"
+		}
+
+		pos := decl.Pos() // start of func decl or associated comment
+		if decl.Doc != nil {
+			pos = decl.Doc.Pos()
+
+			// Skip if there's a preceding //go:linkname directive.
+			//
+			// (A program can link fine without such a directive,
+			// but it is bad style; and the directive may
+			// appear anywhere, not just on the preceding line,
+			// but again that is poor form.)
+			//
+			// TODO(adonovan): use ast.ParseDirective when #68021 lands.
+			for _, comment := range decl.Doc.List {
+				if strings.HasPrefix(comment.Text, "//go:linkname ") {
+					continue nextDecl
+				}
+			}
+		}
+
+		pass.Report(analysis.Diagnostic{
+			Pos:     decl.Name.Pos(),
+			End:     decl.Name.End(),
+			Message: fmt.Sprintf("%s %q is unused", noun, fn.Name()),
+			SuggestedFixes: []analysis.SuggestedFix{{
+				Message: fmt.Sprintf("Delete %s %q", noun, fn.Name()),
+				TextEdits: []analysis.TextEdit{{
+					// delete declaration
+					Pos: pos,
+					End: decl.End(),
+				}},
+			}},
+		})
+	}
+
+	return nil, nil
+}

gopls/internal/analysis/unusedfunc/unusedfunc_test.go

@@ -0,0 +1,17 @@
+// Copyright 2024 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 unusedfunc_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/gopls/internal/analysis/unusedfunc"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.RunWithSuggestedFixes(t, testdata, unusedfunc.Analyzer, "a")
+}