Skip to content

Commit

Permalink
internal/gogrep: allow f($*_) to match variadic calls (#239)
Browse files Browse the repository at this point in the history 
We rename `CallExpr` to `NonVariadicCallExpr` -- it rejects variadic calls.
We keep `VariadicCallExpr` as is.
We introduce a new `CallExpr` op that matches both normal and variadic calls.

`CallExpr` is generated for call patterns that should accept both kinds of calls.
So if the last pattern argument is seq match var, we emit `CallExpr`.

Fixes #237
  • Loading branch information
@quasilyte
quasilyte authored Jun 5, 2021
1 parent 0a721ee commit fc40544
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 147 deletions.
2 changes: 1 addition & 1 deletion analyzer/testdata/src/gocritic/rules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func badCond(m dsl.Matcher) {
}

func appendAssign(m dsl.Matcher) {
m.Match(`$x = append($y, $_, $*_)`).
m.Match(`$x = append($y, $_)`).
Where(m["x"].Text != m["y"].Text &&
m["x"].Text != "_" &&
m["x"].Node.Is(`Ident`) &&
Expand Down
16 changes: 15 additions & 1 deletion internal/gogrep/compile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,10 +396,24 @@ func (c *compiler) compileIdent(n *ast.Ident) {
}

func (c *compiler) compileCallExpr(n *ast.CallExpr) {
op := opCallExpr
canBeVariadic := func(n *ast.CallExpr) bool {
if len(n.Args) == 0 {
return false
}
lastArg, ok := n.Args[len(n.Args)-1].(*ast.Ident)
if !ok {
return false
}
return isWildName(lastArg.Name) && decodeWildName(lastArg.Name).Seq
}

op := opNonVariadicCallExpr
if n.Ellipsis.IsValid() {
op = opVariadicCallExpr
} else if canBeVariadic(n) {
op = opCallExpr
}

c.emitInstOp(op)
c.compileExpr(n.Fun)
for _, arg := range n.Args {
Expand Down
56 changes: 39 additions & 17 deletions internal/gogrep/compile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,28 @@ func TestCompileWildcard(t *testing.T) {
` • BlockStmt`,
` • • End`,
},

`f($*_)`: {
`CallExpr`,
` • Ident f`,
` • NodeSeq`,
` • End`,
},

`f(1, $*_)`: {
`CallExpr`,
` • Ident f`,
` • BasicLit 1`,
` • NodeSeq`,
` • End`,
},

`f($_)`: {
`NonVariadicCallExpr`,
` • Ident f`,
` • Node`,
` • End`,
},
})

for i := range tests {
Expand Down Expand Up @@ -414,7 +436,7 @@ func TestCompile(t *testing.T) {
},

`f(1, 2)`: {
`CallExpr`,
`NonVariadicCallExpr`,
` • Ident f`,
` • BasicLit 1`,
` • BasicLit 2`,
Expand All @@ -424,7 +446,7 @@ func TestCompile(t *testing.T) {
`f(g(), xs...)`: {
`VariadicCallExpr`,
` • Ident f`,
` • CallExpr`,
` • NonVariadicCallExpr`,
` • • Ident g`,
` • • End`,
` • Ident xs`,
Expand Down Expand Up @@ -472,7 +494,7 @@ func TestCompile(t *testing.T) {
},

`([2]int)(x)`: {
`CallExpr`,
`NonVariadicCallExpr`,
` • ParenExpr`,
` • • ArrayType`,
` • • • BasicLit 2`,
Expand All @@ -481,7 +503,7 @@ func TestCompile(t *testing.T) {
` • End`,
},
`([]int)(x)`: {
`CallExpr`,
`NonVariadicCallExpr`,
` • ParenExpr`,
` • • SliceType`,
` • • • Ident int`,
Expand Down Expand Up @@ -532,14 +554,14 @@ func TestCompile(t *testing.T) {

`go f()`: {
`GoStmt`,
` • CallExpr`,
` • NonVariadicCallExpr`,
` • • Ident f`,
` • • End`,
},

`defer f()`: {
`DeferStmt`,
` • CallExpr`,
` • NonVariadicCallExpr`,
` • • Ident f`,
` • • End`,
},
Expand Down Expand Up @@ -576,11 +598,11 @@ func TestCompile(t *testing.T) {
`{ f(); g(); }`: {
`BlockStmt`,
` • ExprStmt`,
` • • CallExpr`,
` • • NonVariadicCallExpr`,
` • • • Ident f`,
` • • • End`,
` • ExprStmt`,
` • • CallExpr`,
` • • NonVariadicCallExpr`,
` • • • Ident g`,
` • • • End`,
` • End`,
Expand All @@ -607,7 +629,7 @@ func TestCompile(t *testing.T) {
` • • End`,
` • BlockStmt`,
` • • ExprStmt`,
` • • • CallExpr`,
` • • • NonVariadicCallExpr`,
` • • • • Ident f`,
` • • • • End`,
` • • End`,
Expand All @@ -621,7 +643,7 @@ func TestCompile(t *testing.T) {
` • • Ident cond2`,
` • • BlockStmt`,
` • • • ExprStmt`,
` • • • • CallExpr`,
` • • • • NonVariadicCallExpr`,
` • • • • • Ident f`,
` • • • • • End`,
` • • • End`,
Expand All @@ -641,7 +663,7 @@ func TestCompile(t *testing.T) {
` • • Ident cond2`,
` • • BlockStmt`,
` • • • ExprStmt`,
` • • • • CallExpr`,
` • • • • NonVariadicCallExpr`,
` • • • • • Ident f`,
` • • • • • End`,
` • • • End`,
Expand Down Expand Up @@ -684,30 +706,30 @@ func TestCompile(t *testing.T) {
` • Ident x`,
` • Ident y`,
` • End`,
` • CallExpr`,
` • NonVariadicCallExpr`,
` • • Ident f`,
` • • End`,
` • End`,
},

`(chan int)(nil)`: {
`CallExpr`,
`NonVariadicCallExpr`,
` • ParenExpr`,
` • • ChanType send recv`,
` • • • Ident int`,
` • Ident nil`,
` • End`,
},
`(chan<- int)(nil)`: {
`CallExpr`,
`NonVariadicCallExpr`,
` • ParenExpr`,
` • • ChanType send`,
` • • • Ident int`,
` • Ident nil`,
` • End`,
},
`(<-chan int)(nil)`: {
`CallExpr`,
`NonVariadicCallExpr`,
` • ParenExpr`,
` • • ChanType recv`,
` • • • Ident int`,
Expand Down Expand Up @@ -848,13 +870,13 @@ func TestCompile(t *testing.T) {
` • • BasicLit 2`,
` • • End`,
` • • ExprStmt`,
` • • • CallExpr`,
` • • • NonVariadicCallExpr`,
` • • • • Ident f`,
` • • • • End`,
` • • End`,
` • DefaultCaseClause`,
` • • ExprStmt`,
` • • • CallExpr`,
` • • • NonVariadicCallExpr`,
` • • • • Ident g`,
` • • • • End`,
` • • End`,
Expand Down
3 changes: 2 additions & 1 deletion internal/gogrep/gen_operations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ var opPrototypes = []operationProto{
{name: "ParenExpr", tag: "ParenExpr", args: "x"},

{name: "VariadicCallExpr", tag: "CallExpr", args: "fn args...", example: "f(1, xs...)"},
{name: "CallExpr", tag: "CallExpr", args: "fn args...", example: "f(1, xs)"},
{name: "NonVariadicCallExpr", tag: "CallExpr", args: "fn args...", example: "f(1, xs)"},
{name: "CallExpr", tag: "CallExpr", args: "fn args...", example: "f(1, xs) or f(1, xs...)"},

{name: "AssignStmt", tag: "AssignStmt", args: "lhs rhs", value: "token.Token | ':=' or '='", example: "lhs := rhs()"},
{name: "MultiAssignStmt", tag: "AssignStmt", args: "lhs... rhs...", value: "token.Token | ':=' or '='", example: "lhs1, lhs2 := rhs()"},
Expand Down
5 changes: 4 additions & 1 deletion internal/gogrep/match.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,12 @@ func (m *matcher) matchNodeWithInst(inst instruction, n ast.Node) bool {
case opVariadicCallExpr:
n, ok := n.(*ast.CallExpr)
return ok && n.Ellipsis.IsValid() && m.matchNode(n.Fun) && m.matchExprSlice(n.Args)
case opCallExpr:
case opNonVariadicCallExpr:
n, ok := n.(*ast.CallExpr)
return ok && !n.Ellipsis.IsValid() && m.matchNode(n.Fun) && m.matchExprSlice(n.Args)
case opCallExpr:
n, ok := n.(*ast.CallExpr)
return ok && m.matchNode(n.Fun) && m.matchExprSlice(n.Args)

case opSimpleSelectorExpr:
n, ok := n.(*ast.SelectorExpr)
Expand Down
7 changes: 7 additions & 0 deletions internal/gogrep/match_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ func TestMatch(t *testing.T) {
{`$x.Method()`, 0, `a.b.Method2()`},
{`x.Method()`, 0, `y.Method2()`},
{`$x.Method()`, 0, `a.Method(1)`},
{`f($*_)`, 1, `f(xs...)`},
{`f($*_)`, 1, `f(1, xs...)`},
{`f($*_)`, 1, `f(1, 2, xs...)`},
{`f($_, $*_)`, 1, `f(1, 2, xs...)`},
{`f($*_, $_)`, 0, `f(1, 2, xs...)`},
{`f($*_, xs)`, 0, `f(1, 2, xs...)`},
{`f($*_, xs...)`, 1, `f(1, 2, xs...)`},

// Selector expr.
{`$x.Field`, 1, `a.Field`},
Expand Down
129 changes: 65 additions & 64 deletions internal/gogrep/operation_string.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit fc40544

Please sign in to comment.