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 <rfindley@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 1deea3e..fcd8164 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md
@@ -4434,7 +4434,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. @@ -4443,17 +4443,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
diff --git a/gopls/internal/analysis/unusedfunc/doc.go b/gopls/internal/analysis/unusedfunc/doc.go index c43d9a6..5150002 100644 --- a/gopls/internal/analysis/unusedfunc/doc.go +++ b/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
diff --git a/gopls/internal/analysis/unusedfunc/testdata/src/a/a.go b/gopls/internal/analysis/unusedfunc/testdata/basic.txtar similarity index 60% rename from gopls/internal/analysis/unusedfunc/testdata/src/a/a.go rename to gopls/internal/analysis/unusedfunc/testdata/basic.txtar index 45f5176..0d75067 100644 --- a/gopls/internal/analysis/unusedfunc/testdata/src/a/a.go +++ b/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 @@ 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 +)
diff --git a/gopls/internal/analysis/unusedfunc/testdata/src/a/a.go.golden b/gopls/internal/analysis/unusedfunc/testdata/src/a/a.go.golden deleted file mode 100644 index 4e9c8fb..0000000 --- a/gopls/internal/analysis/unusedfunc/testdata/src/a/a.go.golden +++ /dev/null
@@ -1,55 +0,0 @@ -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 -)
diff --git a/gopls/internal/analysis/unusedfunc/unusedfunc.go b/gopls/internal/analysis/unusedfunc/unusedfunc.go index 40ecbb5..0bf738e 100644 --- a/gopls/internal/analysis/unusedfunc/unusedfunc.go +++ b/gopls/internal/analysis/unusedfunc/unusedfunc.go
@@ -67,6 +67,12 @@ } 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)
diff --git a/gopls/internal/analysis/unusedfunc/unusedfunc_test.go b/gopls/internal/analysis/unusedfunc/unusedfunc_test.go index 1bf73da..db117b1 100644 --- a/gopls/internal/analysis/unusedfunc/unusedfunc_test.go +++ b/gopls/internal/analysis/unusedfunc/unusedfunc_test.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") }
diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json index 65cbbd3..c055763 100644 --- a/gopls/internal/doc/api.json +++ b/gopls/internal/doc/api.json
@@ -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 },