tools/syz-linter: add a check for fmt.Printf/Fprintf messages

2 files changed, 53 insertions, 23 deletions

diff --git a/tools/syz-linter/linter.go b/tools/syz-linter/linter.go
index e81200521..d6fae7fe4 100644
--- a/tools/syz-linter/linter.go
+++ b/tools/syz-linter/linter.go
@@ -222,17 +222,8 @@ func (pass *Pass) reportFuncArgs(fields []*ast.Field, first, last int) {
 
 // checkLogErrorFormat warns about log/error messages starting with capital letter or ending with a period.
 func (pass *Pass) checkLogErrorFormat(n *ast.CallExpr) {
-	fun, ok := n.Fun.(*ast.SelectorExpr)
-	if !ok {
-		return
-	}
-	arg := 0
-	switch fmt.Sprintf("%v.%v", fun.X, fun.Sel) {
-	case "log.Print", "log.Printf", "log.Fatal", "log.Fatalf", "fmt.Error", "fmt.Errorf":
-		arg = 0
-	case "log.Logf":
-		arg = 1
-	default:
+	arg, newLine, sure := pass.logFormatArg(n)
+	if arg == -1 {
 		return
 	}
 	lit, ok := n.Args[arg].(*ast.BasicLit)
@@ -248,10 +239,18 @@ func (pass *Pass) checkLogErrorFormat(n *ast.CallExpr) {
 		pass.report(lit, "Don't use empty log/error messages")
 		return
 	}
+	// Some Printf's legitimately don't need \n, so this check is based on a heuristic.
+	// Printf's that don't need \n tend to contain % and are short.
+	if !sure && len(val) < 25 && (len(val) < 10 || strings.Contains(val, "%")) {
+		return
+	}
 	if val[ln-1] == '.' && (ln < 3 || val[ln-2] != '.' || val[ln-3] != '.') {
 		pass.report(lit, "Don't use period at the end of log/error messages")
 	}
-	if val[ln-1] == '\n' {
+	if newLine && val[ln-1] != '\n' {
+		pass.report(lit, "Add \\n at the end of printed messages")
+	}
+	if !newLine && val[ln-1] == '\n' {
 		pass.report(lit, "Don't use \\n at the end of log/error messages")
 	}
 	if ln >= 2 && unicode.IsUpper(rune(val[0])) && unicode.IsLower(rune(val[1])) &&
@@ -260,6 +259,27 @@ func (pass *Pass) checkLogErrorFormat(n *ast.CallExpr) {
 	}
 }
 
+func (pass *Pass) logFormatArg(n *ast.CallExpr) (arg int, newLine, sure bool) {
+	fun, ok := n.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return -1, false, false
+	}
+	switch fmt.Sprintf("%v.%v", fun.X, fun.Sel) {
+	case "log.Print", "log.Printf", "log.Fatal", "log.Fatalf", "fmt.Error", "fmt.Errorf":
+		return 0, false, true
+	case "log.Logf":
+		return 1, false, true
+	case "fmt.Print", "fmt.Printf":
+		return 0, true, false
+	case "fmt.Fprint", "fmt.Fprintf":
+		if w, ok := n.Args[0].(*ast.SelectorExpr); !ok || fmt.Sprintf("%v.%v", w.X, w.Sel) != "os.Stderr" {
+			break
+		}
+		return 1, true, true
+	}
+	return -1, false, false
+}
+
 var publicIdentifier = regexp.MustCompile(`^[A-Z][[:alnum:]]+(\.[[:alnum:]]+)+ `)
 
 // checkVarDecl warns about unnecessary long variable declarations "var x type = foo".
diff --git a/tools/syz-linter/testdata/src/lintertest/lintertest.go b/tools/syz-linter/testdata/src/lintertest/lintertest.go
index 341108c0b..9fb1d89c9 100644
--- a/tools/syz-linter/testdata/src/lintertest/lintertest.go
+++ b/tools/syz-linter/testdata/src/lintertest/lintertest.go
@@ -6,6 +6,7 @@ package lintertest
 import (
 	"fmt"
 	"log"
+	"os"
 )
 
 /* some comment */ // want "Use C-style comments // instead of /* */"
@@ -49,29 +50,38 @@ func funcArgsBad0(a int, b int) { // want "Use 'a, b int'"
 }
 
 func funcArgsBad1() (a int, b int) { // want "Use 'a, b int'"
-	return 0, 0	// lower-case comment is OK
+	return 0, 0 // lower-case comment is OK
 }
 
 func funcArgsBad2(a int16, b, c uint32, d uint32, e int16) { // want "Use 'b, c, d uint32'"
 }
 
 func logErrorMessages() {
-	fmt.Errorf("good message")
-	fmt.Errorf("good message %v", 0)
 	msg := "good message"
+	err := fmt.Errorf("good message")
+	fmt.Errorf("good message %v", 0)
 	fmt.Errorf(msg)
 	log.Printf("good message")
 	log.Print("good message")
 	log.Print("Using.An.Identifier is ok as well")
 	log.Print(msg)
+	fmt.Printf("%s", msg)
+	fmt.Printf("fragment")
+	fmt.Printf("Fragment Fragment %s", msg)
+	fmt.Fprintf(nil, "These can be anything")
 
-	fmt.Errorf("Bad message")	// want "Don't start log/error messages with a Capital letter"
-	log.Fatalf("Bad message %v", 1) // want "Don't start log/error messages with a Capital letter"
-	log.Printf("Bad message %v", 1) // want "Don't start log/error messages with a Capital letter"
-	log.Print("Bad message") // want "Don't start log/error messages with a Capital letter"
-	log.Print("also ad message.") // want "Don't use period at the end of log/error messages"
-	log.Print("no new lines\n") // want "Don't use \\\\n at the end of log/error messages"
-	log.Print("") // want "Don't use empty log/error messages"
+	fmt.Errorf("Bad message")                                           // want "Don't start log/error messages with a Capital letter"
+	log.Fatalf("Bad message %v", 1)                                     // want "Don't start log/error messages with a Capital letter"
+	log.Printf("Bad message %v", 1)                                     // want "Don't start log/error messages with a Capital letter"
+	log.Print("Bad message")                                            // want "Don't start log/error messages with a Capital letter"
+	log.Print("also ad message.")                                       // want "Don't use period at the end of log/error messages"
+	log.Print("no new lines\n")                                         // want "Don't use \\\\n at the end of log/error messages"
+	log.Print("")                                                       // want "Don't use empty log/error messages"
+	fmt.Printf("Real output message with capital letter\n")             // want "Don't start log/error messages with a Capital letter"
+	fmt.Printf("real output message without newline")                   // want "Add \\\\n at the end of printed messages"
+	fmt.Fprintf(os.Stderr, "Real output message with capital letter\n") // want "Don't start log/error messages with a Capital letter"
+	fmt.Fprintf(os.Stderr, "real output message without newline")       // want "Add \\\\n at the end of printed messages"
+	fmt.Fprintf(os.Stderr, "%v", err)                                   // want "Add \\\\n at the end of printed messages"
 }
 
 func varDecls() {
@@ -79,6 +89,6 @@ func varDecls() {
 	b := 0
 	c := int64(0)
 	var _ int = 0
-	var d int = 0	// want "Don't use both var, type and value in variable declarations"
+	var d int = 0 // want "Don't use both var, type and value in variable declarations"
 	_, _, _, _ = a, b, c, d
 }