lint: Move custom linter settings to .golangci.yml file

In a prior change
(https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5398), we added
the support for custom linters on top of golangci-lint. In the first
iteration, all settings for them are put in the implemenation file.
This commit moves those settings to the manifeset file, along side with
other linter settings.

golangci-lint doesn't support an official way to load the settings in
de-facto manifest file `.golangci.yml`. Fortunately, we can hook into
configuration parsing library `viper` that golangci-lint uses. Our
custom linters are loaded at the end, just before executing the
commands. At this time, golangci-lint established some state that stores
the configuration. This state is process global, and accessible in our
source code. In the future, if this approach doesn't work anymore, we
can consider parsing the configuration file ourselves.

5 files changed, 80 insertions, 18 deletions

diff --git a/tools/golangci-lint/gitaly/errorwrap.go b/tools/golangci-lint/gitaly/errorwrap.go
index af3530a60..e28db833f 100644
--- a/tools/golangci-lint/gitaly/errorwrap.go
+++ b/tools/golangci-lint/gitaly/errorwrap.go
@@ -9,7 +9,13 @@ import (
 	"golang.org/x/tools/go/analysis"
 )
 
-// NewErrorWrapAnalyzer returns an analyzer to detect unexpected error interpolation without %w.
+const errorWrapAnalyzerName = "error_wrap"
+
+type errorWrapAnalyzerSettings struct {
+	IncludedFunctions []string `mapstructure:"included-functions"`
+}
+
+// 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.
 //
