From 284795867f51b93ce02c14e8f78f040f94297e71 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Wed, 22 Sep 2021 11:06:05 -0400 Subject: [PATCH] internal/lsp/analysis: add a useany analyzer Add an analyzer that checks for empty interfaces in constraint position, that could instead use the new predeclared "any" type. Change-Id: I6c11f74c479c2cba64b3b12e61d70d157f94393b Reviewed-on: https://go-review.googlesource.com/c/tools/+/351549 Trust: Robert Findley Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Rebecca Stambler --- gopls/doc/analyzers.md | 6 ++ .../lsp/analysis/useany/testdata/src/a/a.go | 25 +++++ .../useany/testdata/src/a/a.go.golden | 25 +++++ internal/lsp/analysis/useany/useany.go | 102 ++++++++++++++++++ internal/lsp/analysis/useany/useany_test.go | 21 ++++ internal/lsp/source/api_json.go | 10 ++ internal/lsp/source/options.go | 2 + internal/typeparams/typeparams_go117.go | 8 +- internal/typeparams/typeparams_go118.go | 17 +-- 9 files changed, 205 insertions(+), 11 deletions(-) create mode 100644 internal/lsp/analysis/useany/testdata/src/a/a.go create mode 100644 internal/lsp/analysis/useany/testdata/src/a/a.go.golden create mode 100644 internal/lsp/analysis/useany/useany.go create mode 100644 internal/lsp/analysis/useany/useany_test.go diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index c3e19aca8d..8215f1bb80 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -554,6 +554,12 @@ Another example is about non-pointer receiver: **Disabled by default. Enable it by setting `"analyses": {"unusedwrite": true}`.** +## **useany** + +check for constraints that could be simplified to "any" + +**Enabled by default.** + ## **fillreturns** suggested fixes for "wrong number of return values (want %d, got %d)" diff --git a/internal/lsp/analysis/useany/testdata/src/a/a.go b/internal/lsp/analysis/useany/testdata/src/a/a.go new file mode 100644 index 0000000000..22d6931507 --- /dev/null +++ b/internal/lsp/analysis/useany/testdata/src/a/a.go @@ -0,0 +1,25 @@ +// Copyright 2021 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. + +// This file contains tests for the useany checker. + +package a + +type Any interface{} + +func _[T interface{}]() {} // want "could use \"any\" for this empty interface" +func _[X any, T interface{}]() {} // want "could use \"any\" for this empty interface" +func _[any interface{}]() {} // want "could use \"any\" for this empty interface" +func _[T Any]() {} // want "could use \"any\" for this empty interface" +func _[T interface{ int | interface{} }]() {} // want "could use \"any\" for this empty interface" +func _[T interface{ int | Any }]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} + +type _[T interface{}] int // want "could use \"any\" for this empty interface" +type _[X any, T interface{}] int // want "could use \"any\" for this empty interface" +type _[any interface{}] int // want "could use \"any\" for this empty interface" +type _[T Any] int // want "could use \"any\" for this empty interface" +type _[T interface{ int | interface{} }] int // want "could use \"any\" for this empty interface" +type _[T interface{ int | Any }] int // want "could use \"any\" for this empty interface" +type _[T any] int diff --git a/internal/lsp/analysis/useany/testdata/src/a/a.go.golden b/internal/lsp/analysis/useany/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..efd8fd640a --- /dev/null +++ b/internal/lsp/analysis/useany/testdata/src/a/a.go.golden @@ -0,0 +1,25 @@ +// Copyright 2021 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. + +// This file contains tests for the useany checker. + +package a + +type Any interface{} + +func _[T any]() {} // want "could use \"any\" for this empty interface" +func _[X any, T any]() {} // want "could use \"any\" for this empty interface" +func _[any interface{}]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} // want "could use \"any\" for this empty interface" +func _[T any]() {} + +type _[T any] int // want "could use \"any\" for this empty interface" +type _[X any, T any] int // want "could use \"any\" for this empty interface" +type _[any interface{}] int // want "could use \"any\" for this empty interface" +type _[T any] int // want "could use \"any\" for this empty interface" +type _[T any] int // want "could use \"any\" for this empty interface" +type _[T any] int // want "could use \"any\" for this empty interface" +type _[T any] int diff --git a/internal/lsp/analysis/useany/useany.go b/internal/lsp/analysis/useany/useany.go new file mode 100644 index 0000000000..73e2f76331 --- /dev/null +++ b/internal/lsp/analysis/useany/useany.go @@ -0,0 +1,102 @@ +// Copyright 2021 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 useany defines an Analyzer that checks for usage of interface{} in +// constraints, rather than the predeclared any. +package useany + +import ( + "fmt" + "go/ast" + "go/token" + "go/types" + + "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" +) + +const Doc = `check for constraints that could be simplified to "any"` + +var Analyzer = &analysis.Analyzer{ + Name: "useany", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + universeAny := types.Universe.Lookup("any") + if universeAny == nil { + // Go <= 1.17. Nothing to check. + return nil, nil + } + + nodeFilter := []ast.Node{ + (*ast.TypeSpec)(nil), + (*ast.FuncType)(nil), + } + + inspect.Preorder(nodeFilter, func(node ast.Node) { + var tparams *ast.FieldList + switch node := node.(type) { + case *ast.TypeSpec: + tparams = typeparams.ForTypeSpec(node) + case *ast.FuncType: + tparams = typeparams.ForFuncType(node) + default: + panic(fmt.Sprintf("unexpected node type %T", node)) + } + if tparams.NumFields() == 0 { + return + } + + for _, field := range tparams.List { + typ := pass.TypesInfo.Types[field.Type].Type + if typ == nil { + continue // something is wrong, but not our concern + } + iface, ok := typ.Underlying().(*types.Interface) + if !ok { + continue // invalid constraint + } + + // If the constraint is the empty interface, offer a fix to use 'any' + // instead. + if iface.Empty() { + id, _ := field.Type.(*ast.Ident) + if id != nil && pass.TypesInfo.Uses[id] == universeAny { + continue + } + + diag := analysis.Diagnostic{ + Pos: field.Type.Pos(), + End: field.Type.End(), + Message: `could use "any" for this empty interface`, + } + + // Only suggest a fix to 'any' if we actually resolve the predeclared + // any in this scope. + if scope := pass.TypesInfo.Scopes[node]; scope != nil { + if _, any := scope.LookupParent("any", token.NoPos); any == universeAny { + diag.SuggestedFixes = []analysis.SuggestedFix{{ + Message: `use "any"`, + TextEdits: []analysis.TextEdit{{ + Pos: field.Type.Pos(), + End: field.Type.End(), + NewText: []byte("any"), + }}, + }} + } + } + + pass.Report(diag) + } + } + }) + return nil, nil +} diff --git a/internal/lsp/analysis/useany/useany_test.go b/internal/lsp/analysis/useany/useany_test.go new file mode 100644 index 0000000000..535d915266 --- /dev/null +++ b/internal/lsp/analysis/useany/useany_test.go @@ -0,0 +1,21 @@ +// Copyright 2021 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 useany_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/internal/lsp/analysis/useany" + "golang.org/x/tools/internal/typeparams" +) + +func Test(t *testing.T) { + if !typeparams.Enabled { + t.Skip("type params are not enabled") + } + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, useany.Analyzer, "a") +} diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 4b67341363..b78b57d02b 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -551,6 +551,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "checks for unused writes\n\nThe analyzer reports instances of writes to struct fields and\narrays that are never read. Specifically, when a struct object\nor an array is copied, its elements are copied implicitly by\nthe compiler, and any element write to this copy does nothing\nwith the original object.\n\nFor example:\n\n\ttype T struct { x int }\n\tfunc f(input []T) {\n\t\tfor i, v := range input { // v is a copy\n\t\t\tv.x = i // unused write to field x\n\t\t}\n\t}\n\nAnother example is about non-pointer receiver:\n\n\ttype T struct { x int }\n\tfunc (t T) f() { // t is a copy\n\t\tt.x = i // unused write to field x\n\t}\n", Default: "false", }, + { + Name: "\"useany\"", + Doc: "check for constraints that could be simplified to \"any\"", + Default: "true", + }, { Name: "\"fillreturns\"", Doc: "suggested fixes for \"wrong number of return values (want %d, got %d)\"\n\nThis checker provides suggested fixes for type errors of the\ntype \"wrong number of return values (want %d, got %d)\". For example:\n\tfunc m() (int, string, *bool, error) {\n\t\treturn\n\t}\nwill turn into\n\tfunc m() (int, string, *bool, error) {\n\t\treturn 0, \"\", nil, nil\n\t}\n\nThis functionality is similar to https://github.com/sqs/goreturns.\n", @@ -1124,6 +1129,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "checks for unused writes\n\nThe analyzer reports instances of writes to struct fields and\narrays that are never read. Specifically, when a struct object\nor an array is copied, its elements are copied implicitly by\nthe compiler, and any element write to this copy does nothing\nwith the original object.\n\nFor example:\n\n\ttype T struct { x int }\n\tfunc f(input []T) {\n\t\tfor i, v := range input { // v is a copy\n\t\t\tv.x = i // unused write to field x\n\t\t}\n\t}\n\nAnother example is about non-pointer receiver:\n\n\ttype T struct { x int }\n\tfunc (t T) f() { // t is a copy\n\t\tt.x = i // unused write to field x\n\t}\n", Default: false, }, + { + Name: "useany", + Doc: "check for constraints that could be simplified to \"any\"", + Default: true, + }, { Name: "fillreturns", Doc: "suggested fixes for \"wrong number of return values (want %d, got %d)\"\n\nThis checker provides suggested fixes for type errors of the\ntype \"wrong number of return values (want %d, got %d)\". For example:\n\tfunc m() (int, string, *bool, error) {\n\t\treturn\n\t}\nwill turn into\n\tfunc m() (int, string, *bool, error) {\n\t\treturn 0, \"\", nil, nil\n\t}\n\nThis functionality is similar to https://github.com/sqs/goreturns.\n", diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 19c9911041..1efe177e2b 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -57,6 +57,7 @@ import ( "golang.org/x/tools/internal/lsp/analysis/simplifyslice" "golang.org/x/tools/internal/lsp/analysis/undeclaredname" "golang.org/x/tools/internal/lsp/analysis/unusedparams" + "golang.org/x/tools/internal/lsp/analysis/useany" "golang.org/x/tools/internal/lsp/command" "golang.org/x/tools/internal/lsp/diff" "golang.org/x/tools/internal/lsp/diff/myers" @@ -1245,6 +1246,7 @@ func defaultAnalyzers() map[string]*Analyzer { testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true}, unusedparams.Analyzer.Name: {Analyzer: unusedparams.Analyzer, Enabled: false}, unusedwrite.Analyzer.Name: {Analyzer: unusedwrite.Analyzer, Enabled: false}, + useany.Analyzer.Name: {Analyzer: useany.Analyzer, Enabled: true}, // gofmt -s suite: simplifycompositelit.Analyzer.Name: { diff --git a/internal/typeparams/typeparams_go117.go b/internal/typeparams/typeparams_go117.go index 6ff07eac35..479b5561d7 100644 --- a/internal/typeparams/typeparams_go117.go +++ b/internal/typeparams/typeparams_go117.go @@ -30,15 +30,15 @@ func GetIndexExprData(n ast.Node) *IndexExprData { return nil } -// ForTypeDecl returns an empty field list, as type parameters on not supported +// ForTypeSpec returns an empty field list, as type parameters on not supported // at this Go version. -func ForTypeDecl(*ast.TypeSpec) *ast.FieldList { +func ForTypeSpec(*ast.TypeSpec) *ast.FieldList { return nil } -// ForFuncDecl returns an empty field list, as type parameters are not +// ForFuncType returns an empty field list, as type parameters are not // supported at this Go version. -func ForFuncDecl(*ast.FuncDecl) *ast.FieldList { +func ForFuncType(*ast.FuncType) *ast.FieldList { return nil } diff --git a/internal/typeparams/typeparams_go118.go b/internal/typeparams/typeparams_go118.go index 953e5c6784..16c6c0d157 100644 --- a/internal/typeparams/typeparams_go118.go +++ b/internal/typeparams/typeparams_go118.go @@ -36,17 +36,20 @@ func GetIndexExprData(n ast.Node) *IndexExprData { return nil } -// ForTypeDecl returns n.TypeParams. -func ForTypeDecl(n *ast.TypeSpec) *ast.FieldList { +// ForTypeSpec returns n.TypeParams. +func ForTypeSpec(n *ast.TypeSpec) *ast.FieldList { + if n == nil { + return nil + } return n.TypeParams } -// ForFuncDecl returns n.Type.TypeParams. -func ForFuncDecl(n *ast.FuncDecl) *ast.FieldList { - if n.Type != nil { - return n.Type.TypeParams +// ForFuncType returns n.TypeParams. +func ForFuncType(n *ast.FuncType) *ast.FieldList { + if n == nil { + return nil } - return nil + return n.TypeParams } // TypeParam is an alias for types.TypeParam