gopls/internal/lsp/source: highlight deprecated symbols
This utilizes the code copied from honnef.co/tools/staticcheck.
Unlike staticcheck.CheckDeprecated that assumes analyzers with
`-go` flag specifying the target go version and utilizes structured
knowledge of stdlib deprecated symbols, this analyzer depends
on the facts and surfaces the deprecation notices as written in
the comments. We also simplified implementation during review.
Gopls turns this analyzer's reports to Hint/Deprecated diagnostics.
Editors like VS Code strike out the marked symbols but don't
surface them in the PROBLEMS panel.
Fixes golang/go#40447
Change-Id: I7ac04c6008587e3c71689dec5cb4ec06523b67f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/508508
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 15eb2d9..c6893a6 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -108,6 +108,17 @@
**Enabled by default.**
+## **deprecated**
+
+check for use of deprecated identifiers
+
+The deprecated analyzer looks for deprecated symbols and package imports.
+
+See https://go.dev/wiki/Deprecated to learn about Go's convention
+for documenting and signaling deprecated identifiers.
+
+**Enabled by default.**
+
## **directive**
check Go toolchain directives such as //go:debug
diff --git a/gopls/internal/lsp/analysis/deprecated/deprecated.go b/gopls/internal/lsp/analysis/deprecated/deprecated.go
new file mode 100644
index 0000000..5f7354e
--- /dev/null
+++ b/gopls/internal/lsp/analysis/deprecated/deprecated.go
@@ -0,0 +1,270 @@
+// Copyright 2023 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 deprecated defines an Analyzer that marks deprecated symbols and package imports.
+package deprecated
+
+import (
+ "bytes"
+ "go/ast"
+ "go/format"
+ "go/token"
+ "go/types"
+ "strconv"
+ "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/internal/typeparams"
+)
+
+// TODO(hyangah): use analysisutil.MustExtractDoc.
+var doc = `check for use of deprecated identifiers
+
+The deprecated analyzer looks for deprecated symbols and package imports.
+
+See https://go.dev/wiki/Deprecated to learn about Go's convention
+for documenting and signaling deprecated identifiers.`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "deprecated",
+ Doc: doc,
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: checkDeprecated,
+ FactTypes: []analysis.Fact{(*deprecationFact)(nil)},
+ RunDespiteErrors: true,
+}
+
+// checkDeprecated is a simplified copy of staticcheck.CheckDeprecated.
+func checkDeprecated(pass *analysis.Pass) (interface{}, error) {
+ inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+ deprs, err := collectDeprecatedNames(pass, inspector)
+ if err != nil || (len(deprs.packages) == 0 && len(deprs.objects) == 0) {
+ return nil, err
+ }
+
+ reportDeprecation := func(depr *deprecationFact, node ast.Node) {
+ // TODO(hyangah): staticcheck.CheckDeprecated has more complex logic. Do we need it here?
+ // TODO(hyangah): Scrub depr.Msg. depr.Msg may contain Go comments
+ // markdown syntaxes but LSP diagnostics do not support markdown syntax.
+
+ buf := new(bytes.Buffer)
+ if err := format.Node(buf, pass.Fset, node); err != nil {
+ // This shouldn't happen but let's be conservative.
+ buf.Reset()
+ buf.WriteString("declaration")
+ }
+ pass.ReportRangef(node, "%s is deprecated: %s", buf, depr.Msg)
+ }
+
+ nodeFilter := []ast.Node{(*ast.SelectorExpr)(nil)}
+ inspector.Preorder(nodeFilter, func(node ast.Node) {
+ // Caveat: this misses dot-imported objects
+ sel, ok := node.(*ast.SelectorExpr)
+ if !ok {
+ return
+ }
+
+ obj := pass.TypesInfo.ObjectOf(sel.Sel)
+ if obj_, ok := obj.(*types.Func); ok {
+ obj = typeparams.OriginMethod(obj_)
+ }
+ if obj == nil || obj.Pkg() == nil {
+ // skip invalid sel.Sel.
+ return
+ }
+
+ if obj.Pkg() == pass.Pkg {
+ // A package is allowed to use its own deprecated objects
+ return
+ }
+
+ // A package "foo" has two related packages "foo_test" and "foo.test", for external tests and the package main
+ // generated by 'go test' respectively. "foo_test" can import and use "foo", "foo.test" imports and uses "foo"
+ // and "foo_test".
+
+ if strings.TrimSuffix(pass.Pkg.Path(), "_test") == obj.Pkg().Path() {
+ // foo_test (the external tests of foo) can use objects from foo.
+ return
+ }
+ if strings.TrimSuffix(pass.Pkg.Path(), ".test") == obj.Pkg().Path() {
+ // foo.test (the main package of foo's tests) can use objects from foo.
+ return
+ }
+ if strings.TrimSuffix(pass.Pkg.Path(), ".test") == strings.TrimSuffix(obj.Pkg().Path(), "_test") {
+ // foo.test (the main package of foo's tests) can use objects from foo's external tests.
+ return
+ }
+
+ if depr, ok := deprs.objects[obj]; ok {
+ reportDeprecation(depr, sel)
+ }
+ })
+
+ for _, f := range pass.Files {
+ for _, spec := range f.Imports {
+ var imp *types.Package
+ var obj types.Object
+ if spec.Name != nil {
+ obj = pass.TypesInfo.ObjectOf(spec.Name)
+ } else {
+ obj = pass.TypesInfo.Implicits[spec]
+ }
+ pkgName, ok := obj.(*types.PkgName)
+ if !ok {
+ continue
+ }
+ imp = pkgName.Imported()
+
+ path, err := strconv.Unquote(spec.Path.Value)
+ if err != nil {
+ continue
+ }
+ pkgPath := pass.Pkg.Path()
+ if strings.TrimSuffix(pkgPath, "_test") == path {
+ // foo_test can import foo
+ continue
+ }
+ if strings.TrimSuffix(pkgPath, ".test") == path {
+ // foo.test can import foo
+ continue
+ }
+ if strings.TrimSuffix(pkgPath, ".test") == strings.TrimSuffix(path, "_test") {
+ // foo.test can import foo_test
+ continue
+ }
+ if depr, ok := deprs.packages[imp]; ok {
+ reportDeprecation(depr, spec.Path)
+ }
+ }
+ }
+ return nil, nil
+}
+
+type deprecationFact struct{ Msg string }
+
+func (*deprecationFact) AFact() {}
+func (d *deprecationFact) String() string { return "Deprecated: " + d.Msg }
+
+type deprecatedNames struct {
+ objects map[types.Object]*deprecationFact
+ packages map[*types.Package]*deprecationFact
+}
+
+// collectDeprecatedNames collects deprecated identifiers and publishes
+// them both as Facts and the return value. This is a simplified copy
+// of staticcheck's fact_deprecated analyzer.
+func collectDeprecatedNames(pass *analysis.Pass, ins *inspector.Inspector) (deprecatedNames, error) {
+ extractDeprecatedMessage := func(docs []*ast.CommentGroup) string {
+ for _, doc := range docs {
+ if doc == nil {
+ continue
+ }
+ parts := strings.Split(doc.Text(), "\n\n")
+ for _, part := range parts {
+ if !strings.HasPrefix(part, "Deprecated: ") {
+ continue
+ }
+ alt := part[len("Deprecated: "):]
+ alt = strings.Replace(alt, "\n", " ", -1)
+ return strings.TrimSpace(alt)
+ }
+ }
+ return ""
+ }
+
+ doDocs := func(names []*ast.Ident, docs *ast.CommentGroup) {
+ alt := extractDeprecatedMessage([]*ast.CommentGroup{docs})
+ if alt == "" {
+ return
+ }
+
+ for _, name := range names {
+ obj := pass.TypesInfo.ObjectOf(name)
+ pass.ExportObjectFact(obj, &deprecationFact{alt})
+ }
+ }
+
+ var docs []*ast.CommentGroup
+ for _, f := range pass.Files {
+ docs = append(docs, f.Doc)
+ }
+ if alt := extractDeprecatedMessage(docs); alt != "" {
+ // Don't mark package syscall as deprecated, even though
+ // it is. A lot of people still use it for simple
+ // constants like SIGKILL, and I am not comfortable
+ // telling them to use x/sys for that.
+ if pass.Pkg.Path() != "syscall" {
+ pass.ExportPackageFact(&deprecationFact{alt})
+ }
+ }
+ nodeFilter := []ast.Node{
+ (*ast.GenDecl)(nil),
+ (*ast.FuncDecl)(nil),
+ (*ast.TypeSpec)(nil),
+ (*ast.ValueSpec)(nil),
+ (*ast.File)(nil),
+ (*ast.StructType)(nil),
+ (*ast.InterfaceType)(nil),
+ }
+ ins.Preorder(nodeFilter, func(node ast.Node) {
+ var names []*ast.Ident
+ var docs *ast.CommentGroup
+ switch node := node.(type) {
+ case *ast.GenDecl:
+ switch node.Tok {
+ case token.TYPE, token.CONST, token.VAR:
+ docs = node.Doc
+ for i := range node.Specs {
+ switch n := node.Specs[i].(type) {
+ case *ast.ValueSpec:
+ names = append(names, n.Names...)
+ case *ast.TypeSpec:
+ names = append(names, n.Name)
+ }
+ }
+ default:
+ return
+ }
+ case *ast.FuncDecl:
+ docs = node.Doc
+ names = []*ast.Ident{node.Name}
+ case *ast.TypeSpec:
+ docs = node.Doc
+ names = []*ast.Ident{node.Name}
+ case *ast.ValueSpec:
+ docs = node.Doc
+ names = node.Names
+ case *ast.StructType:
+ for _, field := range node.Fields.List {
+ doDocs(field.Names, field.Doc)
+ }
+ case *ast.InterfaceType:
+ for _, field := range node.Methods.List {
+ doDocs(field.Names, field.Doc)
+ }
+ }
+ if docs != nil && len(names) > 0 {
+ doDocs(names, docs)
+ }
+ })
+
+ // Every identifier is potentially deprecated, so we will need
+ // to look up facts a lot. Construct maps of all facts propagated
+ // to this pass for fast lookup.
+ out := deprecatedNames{
+ objects: map[types.Object]*deprecationFact{},
+ packages: map[*types.Package]*deprecationFact{},
+ }
+ for _, fact := range pass.AllObjectFacts() {
+ out.objects[fact.Object] = fact.Fact.(*deprecationFact)
+ }
+ for _, fact := range pass.AllPackageFacts() {
+ out.packages[fact.Package] = fact.Fact.(*deprecationFact)
+ }
+
+ return out, nil
+}
diff --git a/gopls/internal/lsp/analysis/deprecated/deprecated_test.go b/gopls/internal/lsp/analysis/deprecated/deprecated_test.go
new file mode 100644
index 0000000..0242ef1
--- /dev/null
+++ b/gopls/internal/lsp/analysis/deprecated/deprecated_test.go
@@ -0,0 +1,18 @@
+// Copyright 2023 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 deprecated
+
+import (
+ "testing"
+
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/internal/testenv"
+)
+
+func Test(t *testing.T) {
+ testenv.NeedsGo1Point(t, 19)
+ testdata := analysistest.TestData()
+ analysistest.Run(t, testdata, Analyzer, "a")
+}
diff --git a/gopls/internal/lsp/analysis/deprecated/testdata/src/a/a.go b/gopls/internal/lsp/analysis/deprecated/testdata/src/a/a.go
new file mode 100644
index 0000000..7ffa07d
--- /dev/null
+++ b/gopls/internal/lsp/analysis/deprecated/testdata/src/a/a.go
@@ -0,0 +1,17 @@
+// Copyright 2023 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 usedeprecated
+
+import "io/ioutil" // want "\"io/ioutil\" is deprecated: .*"
+
+func x() {
+ _, _ = ioutil.ReadFile("") // want "ioutil.ReadFile is deprecated: As of Go 1.16, .*"
+ Legacy() // expect no deprecation notice.
+}
+
+// Legacy is deprecated.
+//
+// Deprecated: use X instead.
+func Legacy() {} // want Legacy:"Deprecated: use X instead."
diff --git a/gopls/internal/lsp/analysis/deprecated/testdata/src/a/a_test.go b/gopls/internal/lsp/analysis/deprecated/testdata/src/a/a_test.go
new file mode 100644
index 0000000..bf88d39
--- /dev/null
+++ b/gopls/internal/lsp/analysis/deprecated/testdata/src/a/a_test.go
@@ -0,0 +1,12 @@
+// Copyright 2023 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 usedeprecated
+
+import "testing"
+
+func TestF(t *testing.T) {
+ Legacy() // expect no deprecation notice.
+ x()
+}
diff --git a/gopls/internal/lsp/cache/errors.go b/gopls/internal/lsp/cache/errors.go
index 3b752b4..0e553bb 100644
--- a/gopls/internal/lsp/cache/errors.go
+++ b/gopls/internal/lsp/cache/errors.go
@@ -348,10 +348,11 @@
Message: gobDiag.Message,
Related: related,
SuggestedFixes: fixes,
+ Tags: srcAnalyzer.Tag,
}
// If the fixes only delete code, assume that the diagnostic is reporting dead code.
if onlyDeletions(fixes) {
- diag.Tags = []protocol.DiagnosticTag{protocol.Unnecessary}
+ diag.Tags = append(diag.Tags, protocol.Unnecessary)
}
return diag
}
diff --git a/gopls/internal/lsp/regtest/expectation.go b/gopls/internal/lsp/regtest/expectation.go
index 5e2a4db..ece9aaa 100644
--- a/gopls/internal/lsp/regtest/expectation.go
+++ b/gopls/internal/lsp/regtest/expectation.go
@@ -10,6 +10,7 @@
"sort"
"strings"
+ "github.com/google/go-cmp/cmp"
"golang.org/x/tools/gopls/internal/lsp"
"golang.org/x/tools/gopls/internal/lsp/protocol"
)
@@ -774,3 +775,14 @@
},
}
}
+
+// WithSeverityTags filters to diagnostics whose severity and tags match
+// the given expectation.
+func WithSeverityTags(diagName string, severity protocol.DiagnosticSeverity, tags []protocol.DiagnosticTag) DiagnosticFilter {
+ return DiagnosticFilter{
+ desc: fmt.Sprintf("with diagnostic %q with severity %q and tag %#q", diagName, severity, tags),
+ check: func(_ string, d protocol.Diagnostic) bool {
+ return d.Source == diagName && d.Severity == severity && cmp.Equal(d.Tags, tags)
+ },
+ }
+}
diff --git a/gopls/internal/lsp/source/api_json.go b/gopls/internal/lsp/source/api_json.go
index 45478f2..60103b0 100644
--- a/gopls/internal/lsp/source/api_json.go
+++ b/gopls/internal/lsp/source/api_json.go
@@ -269,6 +269,11 @@
Default: "true",
},
{
+ Name: "\"deprecated\"",
+ Doc: "check for use of deprecated identifiers\n\nThe deprecated analyzer looks for deprecated symbols and package imports.\n\nSee https://go.dev/wiki/Deprecated to learn about Go's convention\nfor documenting and signaling deprecated identifiers.",
+ Default: "true",
+ },
+ {
Name: "\"directive\"",
Doc: "check Go toolchain directives such as //go:debug\n\nThis analyzer checks for problems with known Go toolchain directives\nin all Go source files in a package directory, even those excluded by\n//go:build constraints, and all non-Go source files too.\n\nFor //go:debug (see https://go.dev/doc/godebug), the analyzer checks\nthat the directives are placed only in Go source files, only above the\npackage comment, and only in package main or *_test.go files.\n\nSupport for other known directives may be added in the future.\n\nThis analyzer does not check //go:build, which is handled by the\nbuildtag analyzer.\n",
Default: "true",
@@ -962,6 +967,11 @@
Default: true,
},
{
+ Name: "deprecated",
+ Doc: "check for use of deprecated identifiers\n\nThe deprecated analyzer looks for deprecated symbols and package imports.\n\nSee https://go.dev/wiki/Deprecated to learn about Go's convention\nfor documenting and signaling deprecated identifiers.",
+ Default: true,
+ },
+ {
Name: "directive",
Doc: "check Go toolchain directives such as //go:debug\n\nThis analyzer checks for problems with known Go toolchain directives\nin all Go source files in a package directory, even those excluded by\n//go:build constraints, and all non-Go source files too.\n\nFor //go:debug (see https://go.dev/doc/godebug), the analyzer checks\nthat the directives are placed only in Go source files, only above the\npackage comment, and only in package main or *_test.go files.\n\nSupport for other known directives may be added in the future.\n\nThis analyzer does not check //go:build, which is handled by the\nbuildtag analyzer.\n",
URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/directive",
diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go
index 5a3aabc..267a7d3 100644
--- a/gopls/internal/lsp/source/options.go
+++ b/gopls/internal/lsp/source/options.go
@@ -50,6 +50,7 @@
"golang.org/x/tools/go/analysis/passes/unsafeptr"
"golang.org/x/tools/go/analysis/passes/unusedresult"
"golang.org/x/tools/go/analysis/passes/unusedwrite"
+ "golang.org/x/tools/gopls/internal/lsp/analysis/deprecated"
"golang.org/x/tools/gopls/internal/lsp/analysis/embeddirective"
"golang.org/x/tools/gopls/internal/lsp/analysis/fillreturns"
"golang.org/x/tools/gopls/internal/lsp/analysis/fillstruct"
@@ -1543,6 +1544,7 @@
cgocall.Analyzer.Name: {Analyzer: cgocall.Analyzer, Enabled: true},
composite.Analyzer.Name: {Analyzer: composite.Analyzer, Enabled: true},
copylock.Analyzer.Name: {Analyzer: copylock.Analyzer, Enabled: true},
+ deprecated.Analyzer.Name: {Analyzer: deprecated.Analyzer, Enabled: true, Severity: protocol.SeverityHint, Tag: []protocol.DiagnosticTag{protocol.Deprecated}},
directive.Analyzer.Name: {Analyzer: directive.Analyzer, Enabled: true},
errorsas.Analyzer.Name: {Analyzer: errorsas.Analyzer, Enabled: true},
httpresponse.Analyzer.Name: {Analyzer: httpresponse.Analyzer, Enabled: true},
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index adb244e..4bdb597 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -886,6 +886,10 @@
// Severity is the severity set for diagnostics reported by this
// analyzer. If left unset it defaults to Warning.
Severity protocol.DiagnosticSeverity
+
+ // Tag is extra tags (unnecessary, deprecated, etc) for diagnostics
+ // reported by this analyzer.
+ Tag []protocol.DiagnosticTag
}
func (a *Analyzer) String() string { return a.Analyzer.String() }
diff --git a/gopls/internal/regtest/diagnostics/diagnostics_test.go b/gopls/internal/regtest/diagnostics/diagnostics_test.go
index f9facfc..29a54f1 100644
--- a/gopls/internal/regtest/diagnostics/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics/diagnostics_test.go
@@ -2087,3 +2087,40 @@
}
})
}
+
+// This test demonstrates the deprecated symbol analyzer
+// produces deprecation notices with expected severity and tags.
+func TestDeprecatedAnalysis(t *testing.T) {
+ const src = `
+-- go.mod --
+module example.com
+-- a/a.go --
+package a
+
+import "example.com/b"
+
+func _() {
+ new(b.B).Obsolete() // deprecated
+}
+
+-- b/b.go --
+package b
+
+type B struct{}
+
+// Deprecated: use New instead.
+func (B) Obsolete() {}
+
+func (B) New() {}
+`
+ Run(t, src, func(t *testing.T, env *Env) {
+ env.OpenFile("a/a.go")
+ env.AfterChange(
+ Diagnostics(
+ env.AtRegexp("a/a.go", "new.*Obsolete"),
+ WithMessage("use New instead."),
+ WithSeverityTags("deprecated", protocol.SeverityHint, []protocol.DiagnosticTag{protocol.Deprecated}),
+ ),
+ )
+ })
+}