@@ -30,12 +36,12 @@ import (
 //
 // For more information:
 // https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md#use-w-when-wrapping-errors
-func NewErrorWrapAnalyzer(rules []string) *analysis.Analyzer {
+func newErrorWrapAnalyzer(settings *errorWrapAnalyzerSettings) *analysis.Analyzer {
 	return &analysis.Analyzer{
-		Name: "error_wrap",
+		Name: errorWrapAnalyzerName,
 		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),
+		Run: runErrorWrapAnalyzer(settings.IncludedFunctions),
 	}
 }
 
diff --git a/tools/golangci-lint/gitaly/errorwrap_test.go b/tools/golangci-lint/gitaly/errorwrap_test.go
index 529112ded..dbdc6cf80 100644
--- a/tools/golangci-lint/gitaly/errorwrap_test.go
+++ b/tools/golangci-lint/gitaly/errorwrap_test.go
@@ -15,8 +15,8 @@ func TestNewErrorWrapAnalyzer(t *testing.T) {
 	}
 
 	testdata := filepath.Join(wd, "testdata")
-	analyzer := NewErrorWrapAnalyzer([]string{
+	analyzer := newErrorWrapAnalyzer(&errorWrapAnalyzerSettings{IncludedFunctions: []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
index f03e118eb..23f99e6aa 100644
--- a/tools/golangci-lint/gitaly/lint.go
+++ b/tools/golangci-lint/gitaly/lint.go
@@ -1,23 +1,73 @@
 package main
 
 import (
+	"strings"
+
+	"github.com/spf13/viper"
 	"golang.org/x/tools/go/analysis"
 )
 
 type analyzerPlugin struct{}
 
-func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
+func (p *analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
 	return []*analysis.Analyzer{
-		NewQuoteInterpolationAnalyzer([]string{
-			"fmt.*",
+		newQuoteInterpolationAnalyzer(&quoteInterpolationAnalyzerSettings{
+			IncludedFunctions: p.configStringSlicesAt(
+				quoteInterpolationAnalyzerName,
+				"included-functions",
+			),
 		}),
-		NewErrorWrapAnalyzer([]string{
-			"fmt.Errorf",
-			"gitlab.com/gitlab-org/gitaly/v15/internal/structerr.*",
+		newErrorWrapAnalyzer(&errorWrapAnalyzerSettings{
+			IncludedFunctions: p.configStringSlicesAt(
+				quoteInterpolationAnalyzerName,
+				"included-functions",
+			),
 		}),
 	}
 }
 
+// This method fetches a string slices in golangci-lint config files for the input analyzer. This is
+// an enhancement to golangci-lint. Although it supports custom linters, it doesn't support parsing
+// custom settings for such linters. We have to take care of parsing ourselves. Fortunately,
+// golangci-lint uses `viper` package underlying. This package maintains a global process state.
+// This state stores the parsed configurations of all custom linters. As custom linter is loaded
+// after all other public linters, it's guaranteed that viper state is already established.
+//
+// It's true this behavior may change in the future, but it's still better than reading and parsing
+// the config file ourselves. We may consider that approach if this way doesn't work
+//
+// # The structure for custom linter's settings is described as followed:
+//
+// ```yaml
+//
+//	   linters:
+//		custom:
+//		  gitaly-linters:
+//		    path: ./_build/tools/gitaly-linters.so
+//		    description: A collection of linters tailored for Gitaly
+//		    original-url: gitlab.com/gitlab-org/gitaly
+//		    settings:
+//		      string_interpolation_quote:
+//		        included-functions:
+//		          - fmt.*
+//		      error_wrap:
+//		        included-functions:
+//		          - fmt.Errorf
+//		          - gitlab.com/gitlab-org/gitaly/v15/internal/structerr.*
+//
+// ```
+func (*analyzerPlugin) configStringSlicesAt(analyzer string, key string) []string {
+	path := strings.Join([]string{
+		"linters-settings",
+		"custom",
+		"gitaly-linters",
+		"settings",
+		analyzer,
+		key,
+	}, ".")
+	return viper.GetStringSlice(path)
+}
+
 // AnalyzerPlugin is a convention of golangci-lint to implement a custom linter. This variable
 // must implement `AnalyzerPlugin` interface:
 //
diff --git a/tools/golangci-lint/gitaly/quote.go b/tools/golangci-lint/gitaly/quote.go
index d9648b0b5..cfc2dac97 100644
--- a/tools/golangci-lint/gitaly/quote.go
+++ b/tools/golangci-lint/gitaly/quote.go
@@ -9,7 +9,13 @@ import (
 	"golang.org/x/tools/go/analysis"
 )
 
-// NewQuoteInterpolationAnalyzer returns an analyzer to detect manually quoted string interpolation
+const quoteInterpolationAnalyzerName = "string_interpolation_quote"
+
+type quoteInterpolationAnalyzerSettings struct {
+	IncludedFunctions []string `mapstructure:"included-functions"`
+}
+
+// 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.
 //
@@ -27,13 +33,13 @@ import (
 //
 // For more information:
 // https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md#use-q-when-interpolating-strings
-func NewQuoteInterpolationAnalyzer(rules []string) *analysis.Analyzer {
+func newQuoteInterpolationAnalyzer(settings *quoteInterpolationAnalyzerSettings) *analysis.Analyzer {
 	return &analysis.Analyzer{
-		Name: "string_interpolation_quote",
+		Name: quoteInterpolationAnalyzerName,
 		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),
+		Run: runStringInterpolationQuoteAnalyzer(settings.IncludedFunctions),
 	}
 }
 
diff --git a/tools/golangci-lint/gitaly/quote_test.go b/tools/golangci-lint/gitaly/quote_test.go
index 9ba749288..7d262b5de 100644
--- a/tools/golangci-lint/gitaly/quote_test.go
+++ b/tools/golangci-lint/gitaly/quote_test.go
@@ -15,8 +15,8 @@ func TestQuoteInterpolationAnalyzer(t *testing.T) {
 	}
 
 	testdata := filepath.Join(wd, "testdata")
-	analyzer := NewQuoteInterpolationAnalyzer([]string{
+	analyzer := newQuoteInterpolationAnalyzer(&quoteInterpolationAnalyzerSettings{IncludedFunctions: []string{
 		"fmt.*",
-	})
+	}})
 	analysistest.Run(t, testdata, analyzer, "quote")
 }