feature: new rule use-fmt-print (#1389)

print and println built-in functions are not recommended for use-cases other than language boostraping and are not guaranteed to stay in the language.

This commit adds a new rule, use-fmt-print, that spots uses of print and println, and recommends using their fmt equivalents.

Author: chavacava

README.md

@@ -549,6 +549,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`unused-receiver`](./RULES_DESCRIPTIONS.md#unused-receiver)   | n/a    | Suggests to rename or remove unused method receivers    |    no    |  no   |
 | [`use-any`](./RULES_DESCRIPTIONS.md#use-any)          |  n/a   |  Proposes to replace `interface{}` with its alias `any` |    no    |  no   |
 | [`use-errors-new`](./RULES_DESCRIPTIONS.md#use-errors-new) | n/a   | Spots calls to `fmt.Errorf` that can be replaced by `errors.New` |   no    |  no   |
+| [`use-fmt-print`](./RULES_DESCRIPTIONS.md#use-fmt-print) | n/a   | Proposes to replace calls to built-in `print` and `println` with their equivalents from `fmt`. |   no    |  no   |
 | [`useless-break`](./RULES_DESCRIPTIONS.md#useless-break)          |  n/a   |  Warns on useless `break` statements in case clauses |    no    |  no   |
 | [`var-declaration`](./RULES_DESCRIPTIONS.md#var-declaration)     |  n/a   | Reduces redundancies around variable declaration.                |   yes    |  yes  |
 | [`var-naming`](./RULES_DESCRIPTIONS.md#var-naming)          |  allowlist & blocklist of initialisms   | Naming rules.                                                    |   yes    |  no   |

RULES_DESCRIPTIONS.md

@@ -85,6 +85,7 @@ List of all available rules.
   - [unused-receiver](#unused-receiver)
   - [use-any](#use-any)
   - [use-errors-new](#use-errors-new)
+  - [use-fmt-print](#use-fmt-print)
   - [useless-break](#useless-break)
   - [var-declaration](#var-declaration)
   - [var-naming](#var-naming)
@@ -1287,7 +1288,16 @@ _Configuration_: N/A
 
 ## use-errors-new
 
-_Description_: This rules identifies calls to `fmt.Errorf` that can be safely replaced by, the more efficient, `errors.New`.
+_Description_: This rule identifies calls to `fmt.Errorf` that can be safely replaced by, the more efficient, `errors.New`.
+
+_Configuration_: N/A
+
+## use-fmt-print
+
+_Description_: This rule proposes to replace calls to built-in `print` and `println` with their equivalents from `fmt` standard package.
+
+`print` and `println` built-in functions are not recommended for use-cases other than
+[language boostraping and are not guaranteed to stay in the language](https://go.dev/ref/spec#Bootstrapping).
 
 _Configuration_: N/A
 

config/config.go

@@ -103,6 +103,7 @@ var allRules = append([]lint.Rule{
 	&rule.UseErrorsNewRule{},
 	&rule.RedundantTestMainExitRule{},
 	&rule.UnnecessaryFormatRule{},
+	&rule.UseFmtPrintRule{},
 }, defaultRules...)
 
 // allFormatters is a list of all available formatters to output the linting results.

rule/use_fmt_print.go

@@ -0,0 +1,107 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"strings"
+
+	"github.com/mgechev/revive/internal/astutils"
+	"github.com/mgechev/revive/lint"
+)
+
+// UseFmtPrintRule lints calls to print and println.
+type UseFmtPrintRule struct{}
+
+// Apply applies the rule to given file.
+func (r *UseFmtPrintRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	redefinesPrint, redefinesPrintln := r.analyzeRedefinitions(file.AST.Decls)
+	// Here we could check if both are redefined and if it's the case return nil
+	// but the check being false 99.9999999% of the cases we don't
+
+	var failures []lint.Failure
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := lintUseFmtPrint{onFailure, redefinesPrint, redefinesPrintln}
+	ast.Walk(w, file.AST)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (*UseFmtPrintRule) Name() string {
+	return "use-fmt-print"
+}
+
+type lintUseFmtPrint struct {
+	onFailure        func(lint.Failure)
+	redefinesPrint   bool
+	redefinesPrintln bool
+}
+
+func (w lintUseFmtPrint) Visit(node ast.Node) ast.Visitor {
+	ce, ok := node.(*ast.CallExpr)
+	if !ok {
+		return w // nothing to do, the node is not a call
+	}
+
+	id, ok := (ce.Fun).(*ast.Ident)
+	if !ok {
+		return nil
+	}
+
+	name := id.Name
+	switch name {
+	case "print":
+		if w.redefinesPrint {
+			return nil // it's a call to user-defined print
+		}
+	case "println":
+		if w.redefinesPrintln {
+			return nil // it's a call to user-defined println
+		}
+	default:
+		return nil // nothing to do, the call is not println(...) nor print(...)
+	}
+
+	callArgs := w.callArgsAsStr(ce.Args)
+	w.onFailure(lint.Failure{
+		Confidence: 1,
+		Node:       node,
+		Category:   lint.FailureCategoryBadPractice,
+		Failure:    fmt.Sprintf(`avoid using built-in function %q, replace it by "fmt.F%s(os.Stderr, %s)"`, name, name, callArgs),
+	})
+
+	return w
+}
+
+func (lintUseFmtPrint) callArgsAsStr(args []ast.Expr) string {
+	strs := []string{}
+	for _, expr := range args {
+		strs = append(strs, astutils.GoFmt(expr))
+	}
+
+	return strings.Join(strs, ", ")
+}
+
+func (UseFmtPrintRule) analyzeRedefinitions(decls []ast.Decl) (redefinesPrint, redefinesPrintln bool) {
+	for _, decl := range decls {
+		fnDecl, ok := decl.(*ast.FuncDecl)
+		if !ok {
+			continue // not a function declaration
+		}
+
+		if fnDecl.Recv != nil {
+			continue // it´s a method (not function) declaration
+		}
+
+		switch fnDecl.Name.Name {
+		case "print":
+			redefinesPrint = true
+		case "println":
+			redefinesPrintln = true
+		}
+	}
+	return redefinesPrint, redefinesPrintln
+}

test/use_fmt_print_test.go

@@ -0,0 +1,12 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+func TestUseFmtPrint(t *testing.T) {
+	testRule(t, "use_fmt_print", &rule.UseFmtPrintRule{})
+	testRule(t, "use_fmt_print_with_redefinition", &rule.UseFmtPrintRule{})
+}

testdata/use_fmt_print.go

@@ -0,0 +1,21 @@
+package fixtures
+
+import (
+	"fmt"
+)
+
+type useFmtPrintT struct{}
+
+func (useFmtPrintT) print(s string)   {}
+func (useFmtPrintT) println(s string) {}
+
+func useFmtPrint() {
+	fmt.Println("just testing")
+	fmt.Print("just testing")
+	t := useFmtPrintT{}
+	t.print("just testing")
+	t.println("just testing")
+
+	println("just testing", something)   // MATCH /avoid using built-in function "println", replace it by "fmt.Fprintln(os.Stderr, "just testing", something)"/
+	print("just testing", some, thing+1) // MATCH /avoid using built-in function "print", replace it by "fmt.Fprint(os.Stderr, "just testing", some, thing + 1)"/
+}

testdata/use_fmt_print_with_redefinition.go

@@ -0,0 +1,24 @@
+package fixtures
+
+import (
+	"fmt"
+)
+
+func print()   {}
+func println() {}
+
+type useFmtPrintT struct{}
+
+func (useFmtPrintT) print(s string)   {}
+func (useFmtPrintT) println(s string) {}
+
+func useFmtPrint() {
+	fmt.Println("just testing")
+	fmt.Print("just testing")
+	t := useFmtPrintT{}
+	t.print("just testing")
+	t.println("just testing")
+
+	println("just testing")
+	print("just testing")
+}