ruleguard: improve FieldList and Field matching (#342)

Fixes #315

analyzer/testdata/src/regression/issue315.go

@@ -0,0 +1,34 @@
+package regression
+
+import (
+	"io"
+)
+
+type Issue315_Iface interface {
+	Example()
+}
+
+func Issue315_Func() {
+}
+
+func Issue315_FuncErr() error {
+	return nil
+}
+
+func Issue315_Func1() io.Reader { // want `\Qreturn concrete type instead of io.Reader`
+	return nil
+}
+
+func Issue315_Func2() (io.Reader, error) { // want `\Qreturn concrete type instead of io.Reader`
+	return nil, nil
+}
+
+func Issue315_Func3() (int, Issue315_Iface, error) { // want `\Qreturn concrete type instead of Issue315_Iface`
+	return 0, nil, nil
+}
+
+type Issue315_Example struct{}
+
+func (example Issue315_Example) Func1(x int) (Issue315_Iface, error) { // want `\Qreturn concrete type instead of Issue315_Iface`
+	return nil, nil
+}

analyzer/testdata/src/regression/rules.go

@@ -51,3 +51,16 @@ func issue339(m dsl.Matcher) {
 	m.Match(`println("339"); println("x")`).Report("pattern1")
 	m.Match(`println("x"); println("339")`).Report("pattern2")
 }
+
+func issue315(m dsl.Matcher) {
+	m.Match(
+		`func $name($*_) $arg { $*_ }`,
+		`func $name($*_) ($arg, $_) { $*_ }`,
+		`func $name($*_) ($_, $arg, $_) { $*_ }`,
+		`func ($_ $_) $name($*_) ($arg, $_) { $*_ }`,
+	).Where(
+		m["name"].Text.Matches(`^[A-Z]`) &&
+			m["arg"].Type.Underlying().Is(`interface{ $*_ }`) &&
+			!m["arg"].Type.Is(`error`),
+	).Report(`return concrete type instead of $arg`).At(m["name"])
+}

ruleguard/filters.go

@@ -254,7 +254,7 @@ func makeTypeIsFilter(src, varname string, underlying bool, pat *typematch.Patte
 					return pat.MatchIdentical(params.typeofNode(x).Underlying())
 				})
 			}
-			typ := params.typeofNode(params.subExpr(varname)).Underlying()
+			typ := params.typeofNode(params.subNode(varname)).Underlying()
 			if pat.MatchIdentical(typ) {
 				return filterSuccess
 			}
@@ -268,7 +268,7 @@ func makeTypeIsFilter(src, varname string, underlying bool, pat *typematch.Patte
 				return pat.MatchIdentical(params.typeofNode(x))
 			})
 		}
-		typ := params.typeofNode(params.subExpr(varname))
+		typ := params.typeofNode(params.subNode(varname))
 		if pat.MatchIdentical(typ) {
 			return filterSuccess
 		}

ruleguard/gorule.go

@@ -91,12 +91,16 @@ func (params *filterParams) subExpr(name string) ast.Expr {
 }
 
 func (params *filterParams) typeofNode(n ast.Node) types.Type {
-	if e, ok := n.(ast.Expr); ok {
-		if typ := params.ctx.Types.TypeOf(e); typ != nil {
-			return typ
-		}
+	var e ast.Expr
+	switch n := n.(type) {
+	case ast.Expr:
+		e = n
+	case *ast.Field:
+		e = n.Type
+	}
+	if typ := params.ctx.Types.TypeOf(e); typ != nil {
+		return typ
 	}
-
 	return types.Typ[types.Invalid]
 }
 

ruleguard/runner.go

@@ -9,6 +9,7 @@ import (
 	"go/printer"
 	"io/ioutil"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strconv"
 	"strings"
@@ -390,6 +391,14 @@ func (rr *rulesRunner) renderMessage(msg string, m matchData, truncate bool) str
 		if !strings.Contains(msg, key) {
 			continue
 		}
+		// Some captured nodes are typed, but nil.
+		// We can't really get their text, so skip them here.
+		// For example, pattern `func $_() $results { $*_ }` may
+		// match a nil *ast.FieldList for $results if executed
+		// against a function with no results.
+		if reflect.ValueOf(n).IsNil() {
+			continue
+		}
 		buf.Reset()
 		buf.Write(rr.nodeText(n))
 		replacement := buf.String()