gopls/internal/analysis/unusedfunc: skip std

Too many false positives.

Also, modify the test so that its package has a go.mod
and does not appear to be part of std.

Updates golang/go#71686
Updates golang/go#74130

Change-Id: I471a28d263769d85a46fb90483486845d96d0ea3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/691496
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/doc/analyzers.md

@@ -4434,7 +4434,7 @@ package.
 The tool may report false positives in some situations, for
 example:
 
-  - For a declaration of an unexported function that is referenced
+  - 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.
@@ -4443,17 +4443,19 @@ example:
     annotations, if they must be used at all, should be used on both
     the declaration and the alias.)
 
-  - For compiler intrinsics in the "runtime" package that, though
+  - for compiler intrinsics in the "runtime" package that, though
     never referenced, are known to the compiler and are called
     indirectly by compiled object code.
 
-  - For functions called only from assembly.
+  - for functions called only from assembly.
 
-  - For functions called only from files whose build tags are not
+  - for functions called only from files whose build tags are not
     selected in the current build configuration.
 
-See https://github.com/golang/go/issues/71686 for discussion of
-these limitations.
+Since these situations are relatively common in the low-level parts
+of the runtime, this analyzer ignores the standard library.
+See https://go.dev/issue/71686 and https://go.dev/issue/74130 for
+further discussion of these limitations.
 
 The unusedfunc algorithm is not as precise as the
 golang.org/x/tools/cmd/deadcode tool, but it has the advantage that

gopls/internal/analysis/unusedfunc/doc.go

@@ -23,7 +23,7 @@
 // The tool may report false positives in some situations, for
 // example:
 //
-//   - For a declaration of an unexported function that is referenced
+//   - 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.
@@ -32,17 +32,19 @@
 //     annotations, if they must be used at all, should be used on both
 //     the declaration and the alias.)
 //
-//   - For compiler intrinsics in the "runtime" package that, though
+//   - for compiler intrinsics in the "runtime" package that, though
 //     never referenced, are known to the compiler and are called
 //     indirectly by compiled object code.
 //
-//   - For functions called only from assembly.
+//   - for functions called only from assembly.
 //
-//   - For functions called only from files whose build tags are not
+//   - for functions called only from files whose build tags are not
 //     selected in the current build configuration.
 //
-// See https://github.com/golang/go/issues/71686 for discussion of
-// these limitations.
+// Since these situations are relatively common in the low-level parts
+// of the runtime, this analyzer ignores the standard library.
+// See https://go.dev/issue/71686 and https://go.dev/issue/74130 for
+// further discussion of these limitations.
 //
 // The unusedfunc algorithm is not as precise as the
 // golang.org/x/tools/cmd/deadcode tool, but it has the advantage that

gopls/internal/analysis/unusedfunc/testdata/src/a/a.go renamed to gopls/internal/analysis/unusedfunc/testdata/basic.txtar

@@ -1,3 +1,11 @@
+Basic test of unusedfunc.
+
+-- go.mod --
+module example.com
+
+go 1.21
+
+-- a/a.go --
 package a
 
 func main() {
@@ -70,3 +78,60 @@ const (
 	constOne       = 1
 	unusedConstTwo = constOne // want `const "unusedConstTwo" is unused`
 )
+
+-- a/a.go.golden --
+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() }
+
+
+// -- types without methods --
+
+type ExportedType2 int
+
+// want `type "unusedUnexportedType2" is unused`
+
+type (
+	one int
+)
+
+// -- generic methods --
+
+type g[T any] int
+
+// want `method "method" is unused`
+
+// -- constants --
+
+// want `const "unusedConst" is unused`
+
+const (
+	unusedEnum = iota
+)
+
+const (
+	constOne = 1
+)

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

