tools: Fix incompatible dependencies for new Gitaly linter

In d1c74493a (Add golangci-lint custom linter infrastructure,
2023-02-15), we have added a Gitaly-specific custom linter that is
plugged into golangci-lint. For the time being, we agreed to just make
it use the same `go.mod` as the main module, mostly because we wanted
`make test` to also execute its tests.

This is causing problems already, though. The Renovate bot has decided
to immediately try and upgrade the new dependency on golang.org/x/tools
to the most recent version. But this causes CI failures now because
golangci-lint requires all plugins to always use the same version of
that package as it uses itself.

Fix this by moving the Gitaly-specific linting infrastructure into a
subpackage of `tools/golangci-lint` and make it reuse the `go.mod` file.
This ensures that they always use compatible versions and that the main
production-level `go.mod` file is not tied to a specific dependency
version.

9 files changed, 436 insertions, 0 deletions

diff --git a/tools/golangci-lint/gitaly/errorwrap.go b/tools/golangci-lint/gitaly/errorwrap.go
new file mode 100644
index 000000000..af3530a60
--- /dev/null
+++ b/tools/golangci-lint/gitaly/errorwrap.go
@@ -0,0 +1,95 @@
+package main
+
+import (
+	"go/ast"
+	"go/token"
+	"go/types"
+	"regexp"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// NewErrorWrapAnalyzer returns an analyzer to detect unexpected error interpolation without %w.
+// After error wrapping was introduced, we encourage wrapping error with %w when constructing a new
+// error. The new error contains the original error able to be unwrapped later.
+//
+//   - Bad
+//     return structerr.NewInvalidArgument("GetRepoPath: %s", err)
+//
+//   - Bad
+//     return structerr.NewCanceled("%v", err)
+//
+//   - Bad
+//     return nil, fmt.Errorf("unmarshalling json: %v", err)
+//
+//   - Good
+//     return structerr.NewCanceled("%w", err)
+//
+//   - Good
+//     return nil, fmt.Errorf("failed unmarshalling json: %w", err)
+//
+// For more information:
+// https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md#use-w-when-wrapping-errors
+func NewErrorWrapAnalyzer(rules []string) *analysis.Analyzer {
+	return &analysis.Analyzer{
+		Name: "error_wrap",
+		Doc: `Always wrap an error with %w:
+	https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md#use-w-when-wrapping-errors`,
+		Run: runErrorWrapAnalyzer(rules),
+	}
+}
+
+var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
+
+// Over-simplified pattern to parse interpolation format. This linter targets error wrapping only.
+// Most of the time, %s or %v or %q are used. This pattern is good enough to detect most cases.
+// The std uses a proper parser, which is overkilled for re-implemented:
+// https://github.com/golang/go/blob/518889b35cb07f3e71963f2ccfc0f96ee26a51ce/src/fmt/print.go#L1026
+var formatPattern = regexp.MustCompile(`%.`)
+
+func analyzeErrorInterpolation(pass *analysis.Pass, call *ast.CallExpr) {
+	if len(call.Args) <= 1 {
+		// Irrelevant call, or static format
+		return
+	}
+
+	if str, ok := call.Args[0].(*ast.BasicLit); ok && str.Kind == token.STRING {
+		verbs := formatPattern.FindAllIndex([]byte(str.Value), -1)
+
+		if len(verbs) != len(call.Args)-1 {
+			// Mismatched format verbs and arguments; or our regexp is not correct
+			return
+		}
+		for index, arg := range call.Args[1:] {
+			argType := pass.TypesInfo.Types[arg].Type
+			if types.Implements(argType, errorType) {
+				verb := str.Value[verbs[index][0]:verbs[index][1]]
+				if verb != "%w" {
+					pass.Report(analysis.Diagnostic{
+						Pos:            token.Pos(int(str.Pos()) + verbs[index][0]),
+						End:            token.Pos(int(str.Pos()) + verbs[index][1]),
+						Message:        "please use %w to wrap errors",
+						SuggestedFixes: nil,
+					})
+				}
+			}
+		}
+	}
+}
+
+func runErrorWrapAnalyzer(rules []string) func(*analysis.Pass) (interface{}, error) {
+	return func(pass *analysis.Pass) (interface{}, error) {
+		matcher := NewMatcher(pass)
+		for _, file := range pass.Files {
+			ast.Inspect(file, func(n ast.Node) bool {
+				if call, ok := n.(*ast.CallExpr); ok {
+					if matcher.MatchFunction(call, rules) {
+						analyzeErrorInterpolation(pass, call)
+					}
+				}
+				return true
+			})
+		}
+		return nil, nil
+	}
+}
diff --git a/tools/golangci-lint/gitaly/errorwrap_test.go b/tools/golangci-lint/gitaly/errorwrap_test.go
new file mode 100644
index 000000000..529112ded
--- /dev/null
+++ b/tools/golangci-lint/gitaly/errorwrap_test.go
@@ -0,0 +1,22 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestNewErrorWrapAnalyzer(t *testing.T) {
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Failed to get wd: %s", err)
+	}
+
+	testdata := filepath.Join(wd, "testdata")
+	analyzer := NewErrorWrapAnalyzer([]string{
+		"fmt.Errorf",
+	})
+	analysistest.Run(t, testdata, analyzer, "errorwrap")
+}
diff --git a/tools/golangci-lint/gitaly/lint.go b/tools/golangci-lint/gitaly/lint.go
new file mode 100644
index 000000000..f03e118eb
--- /dev/null
+++ b/tools/golangci-lint/gitaly/lint.go
@@ -0,0 +1,29 @@
+package main
+
+import (
+	"golang.org/x/tools/go/analysis"
+)
+
+type analyzerPlugin struct{}
+
+func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
+	return []*analysis.Analyzer{
+		NewQuoteInterpolationAnalyzer([]string{
+			"fmt.*",
+		}),
+		NewErrorWrapAnalyzer([]string{
+			"fmt.Errorf",
+			"gitlab.com/gitlab-org/gitaly/v15/internal/structerr.*",
+		}),
+	}
+}
+
+// AnalyzerPlugin is a convention of golangci-lint to implement a custom linter. This variable
+// must implement `AnalyzerPlugin` interface:
+//
+//	type AnalyzerPlugin interface {
+//		GetAnalyzers() []*analysis.Analyzer
+//	}
+//
+// For more information, please visit https://golangci-lint.run/contributing/new-linters/
+var AnalyzerPlugin analyzerPlugin
diff --git a/tools/golangci-lint/gitaly/matcher.go b/tools/golangci-lint/gitaly/matcher.go
new file mode 100644
index 000000000..9b180a726
--- /dev/null
+++ b/tools/golangci-lint/gitaly/matcher.go
@@ -0,0 +1,88 @@
+package main
+
+import (
+	"go/ast"
+	"go/types"
+	"regexp"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// Matcher implements some helper methods to filter relevant AST nodes for linter checks. It depends
+// on the TypeInfo of analysis.Pass object passed in the analyzer.
+type Matcher struct {
+	typesInfo *types.Info
+}
+
+// NewMatcher creates a new Matcher object from the input analysis pass.
+func NewMatcher(pass *analysis.Pass) *Matcher {
+	return &Matcher{
+		typesInfo: pass.TypesInfo,
+	}
+}
+
+var funcNamePattern = regexp.MustCompile(`^\(?([^\\)].*)\)?\.(.*)$`)
+
+// MatchFunction returns true if the input call expression matches any of the list of input rules.
+// A rule is a human-friend full name of a function. Some examples:
+//   - A public package function:
+//     "fmt.Errorf"
+//   - Match all package functions:
+//     "fmt.*"
+//   - A public function of a dependent package:
+//     "gitlab.com/gitlab-org/gitaly/v15/internal/structerr.NewInternal",
+//   - A function of a struct inside a package:
+//     "(*gitlab.com/gitlab-org/gitaly/v15/internal/structerr.Error).Unwrap",
+//
+// This Matcher doesn't support interface match (yet).
+func (m *Matcher) MatchFunction(call *ast.CallExpr, rules []string) bool {
+	name := m.functionName(call)
+	if name == "" {
+		return false
+	}
+	for _, rule := range rules {
+		if m.matchRule(name, rule) {
+			return true
+		}
+	}
+	return false
+}
+
+func (m *Matcher) matchRule(name, rule string) bool {
+	nameMatches := funcNamePattern.FindStringSubmatch(name)
+	if len(nameMatches) == 0 {
+		return false
+	}
+
+	ruleMatches := funcNamePattern.FindStringSubmatch(rule)
+	if len(ruleMatches) == 0 {
+		return false
+	}
+
+	if nameMatches[1] != ruleMatches[1] {
+		return false
+	}
+
+	return ruleMatches[2] == "*" || nameMatches[2] == ruleMatches[2]
+}
+
+func (m *Matcher) functionName(call *ast.CallExpr) string {
+	fn, ok := m.getFunction(call)
+	if !ok {
+		return ""
+	}
+
+	return fn.FullName()
+}
+
+func (m *Matcher) getFunction(call *ast.CallExpr) (*types.Func, bool) {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return nil, false
+	}
+	fn, ok := m.typesInfo.ObjectOf(sel.Sel).(*types.Func)
+	if !ok {
+		return nil, false
+	}
+	return fn, true
+}
diff --git a/tools/golangci-lint/gitaly/quote.go b/tools/golangci-lint/gitaly/quote.go
new file mode 100644
index 000000000..d9648b0b5
--- /dev/null
+++ b/tools/golangci-lint/gitaly/quote.go
@@ -0,0 +1,84 @@
+package main
+
+import (
+	"go/ast"
+	"go/token"
+	"regexp"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// NewQuoteInterpolationAnalyzer returns an analyzer to detect manually quoted string interpolation
+// with '%s' and "%s". Quoting this way doesn't escape special characters such as endline and makes
+// debugging harder later. We encourage to use %q verb instead.
+//
+//   - Bad
+//     return fmt.Errorf("gl_id='%s' is invalid", glID)
+//
+//   - Bad
+//     fmt.Sprintf("fatal: not a git repository: '%s'", repoPath)
+//
+//   - Good
+//     return fmt.Errorf("gl_id=%q is invalid", glID)
+//
+//   - Good
+//     fmt.Sprintf("fatal: not a git repository: %q", repoPath)
+//
+// For more information:
+// https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md#use-q-when-interpolating-strings
+func NewQuoteInterpolationAnalyzer(rules []string) *analysis.Analyzer {
+	return &analysis.Analyzer{
+		Name: "string_interpolation_quote",
+		Doc: `Unless it would lead to incorrect results, always use %q when
+		interpolating strings. For more information:
+		https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md#use-q-when-interpolating-strings`,
+		Run: runStringInterpolationQuoteAnalyzer(rules),
+	}
+}
+
+// offendedFormatPattern matches string interpolation having '%s' and "%s" format
+var offendedFormatPattern = regexp.MustCompile(`['"]%s['"]`)
+
+func analyzeInterpolation(call *ast.CallExpr, pass *analysis.Pass) {
+	if len(call.Args) <= 1 {
+		return
+	}
+
+	if str, ok := call.Args[0].(*ast.BasicLit); ok && str.Kind == token.STRING {
+		value := str.Value
+		if strings.HasPrefix(value, `'`) || strings.HasPrefix(value, `"`) {
+			value = value[1:]
+		}
+		if strings.HasSuffix(value, `'`) || strings.HasSuffix(value, `"`) {
+			value = value[:len(value)-1]
+		}
+		for _, index := range offendedFormatPattern.FindAllIndex([]byte(value), -1) {
+			start := token.Pos(int(str.Pos()) + index[0] + 1)
+			end := token.Pos(int(str.Pos()) + index[1])
+			pass.Report(analysis.Diagnostic{
+				Pos:            start,
+				End:            end,
+				Message:        "wrapping %s verb with quotes is not encouraged, please use %q instead",
+				SuggestedFixes: nil,
+			})
+		}
+	}
+}
+
+func runStringInterpolationQuoteAnalyzer(rules []string) func(*analysis.Pass) (interface{}, error) {
+	return func(pass *analysis.Pass) (interface{}, error) {
+		matcher := NewMatcher(pass)
+		for _, file := range pass.Files {
+			ast.Inspect(file, func(n ast.Node) bool {
+				if call, ok := n.(*ast.CallExpr); ok {
+					if matcher.MatchFunction(call, rules) {
+						analyzeInterpolation(call, pass)
+					}
+				}
+				return true
+			})
+		}
+		return nil, nil
+	}
+}
diff --git a/tools/golangci-lint/gitaly/quote_test.go b/tools/golangci-lint/gitaly/quote_test.go
new file mode 100644
index 000000000..9ba749288
--- /dev/null
+++ b/tools/golangci-lint/gitaly/quote_test.go
@@ -0,0 +1,22 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestQuoteInterpolationAnalyzer(t *testing.T) {
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Failed to get wd: %s", err)
+	}
+
+	testdata := filepath.Join(wd, "testdata")
+	analyzer := NewQuoteInterpolationAnalyzer([]string{
+		"fmt.*",
+	})
+	analysistest.Run(t, testdata, analyzer, "quote")
+}
diff --git a/tools/golangci-lint/gitaly/testdata/src/errorwrap/errorwrap_alias_test.go b/tools/golangci-lint/gitaly/testdata/src/errorwrap/errorwrap_alias_test.go
new file mode 100644
index 000000000..d3e0aba27
--- /dev/null
+++ b/tools/golangci-lint/gitaly/testdata/src/errorwrap/errorwrap_alias_test.go
@@ -0,0 +1,32 @@
+package errorwrap
+
+import (
+	f "fmt"
+)
+
+// This file is the test fixture for Gitaly linters
+
+func errorWrapAliasOkay() {
+	err := f.Errorf("test error")
+
+	_ = f.Errorf("error: %s", "something else")
+	_ = f.Errorf("error: %v", "something else")
+	_ = f.Errorf("error: %q", "something else")
+	_ = f.Errorf("error: %s %d", "something else", 5)
+	_ = f.Errorf("error: %w", err)
+	_ = f.Errorf("error: %w", f.Errorf("error: %s", "hello"))
+}
+
+func errorWrapAliasNotOkay() {
+	err := f.Errorf("test error")
+
+	_ = f.Errorf("error: %s", err)                    // want "please use %w to wrap errors"
+	_ = f.Errorf("error: %s", f.Errorf("test error")) // want "please use %w to wrap errors"
+	_ = f.Errorf("error: %v", err)                    // want "please use %w to wrap errors"
+	_ = f.Errorf("error: %v", f.Errorf("test error")) // want "please use %w to wrap errors"
+	_ = f.Errorf("error: %q", err)                    // want "please use %w to wrap errors"
+	_ = f.Errorf("error: %q", f.Errorf("test error")) // want "please use %w to wrap errors"
+	_ = f.Errorf("error number %d: %s", 5, err)       // want "please use %w to wrap errors"
+	_ = f.Errorf("error: %w", err)
+	_ = f.Errorf("error: %w", f.Errorf("error: %s", err)) // want "please use %w to wrap errors"
+}
diff --git a/tools/golangci-lint/gitaly/testdata/src/errorwrap/errorwrap_test.go b/tools/golangci-lint/gitaly/testdata/src/errorwrap/errorwrap_test.go
new file mode 100644
index 000000000..5f6764e63
--- /dev/null
+++ b/tools/golangci-lint/gitaly/testdata/src/errorwrap/errorwrap_test.go
@@ -0,0 +1,37 @@
+package errorwrap
+
+import (
+	"fmt"
+)
+
+// This file is the test fixture for Gitaly linters
+
+func call(format string, err error) {}
+
+func errorWrapOkay() {
+	err := fmt.Errorf("test error")
+
+	_ = fmt.Errorf("error: %s", "something else")
+	_ = fmt.Errorf("error: %v", "something else")
+	_ = fmt.Errorf("error: %q", "something else")
+	_ = fmt.Errorf("error: %s %d", "something else", 5)
+	_ = fmt.Errorf("error: %w", err)
+	_ = fmt.Errorf("error: %w", fmt.Errorf("error: %s", "hello"))
+
+	call("error: %w", fmt.Errorf("error: %s", "hello"))
+	_ = fmt.Sprintf("error: %s", err)
+}
+
+func errorWrapNotOkay() {
+	err := fmt.Errorf("test error")
+
+	_ = fmt.Errorf("error: %s", err)                      // want "please use %w to wrap errors"
+	_ = fmt.Errorf("error: %s", fmt.Errorf("test error")) // want "please use %w to wrap errors"
+	_ = fmt.Errorf("error: %v", err)                      // want "please use %w to wrap errors"
+	_ = fmt.Errorf("error: %v", fmt.Errorf("test error")) // want "please use %w to wrap errors"
+	_ = fmt.Errorf("error: %q", err)                      // want "please use %w to wrap errors"
+	_ = fmt.Errorf("error: %q", fmt.Errorf("test error")) // want "please use %w to wrap errors"
+	_ = fmt.Errorf("error number %d: %s", 5, err)         // want "please use %w to wrap errors"
+	_ = fmt.Errorf("error: %w", err)
+	_ = fmt.Errorf("error: %w", fmt.Errorf("error: %s", err)) // want "please use %w to wrap errors"
+}
diff --git a/tools/golangci-lint/gitaly/testdata/src/quote/quote_test.go b/tools/golangci-lint/gitaly/testdata/src/quote/quote_test.go
new file mode 100644
index 000000000..d0e832c59
--- /dev/null
+++ b/tools/golangci-lint/gitaly/testdata/src/quote/quote_test.go
@@ -0,0 +1,27 @@
+package quote
+
+import "fmt"
+
+// This file is the test fixture for Gitaly linters
+
+func quoteOkay() {
+	fmt.Printf("hello world: %q", "today is a good day")
+	fmt.Printf("hello world: %d", 123)
+	fmt.Printf("%s something", "hello")
+	fmt.Printf("hello world: %s", "this is good")
+	fmt.Printf("hello world: %s something", "this is good")
+}
+
+func quoteNotOkay() {
+	fmt.Printf("hello world: '%s'", "today is a good day") //  want "wrapping %s verb with quotes is not encouraged, please use %q instead"
+	fmt.Printf(`hello world: "%s"`, "today is a good day") // want "wrapping %s verb with quotes is not encouraged, please use %q instead"
+	fmt.Printf(`hello world: "%s"`, "today is a good day") // want "wrapping %s verb with quotes is not encouraged, please use %q instead"
+	str := `so is
+tomorrow`
+	fmt.Printf("hello world: '%s'", str) // want "wrapping %s verb with quotes is not encouraged, please use %q instead"
+	fmt.Printf(`hello world: "%s"`, str) // want "wrapping %s verb with quotes is not encouraged, please use %q instead"
+	fmt.Printf(`hello world: "%s"`, str) // want "wrapping %s verb with quotes is not encouraged, please use %q instead"
+
+	fmt.Printf("hello world:%s %s '%s'", "today", "is a", "good day") // want "wrapping %s verb with quotes is not encouraged, please use %q instead"
+	fmt.Printf("hello world: %d '%s'", 123, "today is a good day")    // want "wrapping %s verb with quotes is not encouraged, please use %q instead"
+}