diff --git a/analyzer/testdata/src/regression/issue315.go b/analyzer/testdata/src/regression/issue315.go new file mode 100644 index 00000000..b200e561 --- /dev/null +++ b/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 +} diff --git a/analyzer/testdata/src/regression/rules.go b/analyzer/testdata/src/regression/rules.go index 2c0cfceb..808d33bc 100644 --- a/analyzer/testdata/src/regression/rules.go +++ b/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"]) +} diff --git a/ruleguard/filters.go b/ruleguard/filters.go index 5d4a8b04..848d2d93 100644 --- a/ruleguard/filters.go +++ b/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 } diff --git a/ruleguard/gorule.go b/ruleguard/gorule.go index 7c213b44..011a82ce 100644 --- a/ruleguard/gorule.go +++ b/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] } diff --git a/ruleguard/runner.go b/ruleguard/runner.go index a2697824..ad988e12 100644 --- a/ruleguard/runner.go +++ b/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()