@@ -67,6 +67,12 @@ var Analyzer = &analysis.Analyzer{
 }
 
 func run(pass *analysis.Pass) (any, error) {
+	// The standard library makes heavy use of intrinsics, linknames, etc,
+	// that confuse this algorithm; so skip it (#74130).
+	if analysisinternal.IsStdPackage(pass.Pkg.Path()) {
+		return nil, nil
+	}
+
 	var (
 		inspect = pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 		index   = pass.ResultOf[typeindexanalyzer.Analyzer].(*typeindex.Index)

gopls/internal/analysis/unusedfunc/unusedfunc.go

@@ -5,13 +5,15 @@
 package unusedfunc_test
 
 import (
+	"path/filepath"
 	"testing"
 
 	"golang.org/x/tools/go/analysis/analysistest"
 	"golang.org/x/tools/gopls/internal/analysis/unusedfunc"
+	"golang.org/x/tools/internal/testfiles"
 )
 
 func Test(t *testing.T) {
-	testdata := analysistest.TestData()
-	analysistest.RunWithSuggestedFixes(t, testdata, unusedfunc.Analyzer, "a")
+	dir := testfiles.ExtractTxtarFileToTmp(t, filepath.Join(analysistest.TestData(), "basic.txtar"))
+	analysistest.RunWithSuggestedFixes(t, dir, unusedfunc.Analyzer, "example.com/a")
 }

gopls/internal/analysis/unusedfunc/unusedfunc_test.go

@@ -1648,7 +1648,7 @@
 						},
 						{
 							"Name": "\"unusedfunc\"",
-							"Doc": "check for unused functions, methods, etc\n\nThe unusedfunc analyzer reports functions and methods that are\nnever referenced outside of their own declaration.\n\nA function is considered unused if it is unexported and not\nreferenced (except within its own declaration).\n\nA method is considered unused if it is unexported, not referenced\n(except within its own declaration), and its name does not match\nthat of any method of an interface type declared within the same\npackage.\n\nThe tool may report false positives in some situations, for\nexample:\n\n  - For a declaration of an unexported function that is referenced\n    from another package using the go:linkname mechanism, if the\n    declaration's doc comment does not also have a go:linkname\n    comment.\n\n    (Such code is in any case strongly discouraged: linkname\n    annotations, if they must be used at all, should be used on both\n    the declaration and the alias.)\n\n  - For compiler intrinsics in the \"runtime\" package that, though\n    never referenced, are known to the compiler and are called\n    indirectly by compiled object code.\n\n  - For functions called only from assembly.\n\n  - For functions called only from files whose build tags are not\n    selected in the current build configuration.\n\nSee https://github.com/golang/go/issues/71686 for discussion of\nthese limitations.\n\nThe unusedfunc algorithm is not as precise as the\ngolang.org/x/tools/cmd/deadcode tool, but it has the advantage that\nit runs within the modular analysis framework, enabling near\nreal-time feedback within gopls.\n\nThe unusedfunc analyzer also reports unused types, vars, and\nconstants. Enums--constants defined with iota--are ignored since\neven the unused values must remain present to preserve the logical\nordering.",
+							"Doc": "check for unused functions, methods, etc\n\nThe unusedfunc analyzer reports functions and methods that are\nnever referenced outside of their own declaration.\n\nA function is considered unused if it is unexported and not\nreferenced (except within its own declaration).\n\nA method is considered unused if it is unexported, not referenced\n(except within its own declaration), and its name does not match\nthat of any method of an interface type declared within the same\npackage.\n\nThe tool may report false positives in some situations, for\nexample:\n\n  - for a declaration of an unexported function that is referenced\n    from another package using the go:linkname mechanism, if the\n    declaration's doc comment does not also have a go:linkname\n    comment.\n\n    (Such code is in any case strongly discouraged: linkname\n    annotations, if they must be used at all, should be used on both\n    the declaration and the alias.)\n\n  - for compiler intrinsics in the \"runtime\" package that, though\n    never referenced, are known to the compiler and are called\n    indirectly by compiled object code.\n\n  - for functions called only from assembly.\n\n  - for functions called only from files whose build tags are not\n    selected in the current build configuration.\n\nSince these situations are relatively common in the low-level parts\nof the runtime, this analyzer ignores the standard library.\nSee https://go.dev/issue/71686 and https://go.dev/issue/74130 for\nfurther discussion of these limitations.\n\nThe unusedfunc algorithm is not as precise as the\ngolang.org/x/tools/cmd/deadcode tool, but it has the advantage that\nit runs within the modular analysis framework, enabling near\nreal-time feedback within gopls.\n\nThe unusedfunc analyzer also reports unused types, vars, and\nconstants. Enums--constants defined with iota--are ignored since\neven the unused values must remain present to preserve the logical\nordering.",
 							"Default": "true",
 							"Status": ""
 						},
@@ -3386,7 +3386,7 @@
 		},
 		{
 			"Name": "unusedfunc",
-			"Doc": "check for unused functions, methods, etc\n\nThe unusedfunc analyzer reports functions and methods that are\nnever referenced outside of their own declaration.\n\nA function is considered unused if it is unexported and not\nreferenced (except within its own declaration).\n\nA method is considered unused if it is unexported, not referenced\n(except within its own declaration), and its name does not match\nthat of any method of an interface type declared within the same\npackage.\n\nThe tool may report false positives in some situations, for\nexample:\n\n  - For a declaration of an unexported function that is referenced\n    from another package using the go:linkname mechanism, if the\n    declaration's doc comment does not also have a go:linkname\n    comment.\n\n    (Such code is in any case strongly discouraged: linkname\n    annotations, if they must be used at all, should be used on both\n    the declaration and the alias.)\n\n  - For compiler intrinsics in the \"runtime\" package that, though\n    never referenced, are known to the compiler and are called\n    indirectly by compiled object code.\n\n  - For functions called only from assembly.\n\n  - For functions called only from files whose build tags are not\n    selected in the current build configuration.\n\nSee https://github.com/golang/go/issues/71686 for discussion of\nthese limitations.\n\nThe unusedfunc algorithm is not as precise as the\ngolang.org/x/tools/cmd/deadcode tool, but it has the advantage that\nit runs within the modular analysis framework, enabling near\nreal-time feedback within gopls.\n\nThe unusedfunc analyzer also reports unused types, vars, and\nconstants. Enums--constants defined with iota--are ignored since\neven the unused values must remain present to preserve the logical\nordering.",
+			"Doc": "check for unused functions, methods, etc\n\nThe unusedfunc analyzer reports functions and methods that are\nnever referenced outside of their own declaration.\n\nA function is considered unused if it is unexported and not\nreferenced (except within its own declaration).\n\nA method is considered unused if it is unexported, not referenced\n(except within its own declaration), and its name does not match\nthat of any method of an interface type declared within the same\npackage.\n\nThe tool may report false positives in some situations, for\nexample:\n\n  - for a declaration of an unexported function that is referenced\n    from another package using the go:linkname mechanism, if the\n    declaration's doc comment does not also have a go:linkname\n    comment.\n\n    (Such code is in any case strongly discouraged: linkname\n    annotations, if they must be used at all, should be used on both\n    the declaration and the alias.)\n\n  - for compiler intrinsics in the \"runtime\" package that, though\n    never referenced, are known to the compiler and are called\n    indirectly by compiled object code.\n\n  - for functions called only from assembly.\n\n  - for functions called only from files whose build tags are not\n    selected in the current build configuration.\n\nSince these situations are relatively common in the low-level parts\nof the runtime, this analyzer ignores the standard library.\nSee https://go.dev/issue/71686 and https://go.dev/issue/74130 for\nfurther discussion of these limitations.\n\nThe unusedfunc algorithm is not as precise as the\ngolang.org/x/tools/cmd/deadcode tool, but it has the advantage that\nit runs within the modular analysis framework, enabling near\nreal-time feedback within gopls.\n\nThe unusedfunc analyzer also reports unused types, vars, and\nconstants. Enums--constants defined with iota--are ignored since\neven the unused values must remain present to preserve the logical\nordering.",
 			"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/unusedfunc",
 			"Default": true
 		},