diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 15eb2d957a8..c6893a6be3a 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -108,6 +108,17 @@ errors is discouraged. **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 00000000000..5f7354e4fa4 --- /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 00000000000..0242ef1fa09 --- /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 00000000000..7ffa07dc517 --- /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 00000000000..bf88d395b00 --- /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 3b752b4756c..0e553bbba73 100644 --- a/gopls/internal/lsp/cache/errors.go +++ b/gopls/internal/lsp/cache/errors.go @@ -348,10 +348,11 @@ func toSourceDiagnostic(srcAnalyzer *source.Analyzer, gobDiag *gobDiagnostic) *s 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 5e2a4db21de..ece9aaae4ec 100644 --- a/gopls/internal/lsp/regtest/expectation.go +++ b/gopls/internal/lsp/regtest/expectation.go @@ -10,6 +10,7 @@ import ( "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 @@ func WithMessage(substring string) DiagnosticFilter { }, } } + +// 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 45478f24f43..60103b0a1b2 100644 --- a/gopls/internal/lsp/source/api_json.go +++ b/gopls/internal/lsp/source/api_json.go @@ -268,6 +268,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "check for calls of reflect.DeepEqual on error values\n\nThe deepequalerrors checker looks for calls of the form:\n\n reflect.DeepEqual(err1, err2)\n\nwhere err1 and err2 are errors. Using reflect.DeepEqual to compare\nerrors is discouraged.", 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", @@ -961,6 +966,11 @@ var GeneratedAPIJSON = &APIJSON{ URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/deepequalerrors", 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", diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go index 5a3aabc3de7..267a7d3a47f 100644 --- a/gopls/internal/lsp/source/options.go +++ b/gopls/internal/lsp/source/options.go @@ -50,6 +50,7 @@ import ( "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 @@ func defaultAnalyzers() map[string]*Analyzer { 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 adb244efef5..4bdb59714f4 100644 --- a/gopls/internal/lsp/source/view.go +++ b/gopls/internal/lsp/source/view.go @@ -886,6 +886,10 @@ type Analyzer struct { // 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 f9facfcd300..29a54f1ce61 100644 --- a/gopls/internal/regtest/diagnostics/diagnostics_test.go +++ b/gopls/internal/regtest/diagnostics/diagnostics_test.go @@ -2087,3 +2087,40 @@ var _ = 1 / "" // type error } }) } + +// 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}), + ), + ) + }) +}