From e1da425f72fd3793b579f4e74d908ba96eb16c8a Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 19 Feb 2020 21:17:00 +0000 Subject: [PATCH] Revert "Revert "go/analysis: add pass to check for impossible interface-to-interface type assertions"" This reverts commit 11eff242d136374289f76e9313c76e9312391172. Reason for revert: The 1.15 tree is now open for early submits. The original CL was authored by Akhil Indurti (aindurti@gmail.com). Change-Id: I8cfcd0253a6666a6392fa938bb19a1b426ba712d Reviewed-on: https://go-review.googlesource.com/c/tools/+/220139 Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot Reviewed-by: Akhil Indurti Reviewed-by: Andrew Bonventre --- .../ifaceassert/cmd/ifaceassert/main.go | 13 +++ go/analysis/passes/ifaceassert/ifaceassert.go | 101 ++++++++++++++++++ .../passes/ifaceassert/ifaceassert_test.go | 17 +++ .../passes/ifaceassert/testdata/src/a/a.go | 40 +++++++ 4 files changed, 171 insertions(+) create mode 100644 go/analysis/passes/ifaceassert/cmd/ifaceassert/main.go create mode 100644 go/analysis/passes/ifaceassert/ifaceassert.go create mode 100644 go/analysis/passes/ifaceassert/ifaceassert_test.go create mode 100644 go/analysis/passes/ifaceassert/testdata/src/a/a.go diff --git a/go/analysis/passes/ifaceassert/cmd/ifaceassert/main.go b/go/analysis/passes/ifaceassert/cmd/ifaceassert/main.go new file mode 100644 index 0000000000..42250f93df --- /dev/null +++ b/go/analysis/passes/ifaceassert/cmd/ifaceassert/main.go @@ -0,0 +1,13 @@ +// Copyright 2020 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. + +// The ifaceassert command runs the ifaceassert analyzer. +package main + +import ( + "golang.org/x/tools/go/analysis/passes/ifaceassert" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { singlechecker.Main(ifaceassert.Analyzer) } diff --git a/go/analysis/passes/ifaceassert/ifaceassert.go b/go/analysis/passes/ifaceassert/ifaceassert.go new file mode 100644 index 0000000000..c5a71a7c57 --- /dev/null +++ b/go/analysis/passes/ifaceassert/ifaceassert.go @@ -0,0 +1,101 @@ +// Copyright 2020 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 ifaceassert defines an Analyzer that flags +// impossible interface-interface type assertions. +package ifaceassert + +import ( + "go/ast" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +const Doc = `detect impossible interface-to-interface type assertions + +This checker flags type assertions v.(T) and corresponding type-switch cases +in which the static type V of v is an interface that cannot possibly implement +the target interface T. This occurs when V and T contain methods with the same +name but different signatures. Example: + + var v interface { + Read() + } + _ = v.(io.Reader) + +The Read method in v has a different signature than the Read method in +io.Reader, so this assertion cannot succeed. +` + +var Analyzer = &analysis.Analyzer{ + Name: "ifaceassert", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +// assertableTo checks whether interface v can be asserted into t. It returns +// nil on success, or the first conflicting method on failure. +func assertableTo(v, t types.Type) *types.Func { + // ensure that v and t are interfaces + V, _ := v.Underlying().(*types.Interface) + T, _ := t.Underlying().(*types.Interface) + if V == nil || T == nil { + return nil + } + if f, wrongType := types.MissingMethod(V, T, false); wrongType { + return f + } + return nil +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{ + (*ast.TypeAssertExpr)(nil), + (*ast.TypeSwitchStmt)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + var ( + assert *ast.TypeAssertExpr // v.(T) expression + targets []ast.Expr // interfaces T in v.(T) + ) + switch n := n.(type) { + case *ast.TypeAssertExpr: + // take care of v.(type) in *ast.TypeSwitchStmt + if n.Type == nil { + return + } + assert = n + targets = append(targets, n.Type) + case *ast.TypeSwitchStmt: + // retrieve type assertion from type switch's 'assign' field + switch t := n.Assign.(type) { + case *ast.ExprStmt: + assert = t.X.(*ast.TypeAssertExpr) + case *ast.AssignStmt: + assert = t.Rhs[0].(*ast.TypeAssertExpr) + } + // gather target types from case clauses + for _, c := range n.Body.List { + targets = append(targets, c.(*ast.CaseClause).List...) + } + } + V := pass.TypesInfo.TypeOf(assert.X) + for _, target := range targets { + T := pass.TypesInfo.TypeOf(target) + if f := assertableTo(V, T); f != nil { + pass.Reportf( + target.Pos(), + "impossible type assertion: no type can implement both %v and %v (conflicting types for %v method)", + V, T, f.Name(), + ) + } + } + }) + return nil, nil +} diff --git a/go/analysis/passes/ifaceassert/ifaceassert_test.go b/go/analysis/passes/ifaceassert/ifaceassert_test.go new file mode 100644 index 0000000000..4607338928 --- /dev/null +++ b/go/analysis/passes/ifaceassert/ifaceassert_test.go @@ -0,0 +1,17 @@ +// Copyright 2020 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 ifaceassert_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/ifaceassert" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, ifaceassert.Analyzer, "a") +} diff --git a/go/analysis/passes/ifaceassert/testdata/src/a/a.go b/go/analysis/passes/ifaceassert/testdata/src/a/a.go new file mode 100644 index 0000000000..ca9beb00ef --- /dev/null +++ b/go/analysis/passes/ifaceassert/testdata/src/a/a.go @@ -0,0 +1,40 @@ +// Copyright 2020 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 ifaceassert checker. + +package a + +import "io" + +func InterfaceAssertionTest() { + var ( + a io.ReadWriteSeeker + b interface { + Read() + Write() + } + ) + _ = a.(io.Reader) + _ = a.(io.ReadWriter) + _ = b.(io.Reader) // want `^impossible type assertion: no type can implement both interface{Read\(\); Write\(\)} and io.Reader \(conflicting types for Read method\)$` + _ = b.(interface { // want `^impossible type assertion: no type can implement both interface{Read\(\); Write\(\)} and interface{Read\(p \[\]byte\) \(n int, err error\)} \(conflicting types for Read method\)$` + Read(p []byte) (n int, err error) + }) + + switch a.(type) { + case io.ReadWriter: + case interface { // want `^impossible type assertion: no type can implement both io.ReadWriteSeeker and interface{Write\(\)} \(conflicting types for Write method\)$` + Write() + }: + default: + } + + switch b := b.(type) { + case io.ReadWriter, interface{ Read() }: // want `^impossible type assertion: no type can implement both interface{Read\(\); Write\(\)} and io.ReadWriter \(conflicting types for Read method\)$` + case io.Writer: // want `^impossible type assertion: no type can implement both interface{Read\(\); Write\(\)} and io.Writer \(conflicting types for Write method\)$` + default: + _ = b + } +}