Skip to content

Commit 4a96fd7

Browse files
authored
ruleguard: improve FieldList and Field matching (#342)
Fixes #315
1 parent 0a106b4 commit 4a96fd7

File tree

5 files changed

+67
-7
lines changed

5 files changed

+67
-7
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package regression
2+
3+
import (
4+
"io"
5+
)
6+
7+
type Issue315_Iface interface {
8+
Example()
9+
}
10+
11+
func Issue315_Func() {
12+
}
13+
14+
func Issue315_FuncErr() error {
15+
return nil
16+
}
17+
18+
func Issue315_Func1() io.Reader { // want `\Qreturn concrete type instead of io.Reader`
19+
return nil
20+
}
21+
22+
func Issue315_Func2() (io.Reader, error) { // want `\Qreturn concrete type instead of io.Reader`
23+
return nil, nil
24+
}
25+
26+
func Issue315_Func3() (int, Issue315_Iface, error) { // want `\Qreturn concrete type instead of Issue315_Iface`
27+
return 0, nil, nil
28+
}
29+
30+
type Issue315_Example struct{}
31+
32+
func (example Issue315_Example) Func1(x int) (Issue315_Iface, error) { // want `\Qreturn concrete type instead of Issue315_Iface`
33+
return nil, nil
34+
}

analyzer/testdata/src/regression/rules.go

+13
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,16 @@ func issue339(m dsl.Matcher) {
5151
m.Match(`println("339"); println("x")`).Report("pattern1")
5252
m.Match(`println("x"); println("339")`).Report("pattern2")
5353
}
54+
55+
func issue315(m dsl.Matcher) {
56+
m.Match(
57+
`func $name($*_) $arg { $*_ }`,
58+
`func $name($*_) ($arg, $_) { $*_ }`,
59+
`func $name($*_) ($_, $arg, $_) { $*_ }`,
60+
`func ($_ $_) $name($*_) ($arg, $_) { $*_ }`,
61+
).Where(
62+
m["name"].Text.Matches(`^[A-Z]`) &&
63+
m["arg"].Type.Underlying().Is(`interface{ $*_ }`) &&
64+
!m["arg"].Type.Is(`error`),
65+
).Report(`return concrete type instead of $arg`).At(m["name"])
66+
}

ruleguard/filters.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func makeTypeIsFilter(src, varname string, underlying bool, pat *typematch.Patte
254254
return pat.MatchIdentical(params.typeofNode(x).Underlying())
255255
})
256256
}
257-
typ := params.typeofNode(params.subExpr(varname)).Underlying()
257+
typ := params.typeofNode(params.subNode(varname)).Underlying()
258258
if pat.MatchIdentical(typ) {
259259
return filterSuccess
260260
}
@@ -268,7 +268,7 @@ func makeTypeIsFilter(src, varname string, underlying bool, pat *typematch.Patte
268268
return pat.MatchIdentical(params.typeofNode(x))
269269
})
270270
}
271-
typ := params.typeofNode(params.subExpr(varname))
271+
typ := params.typeofNode(params.subNode(varname))
272272
if pat.MatchIdentical(typ) {
273273
return filterSuccess
274274
}

ruleguard/gorule.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,16 @@ func (params *filterParams) subExpr(name string) ast.Expr {
9191
}
9292

9393
func (params *filterParams) typeofNode(n ast.Node) types.Type {
94-
if e, ok := n.(ast.Expr); ok {
95-
if typ := params.ctx.Types.TypeOf(e); typ != nil {
96-
return typ
97-
}
94+
var e ast.Expr
95+
switch n := n.(type) {
96+
case ast.Expr:
97+
e = n
98+
case *ast.Field:
99+
e = n.Type
100+
}
101+
if typ := params.ctx.Types.TypeOf(e); typ != nil {
102+
return typ
98103
}
99-
100104
return types.Typ[types.Invalid]
101105
}
102106

ruleguard/runner.go

+9
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"go/printer"
1010
"io/ioutil"
1111
"path/filepath"
12+
"reflect"
1213
"sort"
1314
"strconv"
1415
"strings"
@@ -390,6 +391,14 @@ func (rr *rulesRunner) renderMessage(msg string, m matchData, truncate bool) str
390391
if !strings.Contains(msg, key) {
391392
continue
392393
}
394+
// Some captured nodes are typed, but nil.
395+
// We can't really get their text, so skip them here.
396+
// For example, pattern `func $_() $results { $*_ }` may
397+
// match a nil *ast.FieldList for $results if executed
398+
// against a function with no results.
399+
if reflect.ValueOf(n).IsNil() {
400+
continue
401+
}
393402
buf.Reset()
394403
buf.Write(rr.nodeText(n))
395404
replacement := buf.String()

0 commit comments

Comments
 (0)