[#4] linters: add disable-packages option

Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>

.forgejo/workflows/tests.yml

@@ -13,7 +13,7 @@ jobs:
         with:
           go-version: '1.20'
           cache: true
-      - name: Run staticcheck
+      - name: Build lib
         run: make lib
 
   lint:

README.md

@@ -4,34 +4,6 @@
 
 `linters` is a project that enables the integration of custom linting rules into the [golangci-lint](https://github.com/golangci/golangci-lint) framework. 
 
-## Available linters
-
-| Name                    | Description                                                                 |
-| ----------------------- | --------------------------------------------------------------------------- |
-| [noliteral](#noliteral) | The tool prohibits the use of literal string arguments in logging functions |
-
-## Linters Configuration
-
-### noliteral 
-
-```yml
-linters-settings:
-    custom:
-    noliteral:
-      path: .bin/external_linters.so
-      original-url: git.frostfs.info/TrueCloudLab/linters.git 
-      settings:
-        target-methods : ["reportFlushError", "reportError"] #optional. Enabled by default "Debug", "Info", "Warn", "Error"
-```
-
-## Installation
-
-```bash
-    git clone git.frostfs.info/TrueCloudLab/linters
-    cd linters
-    make lib OUT_DIR=<Path to the directory with libraries>
-```
-
 ## Usage
 
 Add to .golangci.yml 
@@ -44,7 +16,56 @@ Add to .golangci.yml
             path: <Path to the directory with libraries>
             original-url: git.frostfs.info/TrueCloudLab/linters
 
+
     linters:
         enable:
             custom-linters
 ```
+
+
+## Installation
+
+```bash
+    git clone git.frostfs.info/TrueCloudLab/linters
+    cd linters
+    make lib OUT_DIR=<Path to the directory with libraries>
+```
+
+## Available linters
+
+| Name                    | Description                                                                 |
+| ----------------------- | --------------------------------------------------------------------------- |
+| [noliteral](#noliteral) | The tool prohibits the use of literal string arguments in logging functions |
+
+## Linters Configuration
+
+Settings via a configuration file are available if golangci-lint >= 1.5.4 is used
+
+### noliteral 
+
+##### File Configuration
+
+```yml
+linters-settings:
+    custom:
+        noliteral:
+            path: .bin/external_linters.so
+            original-url: git.frostfs.info/TrueCloudLab/linters.git 
+            settings:
+                target-methods: ["reportFlushError", "reportError"] # optional. Enabled by default "Debug", "Info", "Warn", "Error"
+                disable-packages: ["pkg1", "pkg2"] # List of packages for which the check should be disabled.
+                constants-package: "git.frostfs.info/rep/logs" # if not set, then the check is disabled
+
+```
+##### ENV Configuration
+
+
+| Variable                      | Description                                             |
+| ----------------------------- | ------------------------------------------------------- |
+| `NOLITERAL_TARGET_METHODS`    | List of methods to analyze                              |
+| `NOLITERAL_DISABLE_PACKAGES`  | List of packages for which the check should be disabled |
+| `NOLITERAL_CONSTANTS_PACKAGE` | Path to the package with constants                      |
+
+
+**Note:** You may need to clear the golangci-lint cache when configuring through ENV. More details can be found [here](https://golangci-lint.run/usage/configuration/#cache).
+

internal/analyzers/noliteral/linter.go

@@ -3,6 +3,8 @@ package noliteral
 import (
 	"go/ast"
 	"go/token"
+	"strings"
+	"sync"
 
 	"golang.org/x/tools/go/analysis"
 )
@@ -15,8 +17,14 @@ var LogsAnalyzer = &analysis.Analyzer{
 	Run:  run,
 }
 
+var (
+	aliasCache = sync.Map{}
+)
+
 type Configuration struct {
-	TargetMethods []string `mapstructure:"target-methods"`
+	TargetMethods    []string `mapstructure:"target-methods"`
+	DisablePackages  []string `mapstructure:"disable-packages"`
+	ConstantsPackage string   `mapstructure:"constants-package"`
 }
 
 var Config = Configuration{
@@ -32,11 +40,29 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			}
 
 			isLog, _ := isLogDot(expr.Fun)
-			if !isLog {
+			if !isLog || len(expr.Args) == 0 {
 				return true
 			}
 
-			if len(expr.Args) == 0 || !isStringValue(expr.Args[0]) {
+			if !isStringValue(expr.Args[0]) {
+				alias, _ := getAliasByPkgName(file, Config.ConstantsPackage)
+				if Config.ConstantsPackage == "" || getPackageName(expr.Args[0]) == alias || getPackageName(expr.Args[0]) == "" {
+					return true
+				}
+
+				for _, pkgName := range Config.DisablePackages {
+					if pkgName == getPackageName(expr.Args[0]) {
+						return true
+					}
+				}
+
+				pass.Report(analysis.Diagnostic{
+					Pos:            expr.Pos(),
+					End:            expr.End(),
+					Category:       LinterName,
+					Message:        "Wrong package for constants",
+					SuggestedFixes: nil,
+				})
 				return true
 			}
 
@@ -82,3 +108,53 @@ func isStringValue(expr ast.Expr) bool {
 	basicLit, ok := expr.(*ast.BasicLit)
 	return ok && basicLit.Kind == token.STRING
 }
+
+func getAliasByPkgName(file *ast.File, pkgName string) (string, error) {
+	if alias, ok := aliasCache.Load(file); ok {
+		return alias.(string), nil
+	}
+
+	var alias string
+	specs := file.Imports
+
+	for _, spec := range specs {
+		alias = getAliasFromImportSpec(spec, pkgName)
+		if alias != "" {
+			break
+		}
+	}
+
+	aliasCache.Store(file, alias)
+	return alias, nil
+}
+
+func getAliasFromImportSpec(spec *ast.ImportSpec, pkgName string) string {
+	if spec == nil {
+		return ""
+	}
+	importName := strings.Replace(spec.Path.Value, "\"", "", -1)
+	if importName != pkgName {
+		return ""
+	}
+
+	split := strings.Split(importName, "/")
+
+	if len(split) == 0 {
+		return ""
+	}
+	alias := split[len(split)-1]
+	if spec.Name != nil {
+		alias = spec.Name.Name
+	}
+
+	return alias
+}
+
+func getPackageName(expr ast.Expr) string {
+	if selectorExpr, ok := expr.(*ast.SelectorExpr); ok {
+		if ident, ok := selectorExpr.X.(*ast.Ident); ok {
+			return ident.Name
+		}
+	}
+	return ""
+}

internal/analyzers/noliteral/linter_test.go

@@ -11,14 +11,17 @@ import (
 	"golang.org/x/tools/go/analysis"
 )
 
+func init() {
+	Config.ConstantsPackage = "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"
+}
+
 func TestAnalyzer_negative(t *testing.T) {
 	const countNegativeCases = 4
 
 	_, filename, _, _ := runtime.Caller(0)
 	dir := filepath.Dir(filename)
-
 	fset := token.NewFileSet()
-	f, err := parser.ParseFile(fset, filepath.Join(dir, "test-case/src/negative._go"), nil, parser.AllErrors)
+	f, err := parser.ParseFile(fset, filepath.Join(dir, "test-case/literals/negative._go"), nil, parser.AllErrors)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -42,12 +45,71 @@ func TestAnalyzer_negative(t *testing.T) {
 	}
 }
 
-func TestAnalyzer_positive(t *testing.T) {
+func TestAnalyzer_literals_positive(t *testing.T) {
 	_, filename, _, _ := runtime.Caller(0)
 	dir := filepath.Dir(filename)
 
 	fset := token.NewFileSet()
-	f, err := parser.ParseFile(fset, filepath.Join(dir, "test-case/src/positive._go"), nil, parser.AllErrors)
+	f, err := parser.ParseFile(fset, filepath.Join(dir, "test-case/literals/positive._go"), nil, parser.AllErrors)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	flag := false
+	pass := &analysis.Pass{
+		Fset:  fset,
+		Files: []*ast.File{f},
+		Report: func(diag analysis.Diagnostic) {
+			flag = true
+		},
+	}
+	_, err = run(pass)
+
+	if flag {
+		t.Fail()
+	}
+
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestAnalyzer_const_location_negative(t *testing.T) {
+	const countNegativeCases = 3
+	_, filename, _, _ := runtime.Caller(0)
+	dir := filepath.Dir(filename)
+
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, filepath.Join(dir, "test-case/const-location/negative._go"), nil, parser.AllErrors)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	Count := 0
+	pass := &analysis.Pass{
+		Fset:  fset,
+		Files: []*ast.File{f},
+		Report: func(diag analysis.Diagnostic) {
+			Count++
+		},
+	}
+
+	_, err = run(pass)
+
+	if Count != countNegativeCases {
+		t.Fail()
+	}
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestAnalyzer_const_location_positive(t *testing.T) {
+	_, filename, _, _ := runtime.Caller(0)
+	dir := filepath.Dir(filename)
+
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, filepath.Join(dir, "test-case/const-location/positive._go"), nil, parser.AllErrors)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/analyzers/noliteral/test-case/const-location/negative._go

@@ -0,0 +1,61 @@
+package src_test
+
+import (
+	"fmt"
+	tochno_ne_const_dly_log "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"// The alias of the package with constants differs from the one used
+)
+
+/*After upgrading to golangci-lint version 1.5.4 and adding linter configuration, it will be possible to get rid of 'magic repositories'*/
+
+func (c *cfg) info_n() {
+	c.log.Info(logs.MSG)
+}
+
+func (c *cfg) debug_n() {
+	c.log.Debug(logs.MSG)
+}
+
+func (c *cfg) error_n() {
+	c.log.Error(logs.MSG)
+}
+
+func (c *cfg) reportFlushError_n() {
+	c.log.reportFlushError(logs.MSG)
+}
+
+func (c *cfg) reportError_n() {
+	c.log.reportError(logs.MSG) 
+}
+
+
+
+type Logger interface {
+	Debug(msg string)
+	Info(msg string)
+	Warn(msg string)
+	Error(msg string)
+	Abyr(msg string)
+}
+
+type RealLogger struct{}
+
+func (l RealLogger) Debug(msg string) {
+}
+func (l RealLogger) Info(msg string) {
+}
+func (l RealLogger) Warn(msg string) {
+}
+func (l RealLogger) Error(msg string) {
+}
+func (l RealLogger) Abyr(msg string) {
+}
+
+var logs = struct {
+	MSG string
+}{
+	MSG: "some message",
+}
+
+type cfg struct {
+	log Logger
+}

internal/analyzers/noliteral/test-case/const-location/positive._go

@@ -0,0 +1,62 @@
+package logs //declaration package
+
+/*After upgrading to golangci-lint version 1.5.4 and adding linter configuration, it will be possible to get rid of 'magic repositories'*/
+
+import (
+	"fmt"
+	tochno_ne_const_dly_log "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"// The alias of the package with constants differs from the one used
+)
+
+func (c *cfg) info_ok() {
+	c.log.Info(tochno_ne_const_dly_log.MSG)
+}
+
+func (c *cfg) debug_ok() {
+	c.log.Debug(tochno_ne_const_dly_log.MSG)
+}
+
+func (c *cfg) error_ok() {
+	c.log.Error(tochno_ne_const_dly_log.MSG)
+}
+
+func (c *cfg) custom_ok_const() {
+	c.log.Abyr(tochno_ne_const_dly_log.MSG)
+}
+
+
+type Logger interface {
+	Debug(msg string)
+	Info(msg string)
+	Warn(msg string)
+	Error(msg string)
+	Abyr(msg string)
+}
+
+type RealLogger struct{}
+
+func (l RealLogger) Debug(msg string) {
+}
+func (l RealLogger) Info(msg string) {
+}
+func (l RealLogger) Warn(msg string) {
+}
+func (l RealLogger) Error(msg string) {
+}
+func (l RealLogger) Abyr(msg string) {
+}
+
+type RealLogger struct{}
+
+func (l RealLogger) Info(msg string) {
+	fmt.Println(msg)
+}
+
+var logs = struct {
+	MSG string
+}{
+	MSG: "some message",
+}
+
+type cfg struct {
+	log Logger
+}

internal/analyzers/noliteral/test-case/literals/positive._go

@@ -2,8 +2,11 @@ package src_test
 
 import (
 	"fmt"
+	"git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"
 )
 
+/*After upgrading to golangci-lint version 1.5.4 and adding linter configuration, it will be possible to get rid of 'magic repositories'*/
+
 func (c *cfg) info_ok() {
 	c.log.Info(logs.MSG) //acceptable
 }

main.go

@@ -1,6 +1,9 @@
 package main
 
 import (
+	"os"
+	"strings"
+
 	noliteral "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral"
 	"github.com/mitchellh/mapstructure"
 	"golang.org/x/tools/go/analysis"
@@ -12,21 +15,37 @@ type analyzerPlugin struct{}
 
 // for version ci-lint < '1.5.4'.
 func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
-	return []*analysis.Analyzer{noliteral.LogsAnalyzer}
+	analyzer, _ := New(nil)
+	return analyzer
 }
 
 // for version ci-lint >= '1.5.4'.
 func New(conf any) ([]*analysis.Analyzer, error) {
-	var config noliteral.Configuration
+	targetMethods := strings.Split(os.Getenv("NOLITERAL_TARGET_METHODS"), ",")
+	disablePackages := strings.Split(os.Getenv("NOLITERAL_DISABLE_PACKAGES"), ",")
+	constantsPackage := os.Getenv("NOLITERAL_CONSTANTS_PACKAGE")
+
+	configMap := map[string]any{
+		"target-methods":    targetMethods,
+		"constants-package": constantsPackage,
+		"disable-packages":  disablePackages,
+	}
 
 	if confMap, ok := conf.(map[string]any); ok {
-		err := mapstructure.Decode(confMap, &config)
-		if err != nil {
-			return nil, err
+		for k, v := range confMap {
+			configMap[k] = v
 		}
-
-		noliteral.Config.TargetMethods = append(noliteral.Config.TargetMethods, config.TargetMethods...)
 	}
 
+	var config noliteral.Configuration
+	err := mapstructure.Decode(configMap, &config)
+	if err != nil {
+		return nil, err
+	}
+
+	noliteral.Config.TargetMethods = append(noliteral.Config.TargetMethods, config.TargetMethods...)
+	noliteral.Config.ConstantsPackage = config.ConstantsPackage
+	noliteral.Config.DisablePackages = config.DisablePackages
+
 	return []*analysis.Analyzer{noliteral.LogsAnalyzer}, nil
 }