Skip to content

Commit

Permalink
checkers: add sloppyTypeAssert checker (#931)
Browse files Browse the repository at this point in the history
Reports interface->interface type assertions that
may be redundant.

- Any type assertion to interface{} is redundant
- Any I->I (identical) type assertion is redundant
- I1->I2 assertion is redundant if I1 implements I2

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
  • Loading branch information
quasilyte committed May 26, 2020
1 parent 9ac4766 commit 3916229
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 0 deletions.
75 changes: 75 additions & 0 deletions checkers/sloppyTypeAssert_checker.go
@@ -0,0 +1,75 @@
package checkers

import (
"go/ast"
"go/types"

"github.com/go-lintpack/lintpack"
"github.com/go-lintpack/lintpack/astwalk"
"github.com/go-toolsmith/astcast"
)

func init() {
var info lintpack.CheckerInfo
info.Name = "sloppyTypeAssert"
info.Tags = []string{"diagnostic", "experimental"}
info.Summary = "Detects redundant type assertions"
info.Before = `
function f(r io.Reader) interface{} {
return r.(interface{})
}
`
info.After = `
function f(r io.Reader) interface{} {
return r
}
`

collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker {
return astwalk.WalkerForExpr(&sloppyTypeAssertChecker{ctx: ctx})
})
}

type sloppyTypeAssertChecker struct {
astwalk.WalkHandler
ctx *lintpack.CheckerContext
}

func (c *sloppyTypeAssertChecker) VisitExpr(expr ast.Expr) {
assert := astcast.ToTypeAssertExpr(expr)
if assert.Type == nil {
return
}

toType := c.ctx.TypesInfo.TypeOf(expr)
fromType := c.ctx.TypesInfo.TypeOf(assert.X)

if types.Identical(toType, fromType) {
c.warnIdentical(expr)
return
}

toIface, ok := toType.Underlying().(*types.Interface)
if !ok {
return
}

switch {
case toIface.Empty():
c.warnEmpty(expr)
case types.Implements(fromType, toIface):
c.warnImplements(expr, assert.X)
}
}

func (c *sloppyTypeAssertChecker) warnIdentical(cause ast.Expr) {
c.ctx.Warn(cause, "type assertion from/to types are identical")
}

func (c *sloppyTypeAssertChecker) warnEmpty(cause ast.Expr) {
c.ctx.Warn(cause, "type assertion to interface{} may be redundant")
}

func (c *sloppyTypeAssertChecker) warnImplements(cause, val ast.Expr) {
c.ctx.Warn(cause, "type assertion may be redundant as %s always implements selected interface", val)
}
13 changes: 13 additions & 0 deletions checkers/testdata/sloppyTypeAssert/negative_tests.go
@@ -0,0 +1,13 @@
package checker_test

import (
"io"
)

func noWarnings(eface interface{}, r io.Reader, rc io.ReadCloser) {
// interface{} -> other non-empty interface assertion.
_ = eface.(io.Reader)

// assertion to a wider interface.
_ = r.(io.ReadCloser)
}
26 changes: 26 additions & 0 deletions checkers/testdata/sloppyTypeAssert/positive_tests.go
@@ -0,0 +1,26 @@
package checker_test

import (
"io"
)

type underlyingReader io.Reader

func redundantTypeAsserts(eface interface{}, r io.Reader, rc io.ReadCloser) {
/*! type assertion to interface{} may be redundant */
_ = r.(interface{})

/*! type assertion may be redundant as rc always implements selected interface */
_ = rc.(io.Reader)

/*! type assertion from/to types are identical */
_ = rc.(io.ReadCloser)

var ur underlyingReader

/*! type assertion may be redundant as ur always implements selected interface */
_ = ur.(io.Reader)

/*! type assertion may be redundant as r always implements selected interface */
_ = r.(underlyingReader)
}

0 comments on commit 3916229

Please sign in to comment.