Add a linter for detecting fmt.Errorf w/ %w (#1952)

It was decided by the team that we prefer errors.Wrap over the standard
library error wrapping mechanism because pkg/errors provides a more
consistent experience by default. This adds a simple linter to detect
usages of the standard library error wrapping.

pkgerrlint/README.md

@@ -0,0 +1,56 @@
+# pkgerrlint
+
+`pkgerrlint` is a linting utility for the Go programming language that analyzes
+a Go module and detects instances of `fmt.Errorf` when used with the `%w`
+formatting verb. If detected, it suggests using `github.com/pkg/errors.Wrap`
+instead of `fmt.Errorf` and can automatically rewrite the code to use the
+recommended package.
+
+## Installation
+
+To install `pkgerrlint`, follow these steps:
+
+1. Clone the `azure-container-networking` repository:
+
+```
+git clone https://github.com/Azure/azure-container-networking.git
+```
+
+2. Navigate to the project directory:
+
+```
+cd pkgerrlint
+```
+
+3. Build the binary:
+
+```
+go build -o pkgerrlint
+```
+
+Optionally, you can add the binary to your `$PATH` or move it to a directory
+that is already in your `$PATH`.
+
+## Usage
+
+To analyze a Go module, run the following command:
+
+```
+./pkgerrlint /path/to/go/module
+```
+
+This command will print the file, line, and column of any detected instances of
+`fmt.Errorf` with the `%w` verb, along with the message "use
+`github.com/pkg/errors.Wrap` to wrap errors instead of `fmt.Errorf`."
+
+To automatically rewrite instances of `fmt.Errorf` with the `%w` verb to use
+`errors.Wrap` or `errors.WrapF` instead, run the command with the `--rewrite`
+flag:
+
+```
+./pkgerrlint --rewrite /path/to/go/module
+```
+
+Please note that this utility assumes the `github.com/pkg/errors` package is
+imported in the source files. It may be necessary to manually alter the imports
+of modified files using a utility like `goimports`.

pkgerrlint/cmd/pkgerrlint/internal/run.go

@@ -0,0 +1,86 @@
+package internal
+
+import (
+	"errors"
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+
+	pkgerrs "github.com/pkg/errors"
+)
+
+func Run(out io.Writer, args ...string) error {
+	if len(args) != 1 {
+		return errors.New("usage: golintwrap <module_path>")
+	}
+
+	modulePath := args[0]
+	err := inspectFiles(out, modulePath)
+	if err != nil {
+		pkgerrs.Wrap(err, "inspecting files")
+	}
+	return nil
+}
+
+func inspectFiles(out io.Writer, root string) error {
+	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return pkgerrs.Wrapf(err, "walking %q", path)
+		}
+
+		if !info.IsDir() && filepath.Ext(path) == ".go" {
+			err := inspectFile(out, path)
+			if err != nil {
+				return pkgerrs.Wrapf(err, "inspecting file %q", path)
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		return pkgerrs.Wrap(err, "walking filepath")
+	}
+	return nil
+}
+
+func inspectFile(out io.Writer, file string) error {
+	fset := token.NewFileSet()
+	node, err := parser.ParseFile(fset, file, nil, 0)
+	if err != nil {
+		return pkgerrs.Wrapf(err, "parsing file %q", file)
+	}
+
+	ast.Inspect(node, func(n ast.Node) bool {
+		callExpr, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+
+		selExpr, ok := callExpr.Fun.(*ast.SelectorExpr)
+		if !ok {
+			return true
+		}
+
+		if selExpr.Sel.Name == "Errorf" {
+			pkgIdent, ok := selExpr.X.(*ast.Ident)
+			if ok && pkgIdent.Name == "fmt" {
+				for _, arg := range callExpr.Args {
+					basicLit, ok := arg.(*ast.BasicLit)
+					if ok && basicLit.Kind == token.STRING && strings.Contains(basicLit.Value, "%w") {
+						position := fset.Position(callExpr.Pos())
+						fmt.Fprintf(out, "%s:%d:%d: use `github.com/pkg/errors.Wrap` to wrap errors instead of `fmt.Errorf`\n", position.Filename, position.Line, position.Column)
+						break
+					}
+				}
+			}
+		}
+
+		return true
+	})
+
+	return nil
+}

pkgerrlint/cmd/pkgerrlint/internal/run_test.go

@@ -0,0 +1,117 @@
+package internal_test
+
+import (
+	"bufio"
+	"bytes"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/Azure/azure-container-networking/pkgerrlint/cmd/pkgerrlint/internal"
+)
+
+func TestRun(t *testing.T) {
+	runTests, err := filepath.Glob("./testdata/*.go")
+	if err != nil {
+		t.Fatal("error loading test files: err:", err)
+	}
+
+	for _, testPath := range runTests {
+		testPath := testPath
+
+		t.Run(testPath, func(t *testing.T) {
+			// similarly to example tests, each test file has, at its end, a set of
+			// comments depicting the expected standard output when run on that file.
+			// Example tests themselves can't be used because the Go source file is not
+			// being executed.
+			testFile, err := os.Open(testPath)
+			if err != nil {
+				t.Fatal("error opening test file: err:", err)
+			}
+			defer testFile.Close()
+
+			sub, err := io.ReadAll(testFile)
+			if err != nil {
+				t.Fatal("error reading contents of test file: err:", err)
+			}
+
+			// extract the expected output
+			scn := bufio.NewScanner(bytes.NewReader(sub))
+			exp := []string{}
+			scanningOutput := false // serves as scanner state for loading exp
+			for scn.Scan() {
+				line := scn.Text()
+
+				// search for "Output" as a signifier that the expected output follows
+				if strings.HasPrefix(line, "// Output:") {
+					scanningOutput = true
+					continue
+				}
+
+				const commentStart = "//"
+
+				if scanningOutput {
+					if strings.HasPrefix(line, commentStart) {
+						next := strings.TrimPrefix(line, commentStart)
+						next = strings.TrimLeft(next, " ") // remove leading spaces as well
+						exp = append(exp, next)
+					} else {
+						// the end of comments signifies the end of the Output block
+						scanningOutput = false
+					}
+				}
+			}
+
+			// we need a fake "standard output"
+			stdout := bytes.NewBufferString("")
+			err = internal.Run(stdout, testPath)
+			if err != nil {
+				t.Fatal("unexpected error: err:", err)
+			}
+
+			outLines := bytes.Split(stdout.Bytes(), []byte{'\n'})
+			got := make([]string, 0, len(outLines))
+			for _, line := range outLines {
+				// trim empty newlines:
+				if string(line) == "" {
+					continue
+				}
+				got = append(got, string(line))
+			}
+
+			// ensure the output was as expected
+			if len(got) != len(exp) {
+				diff(t, exp, got)
+			}
+
+			for lineIdx := range got {
+				gotLine := got[lineIdx]
+				expLine := exp[lineIdx]
+
+				if expLine != gotLine {
+					diff(t, exp, got)
+				}
+			}
+		})
+	}
+}
+
+func diff(t *testing.T, exp, got []string) {
+	t.Helper()
+	t.Log("expected output differs from received output:")
+	t.Logf("exp (len %d):\n", len(exp))
+
+	for _, line := range exp {
+		t.Log(line)
+	}
+
+	t.Logf("got (len %d):\n", len(got))
+
+	for _, line := range got {
+		t.Logf(line)
+	}
+
+	t.FailNow()
+}

pkgerrlint/cmd/pkgerrlint/internal/testdata/no_output.go

@@ -0,0 +1,9 @@
+package main
+
+import "fmt"
+
+func main() {
+	fmt.Println("no output because there's no errors here!")
+}
+
+// Output:

pkgerrlint/cmd/pkgerrlint/internal/testdata/no_wrapping.go

@@ -0,0 +1,12 @@
+package main
+
+import (
+	"fmt"
+)
+
+func main() {
+	err := fmt.Errorf("no wrapping verb: %d", 42)
+	fmt.Println("error:", err.Error())
+}
+
+// Output:

pkgerrlint/cmd/pkgerrlint/internal/testdata/simple.go

@@ -0,0 +1,15 @@
+package main
+
+import (
+	"errors"
+	"fmt"
+)
+
+func main() {
+	baseErr := errors.New("boom!")
+	err := fmt.Errorf("wrapping: %w", err)
+	fmt.Println("error:", err.Error())
+}
+
+// Output:
+// testdata/simple.go:10:9: use `github.com/pkg/errors.Wrap` to wrap errors instead of `fmt.Errorf`

pkgerrlint/cmd/pkgerrlint/main.go

@@ -0,0 +1,41 @@
+package main
+
+import (
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/Azure/azure-container-networking/pkgerrlint/cmd/pkgerrlint/internal"
+)
+
+var _ io.Writer = &DetectingWriter{}
+
+type DetectingWriter struct {
+	DidWrite bool
+	w        io.Writer
+}
+
+func (d *DetectingWriter) Write(in []byte) (int, error) {
+	d.DidWrite = true
+	return d.w.Write(in)
+}
+
+func main() {
+	w := &DetectingWriter{
+		w: os.Stdout,
+	}
+
+	// this adhere's to the exit codes returned by `go test`. If there's abnormal
+	// errors (e.g. compilation failures), an exit code of "2" is returned.
+	// Otherwise linting failures produce an error code of "1". Success is a "0"
+	// with no output.
+	if err := internal.Run(w, os.Args[1:]...); err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(2)
+	}
+
+	if w.DidWrite {
+		// the presence of any output on standard out indicates a linting failure
+		os.Exit(1)
+	}
+}

pkgerrlint/go.mod

@@ -0,0 +1,5 @@
+module github.com/Azure/azure-container-networking/pkgerrlint
+
+go 1.19
+
+require github.com/pkg/errors v0.9.1

pkgerrlint/go.sum

@@ -0,0 +1,2 @@
+github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
+github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=