tools/syz-linter: improve duplicate parameter type detection

Duplicate parameter type warning suggests to replace
"a int, b int" with "a, b int" in function declarations.
Currently it does not work for non-builtin types because
we compare type pointers directly but they are different
for non-builtin types. Compare type strings instead.

2 files changed, 26 insertions, 12 deletions

diff --git a/tools/syz-linter/linter.go b/tools/syz-linter/linter.go
index 52da71b1f..b52e907ba 100644
--- a/tools/syz-linter/linter.go
+++ b/tools/syz-linter/linter.go
@@ -70,7 +70,7 @@ func run(p *analysis.Pass) (interface{}, error) {
 			switch n := n.(type) {
 			case *ast.BinaryExpr:
 				pass.checkStringLenCompare(n)
-			case *ast.FuncType:
+			case *ast.FuncDecl:
 				pass.checkFuncArgs(n)
 			case *ast.CallExpr:
 				pass.checkFlagDefinition(n)
@@ -98,8 +98,8 @@ func (pass *Pass) report(pos ast.Node, msg string, args ...interface{}) {
 	})
 }
 
-func (pass *Pass) typ(e ast.Expr) types.Type {
-	return pass.TypesInfo.Types[e].Type
+func (pass *Pass) typ(e ast.Expr) string {
+	return pass.TypesInfo.Types[e].Type.String()
 }
 
 // checkComment warns about C++-style multiline comments (we don't use them in the codebase)
@@ -170,7 +170,7 @@ func (pass *Pass) isStringLenCall(n ast.Expr) bool {
 	if !ok || fun.Name != "len" {
 		return false
 	}
-	return pass.typ(call.Args[0]).String() == "string"
+	return pass.typ(call.Args[0]) == "string"
 }
 
 func (pass *Pass) isIntZeroLiteral(n ast.Expr) bool {
@@ -179,23 +179,29 @@ func (pass *Pass) isIntZeroLiteral(n ast.Expr) bool {
 }
 
 // checkFuncArgs checks for "func foo(a int, b int)" -> "func foo(a, b int)".
-func (pass *Pass) checkFuncArgs(n *ast.FuncType) {
-	pass.checkFuncArgList(n.Params.List)
-	if n.Results != nil {
-		pass.checkFuncArgList(n.Results.List)
+func (pass *Pass) checkFuncArgs(n *ast.FuncDecl) {
+	variadic := pass.TypesInfo.ObjectOf(n.Name).(*types.Func).Type().(*types.Signature).Variadic()
+	pass.checkFuncArgList(n.Type.Params.List, variadic)
+	if n.Type.Results != nil {
+		pass.checkFuncArgList(n.Type.Results.List, false)
 	}
 }
 
-func (pass *Pass) checkFuncArgList(fields []*ast.Field) {
+func (pass *Pass) checkFuncArgList(fields []*ast.Field, variadic bool) {
 	firstBad := -1
-	var prev types.Type
+	var prev string
 	for i, field := range fields {
 		if len(field.Names) == 0 {
 			pass.reportFuncArgs(fields, firstBad, i)
-			firstBad, prev = -1, nil
+			firstBad, prev = -1, ""
 			continue
 		}
 		this := pass.typ(field.Type)
+		// For variadic functions the actual type of the last argument is a slice,
+		// but we don't want to warn on "a []int, b ...int".
+		if variadic && i == len(fields)-1 {
+			this = "..." + this
+		}
 		if prev != this {
 			pass.reportFuncArgs(fields, firstBad, i)
 			firstBad, prev = -1, this
@@ -218,7 +224,7 @@ func (pass *Pass) reportFuncArgs(fields []*ast.Field, first, last int) {
 			names += ", " + name.Name
 		}
 	}
-	pass.report(fields[first], "Use '%v %v'", names[2:], fields[first].Type)
+	pass.report(fields[first], "Use '%v %v'", names[2:], pass.typ(fields[first].Type))
 }
 
 func (pass *Pass) checkFlagDefinition(n *ast.CallExpr) {
diff --git a/tools/syz-linter/testdata/src/lintertest/lintertest.go b/tools/syz-linter/testdata/src/lintertest/lintertest.go
index 778ccb6ae..51700726e 100644
--- a/tools/syz-linter/testdata/src/lintertest/lintertest.go
+++ b/tools/syz-linter/testdata/src/lintertest/lintertest.go
@@ -47,6 +47,9 @@ func funcArgsGood(a, b int) (int, int) {
 	return 0, 0
 }
 
+func funcArgsGood2(a []int, b ...int) {
+}
+
 func funcArgsBad0(a int, b int) { // want "Use 'a, b int'"
 }
 
@@ -57,6 +60,11 @@ func funcArgsBad1() (a int, b int) { // want "Use 'a, b int'"
 func funcArgsBad2(a int16, b, c uint32, d uint32, e int16) { // want "Use 'b, c, d uint32'"
 }
 
+type Foo struct{}
+
+func funcArgsBad3(s string, b *Foo, c *Foo) { // want "b, c \\*lintertest\\.Foo"
+}
+
 func flagDefinitions() {
 	flag.Int("good", 0, "fine description")
 	flag.Int("camelCase", 0, "fine description") // want "Don't use Capital letters in flag names"