Skip to content

Commit

Permalink
internal/lsp/analysis: add a useany analyzer
Browse files Browse the repository at this point in the history
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 <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
  • Loading branch information
findleyr committed Sep 22, 2021
1 parent efaec4e commit 2847958
Show file tree
Hide file tree
Showing 9 changed files with 205 additions and 11 deletions.
6 changes: 6 additions & 0 deletions gopls/doc/analyzers.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
25 changes: 25 additions & 0 deletions internal/lsp/analysis/useany/testdata/src/a/a.go
Original file line number Diff line number Diff line change
@@ -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
25 changes: 25 additions & 0 deletions internal/lsp/analysis/useany/testdata/src/a/a.go.golden
Original file line number Diff line number Diff line change
@@ -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
102 changes: 102 additions & 0 deletions internal/lsp/analysis/useany/useany.go
Original file line number Diff line number Diff line change
@@ -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
}
21 changes: 21 additions & 0 deletions internal/lsp/analysis/useany/useany_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
10 changes: 10 additions & 0 deletions internal/lsp/source/api_json.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions internal/lsp/source/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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: {
Expand Down
8 changes: 4 additions & 4 deletions internal/typeparams/typeparams_go117.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
17 changes: 10 additions & 7 deletions internal/typeparams/typeparams_go118.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 2847958

Please sign in to comment.