Skip to content

Commit

Permalink
ruleguard: handle variadic vars in filters properly (#238)
Browse files Browse the repository at this point in the history 
For patterns like `f($*args)` we can now apply a
`m["args"].Pure` filter to check whether **all** arguments are pure.

It works with most filters.

The current implementation is messy, but it will do for now.
  • Loading branch information
@quasilyte
quasilyte authored Jun 5, 2021
1 parent f8e9dc0 commit c33c867
Show file tree
Hide file tree
Showing 10 changed files with 319 additions and 54 deletions.
170 changes: 156 additions & 14 deletions analyzer/testdata/src/filtertest/f1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"os"
"time"
"unsafe"
)

type implementsAll struct{}
Expand Down Expand Up @@ -49,6 +50,17 @@ func assignableTo() {
typeTest("", "assignable to interface{}") // want `YES`
}

func detectValue() {
valueTest("variadic value 5") // want `true`
valueTest(5, "variadic value 5") // want `true`
valueTest(2+3, 6-1, "variadic value 5") // want `true`

valueTest(0, "variadic value 5") // want `false`
valueTest("5", "variadic value 5") // want `false`
valueTest(5, 0, "variadic value 5") // want `false`
valueTest(0, 5, "variadic value 5") // want `false`
}

func detectType() {
{
var s fmt.Stringer
Expand Down Expand Up @@ -151,6 +163,22 @@ func detectType() {
typeTest(embedImplementsAllPtr{}, "implements error") // want `YES`
typeTest(&embedImplementsAllPtr{}, "implements error") // want `YES`

typeTest(&implementsAll{}, "variadic implements error") // want `true`
typeTest(&implementsAll{}, error(nil), "variadic implements error") // want `true`
typeTest(error(nil), "variadic implements error") // want `true`
typeTest(errors.New("example"), "variadic implements error") // want `true`
typeTest(&embedImplementsAll{}, "variadic implements error") // want `true`
typeTest(embedImplementsAllPtr{}, "variadic implements error") // want `true`
typeTest(&embedImplementsAllPtr{}, "variadic implements error") // want `true`
typeTest(implementsAll{}, "variadic implements error") // want `false`
typeTest(i1, "variadic implements error") // want `false`
typeTest(implementsAllNewtype{}, "variadic implements error") // want `false`
typeTest(&implementsAllNewtype{}, "variadic implements error") // want `false`
typeTest(embedImplementsAll{}, "variadic implements error") // want `false`
typeTest(embedImplementsAll{}, 1, "variadic implements error") // want `false`
typeTest(embedImplementsAll{}, 1, "variadic implements error") // want `false`
typeTest(embedImplementsAll{}, error(nil), "variadic implements error") // want `false`

typeTest([100]byte{}, "size>=100") // want `YES`
typeTest([105]byte{}, "size>=100") // want `YES`
typeTest([10]byte{}, "size>=100")
Expand All @@ -170,6 +198,14 @@ func detectType() {
typeTest([105]byte{}, "size!=100") // want `YES`
typeTest([10]byte{}, "size!=100") // want `YES`

typeTest("variadic size==4") // want `true`
typeTest([4]byte{}, "variadic size==4") // want `true`
typeTest(int32(0), rune(0), [2]uint16{}, "variadic size==4") // want `true`

typeTest([6]byte{}, "variadic size==4") // want `false`
typeTest(uint32(0), [6]byte{}, "variadic size==4") // want `false`
typeTest([6]byte{}, uint32(0), "variadic size==4") // want `false`

var time1, time2 time.Time
var err error
typeTest(time1 == time2, "time==time") // want `YES`
Expand All @@ -195,6 +231,99 @@ func detectType() {
typeTest(implementsAll.String(v), "func() string")
typeTest(implementsAll.String, "func() string")

{
type newInt int

var x int
y := newInt(5)

typeTest("variadic int") // want `true`
typeTest(1, "variadic int") // want `true`
typeTest(x, 2, "variadic int") // want `true`
typeTest(-x, x+1, +x, "variadic int") // want `true`

typeTest("no", "variadic int") // want `false`
typeTest(1, "no", "variadic int") // want `false`
typeTest(y, "variadic int") // want `false`
typeTest("no", 2, "variadic int") // want `false`
typeTest(1, "no", 3, "variadic int") // want `false`
typeTest(1, 2, "no", "variadic int") // want `false`
}

{
type newInt int

var x int
y := newInt(5)

typeTest("variadic underlying int") // want `true`
typeTest(1, "variadic underlying int") // want `true`
typeTest(y, "variadic underlying int") // want `true`
typeTest(x, 2, "variadic underlying int") // want `true`
typeTest(-y, y+1, +x, "variadic underlying int") // want `true`

typeTest("no", "variadic underlying int") // want `false`
typeTest(1, "no", "variadic underlying int") // want `false`
typeTest("no", 2, "variadic underlying int") // want `false`
typeTest(1, "no", 3, "variadic underlying int") // want `false`
typeTest(1, 2, "no", "variadic underlying int") // want `false`
}
}

func detectAddressable(x int, xs []int) {
typeTest("variadic addressable") // want `true`
typeTest(x, "variadic addressable") // want `true`
typeTest(xs, x, "variadic addressable") // want `true`
typeTest(xs, x, xs[0], "variadic addressable") // want `true`

typeTest(x, "", "variadic addressable") // want `false`
typeTest(1, x, xs[0], "variadic addressable") // want `false`
}

func detectConvertibleTo(x int, xs []int) {
type newString string
stringSlice := []string{""}

typeTest("variadic convertible to string") // want `true`
typeTest("yes", "variadic convertible to string") // want `true`
typeTest("yes", newString("yes"), "variadic convertible to string") // want `true`
typeTest([]byte("yes"), newString("yes"), "", stringSlice[0], "variadic convertible to string") // want `true`

typeTest(xs, "variadic convertible to string") // want `false`
typeTest(xs, "", "variadic convertible to string") // want `false`
typeTest("", xs[:], "variadic convertible to string") // want `false`
}

func detectAssignableTo(x int, xs []int) {
type newString string
type stringAlias = string
stringSlice := []string{""}

typeTest("variadic assignable to string") // want `true`
typeTest("yes", "variadic assignable to string") // want `true`
typeTest(stringAlias("yes"), "yes", "variadic assignable to string") // want `true`

typeTest("yes", newString("yes"), "variadic assignable to string") // want `false`
typeTest([]byte("yes"), newString("yes"), "", stringSlice[0], "variadic assignable to string") // want `false`
typeTest([]byte("no"), "variadic assignable to string") // want `false`
typeTest(xs, "variadic assignable to string") // want `false`
typeTest(xs, "", "variadic assignable to string") // want `false`
typeTest("", xs[:], "variadic assignable to string") // want `false`
}

func detectConst(x int, xs []int) {
const namedIntConst = 130

constTest("variadic const") // want `true`
constTest(1, "variadic const") // want `true`
constTest(namedIntConst, 1<<3, "variadic const") // want `true`
constTest(namedIntConst, namedIntConst*2, "variadic const") // want `true`
constTest(1, unsafe.Sizeof(int(0)), 3, "variadic const") // want `true`

constTest(x, "variadic const") // want `false`
constTest(random(), "variadic const") // want `false`
constTest(1, random(), "variadic const") // want `false`
constTest(random(), 2, "variadic const") // want `false`
}

func detectPure(x int, xs []int) {
Expand All @@ -204,20 +333,33 @@ func detectPure(x int, xs []int) {

xptr := &x

pureTest(random()) // want `!pure`
pureTest([]int{random()}) // want `!pure`

pureTest(*xptr) // want `pure`
pureTest(int(x)) // want `pure`
pureTest((*int)(&x)) // want `pure`
pureTest((func())(func() {})) // want `pure`
pureTest(foo.a) // want `pure`
pureTest(x * x) // want `pure`
pureTest((x * x)) // want `pure`
pureTest(+x) // want `pure`
pureTest(xs[0]) // want `pure`
pureTest(xs[x]) // want `pure`
pureTest([]int{0}) // want `pure`
pureTest(random()) // want `false`
pureTest([]int{random()}) // want `false`

pureTest(*xptr) // want `true`
pureTest(int(x)) // want `true`
pureTest((*int)(&x)) // want `true`
pureTest((func())(func() {})) // want `true`
pureTest(foo.a) // want `true`
pureTest(x * x) // want `true`
pureTest((x * x)) // want `true`
pureTest(+x) // want `true`
pureTest(xs[0]) // want `true`
pureTest(xs[x]) // want `true`
pureTest([]int{0}) // want `true`

pureTest("variadic pure") // want `true`
pureTest("", "variadic pure") // want `true`
pureTest(1, 2, "variadic pure") // want `true`
pureTest(1, xs[0], "variadic pure") // want `true`
pureTest(*xptr, xs[0], xptr, "variadic pure") // want `true`

pureTest(random(), "variadic pure") // want `false`
pureTest(1, random(), "variadic pure") // want `false`
pureTest(random(), 2, "variadic pure") // want `false`
pureTest(1, xs[0], random(), "variadic pure") // want `false`
pureTest(random(), xs[0], 3, "variadic pure") // want `false`
pureTest(random(), random(), "variadic pure") // want `false`
}

func detectText(foo, bar int) {
Expand Down
46 changes: 44 additions & 2 deletions analyzer/testdata/src/filtertest/rules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ func testRules(m dsl.Matcher) {
Where(!m["x"].Type.Is(`int`)).
Report(`$x !is(int)`)

m.Match(`typeTest($*xs, "variadic int")`).Where(m["xs"].Type.Is(`int`)).Report(`true`)
m.Match(`typeTest($*xs, "variadic int")`).Where(!m["xs"].Type.Is(`int`)).Report(`false`)

m.Match(`typeTest($*xs, "variadic underlying int")`).Where(m["xs"].Type.Underlying().Is(`int`)).Report(`true`)
m.Match(`typeTest($*xs, "variadic underlying int")`).Where(!m["xs"].Type.Underlying().Is(`int`)).Report(`false`)

m.Match(`typeTest($x > $y)`).
Where(!m["x"].Type.Is(`string`) && m["x"].Pure).
Report(`$x !is(string) && pure`)
Expand All @@ -54,6 +60,14 @@ func testRules(m dsl.Matcher) {
m.Match(`typeTest($x, "implements error")`).
Where(m["x"].Type.Implements(`error`)).Report(`YES`)

m.Match(`typeTest($*xs, "variadic implements error")`).
Where(m["xs"].Type.Implements(`error`)).Report(`true`)
m.Match(`typeTest($*xs, "variadic implements error")`).
Where(!m["xs"].Type.Implements(`error`)).Report(`false`)

m.Match(`typeTest($*xs, "variadic size==4")`).Where(m["xs"].Type.Size == 4).Report(`true`)
m.Match(`typeTest($*xs, "variadic size==4")`).Where(!(m["xs"].Type.Size == 4)).Report(`false`)

m.Match(`typeTest($x, "size>=100")`).Where(m["x"].Type.Size >= 100).Report(`YES`)
m.Match(`typeTest($x, "size<=100")`).Where(m["x"].Type.Size <= 100).Report(`YES`)
m.Match(`typeTest($x, "size>100")`).Where(m["x"].Type.Size > 100).Report(`YES`)
Expand All @@ -78,11 +92,39 @@ func testRules(m dsl.Matcher) {

m.Match(`pureTest($x)`).
Where(m["x"].Pure).
Report("pure")
Report("true")

m.Match(`pureTest($x)`).
Where(!m["x"].Pure).
Report("!pure")
Report("false")

m.Match(`pureTest($*xs, "variadic pure")`).
Where(m["xs"].Pure).
Report("true")

m.Match(`pureTest($*xs, "variadic pure")`).
Where(!m["xs"].Pure).
Report("false")

m.Match(`constTest($*xs, "variadic const")`).
Where(m["xs"].Const).
Report("true")

m.Match(`constTest($*xs, "variadic const")`).
Where(!m["xs"].Const).
Report("false")

m.Match(`typeTest($*xs, "variadic addressable")`).Where(m["xs"].Addressable).Report("true")
m.Match(`typeTest($*xs, "variadic addressable")`).Where(!m["xs"].Addressable).Report("false")

m.Match(`typeTest($*xs, "variadic convertible to string")`).Where(m["xs"].Type.ConvertibleTo(`string`)).Report("true")
m.Match(`typeTest($*xs, "variadic convertible to string")`).Where(!m["xs"].Type.ConvertibleTo(`string`)).Report("false")

m.Match(`typeTest($*xs, "variadic assignable to string")`).Where(m["xs"].Type.AssignableTo(`string`)).Report("true")
m.Match(`typeTest($*xs, "variadic assignable to string")`).Where(!m["xs"].Type.AssignableTo(`string`)).Report("false")

m.Match(`valueTest($*xs, "variadic value 5")`).Where(m["xs"].Value.Int() == 5).Report(`true`)
m.Match(`valueTest($*xs, "variadic value 5")`).Where(!(m["xs"].Value.Int() == 5)).Report(`false`)

m.Match(`lineTest($x, "line 4")`).Where(m["x"].Line == 4).Report(`YES`)
m.Match(`lineTest($x, $y, "same line")`).Where(m["x"].Line == m["y"].Line).Report(`YES`)
Expand Down
2 changes: 2 additions & 0 deletions analyzer/testdata/src/filtertest/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package filtertest

func typeTest(args ...interface{}) {}
func pureTest(args ...interface{}) {}
func constTest(args ...interface{}) {}
func textTest(args ...interface{}) {}
func parensFilterTest(args ...interface{}) {}
func importsTest(args ...interface{}) {}
func fileTest(args ...interface{}) {}
func nodeTest(args ...interface{}) {}
func lineTest(args ...interface{}) {}
func valueTest(args ...interface{}) {}

func random() int {
return 42
Expand Down
4 changes: 2 additions & 2 deletions internal/gogrep/compile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (c *compiler) compileNode(n ast.Node) {
c.compileValueSpec(n)
case stmtSlice:
c.compileStmtSlice(n)
case exprSlice:
case ExprSlice:
c.compileExprSlice(n)
default:
panic(c.errorf(n, "compileNode: unexpected %T", n))
Expand Down Expand Up @@ -971,7 +971,7 @@ func (c *compiler) compileStmtSlice(stmts stmtSlice) {
c.emitInstOp(opEnd)
}

func (c *compiler) compileExprSlice(exprs exprSlice) {
func (c *compiler) compileExprSlice(exprs ExprSlice) {
c.emitInstOp(opMultiExpr)
for _, n := range exprs {
c.compileExpr(n)
Expand Down
4 changes: 2 additions & 2 deletions internal/gogrep/gogrep.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
)

func IsEmptyNodeSlice(n ast.Node) bool {
if list, ok := n.(nodeSlice); ok {
return list.len() == 0
if list, ok := n.(NodeSlice); ok {
return list.Len() == 0
}
return false
}
Expand Down
18 changes: 9 additions & 9 deletions internal/gogrep/match.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ func (m *matcher) MatchNode(n ast.Node, accept func(MatchData)) {
}

func (m *matcher) walkExprSlice(exprs []ast.Expr, accept func(MatchData)) {
m.walkNodeSlice(exprSlice(exprs), accept)
m.walkNodeSlice(ExprSlice(exprs), accept)
}

func (m *matcher) walkStmtSlice(stmts []ast.Stmt, accept func(MatchData)) {
m.walkNodeSlice(stmtSlice(stmts), accept)
}

func (m *matcher) walkNodeSlice(nodes nodeSlice, accept func(MatchData)) {
sliceLen := nodes.len()
func (m *matcher) walkNodeSlice(nodes NodeSlice, accept func(MatchData)) {
sliceLen := nodes.Len()
from := 0
for {
m.pc = 1 // FIXME: this is a kludge
Expand Down Expand Up @@ -506,7 +506,7 @@ func (m *matcher) matchStmtSlice(stmts []ast.Stmt) bool {
}

func (m *matcher) matchExprSlice(exprs []ast.Expr) bool {
matched, _ := m.matchNodeList(exprSlice(exprs), false)
matched, _ := m.matchNodeList(ExprSlice(exprs), false)
return matched != nil
}

Expand All @@ -527,8 +527,8 @@ func (m *matcher) matchSpecSlice(specs []ast.Spec) bool {

// matchNodeList matches two lists of nodes. It uses a common algorithm to match
// wildcard patterns with any number of nodes without recursion.
func (m *matcher) matchNodeList(nodes nodeSlice, partial bool) (ast.Node, int) {
sliceLen := nodes.len()
func (m *matcher) matchNodeList(nodes NodeSlice, partial bool) (ast.Node, int) {
sliceLen := nodes.Len()
inst := m.nextInst()
if inst.op == opEnd {
if sliceLen == 0 {
Expand Down Expand Up @@ -615,7 +615,7 @@ func (m *matcher) matchNodeList(nodes nodeSlice, partial bool) (ast.Node, int) {
partialStart = j
push(j + 1)
}
if j < sliceLen && wouldMatch() && m.matchNodeWithInst(inst, nodes.at(j)) {
if j < sliceLen && wouldMatch() && m.matchNodeWithInst(inst, nodes.At(j)) {
// ordinary match
wildName = ""
j++
Expand Down Expand Up @@ -699,8 +699,8 @@ func equalNodes(x, y ast.Node) bool {
}
}
return true
case exprSlice:
y, ok := y.(exprSlice)
case ExprSlice:
y, ok := y.(ExprSlice)
if !ok || len(x) != len(y) {
return false
}
Expand Down
2 changes: 1 addition & 1 deletion internal/gogrep/parse.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func parseDetectingNode(fset *token.FileSet, src string) (ast.Node, *ast.File, e
if len(cl.Elts) == 1 {
return cl.Elts[0], f, nil
}
return exprSlice(cl.Elts), f, nil
return ExprSlice(cl.Elts), f, nil
}

// then try as statements
Expand Down
Loading

0 comments on commit c33c867

Please sign in to comment.