Compare commits

...

3 commits

Author SHA1 Message Date
1c1b1596a8 [#4] linters: add disable-packages option
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
2023-08-07 17:36:41 +03:00
7450953e5f [#4] linters: Add configuration through ENV
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
2023-08-07 17:24:19 +03:00
420dd98c24 [#4] linters: Add check for source of constants
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
2023-08-04 14:26:40 +03:00
10 changed files with 350 additions and 46 deletions

View file

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

View file

@ -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. `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 ## Usage
Add to .golangci.yml Add to .golangci.yml
@ -44,7 +16,56 @@ Add to .golangci.yml
path: <Path to the directory with libraries> path: <Path to the directory with libraries>
original-url: git.frostfs.info/TrueCloudLab/linters original-url: git.frostfs.info/TrueCloudLab/linters
linters: linters:
enable: enable:
custom-linters 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).

View file

@ -3,6 +3,8 @@ package noliteral
import ( import (
"go/ast" "go/ast"
"go/token" "go/token"
"strings"
"sync"
"golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis"
) )
@ -15,8 +17,14 @@ var LogsAnalyzer = &analysis.Analyzer{
Run: run, Run: run,
} }
var (
aliasCache = sync.Map{}
)
type Configuration struct { 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{ var Config = Configuration{
@ -32,11 +40,29 @@ func run(pass *analysis.Pass) (interface{}, error) {
} }
isLog, _ := isLogDot(expr.Fun) isLog, _ := isLogDot(expr.Fun)
if !isLog { if !isLog || len(expr.Args) == 0 {
return true 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 return true
} }
@ -82,3 +108,53 @@ func isStringValue(expr ast.Expr) bool {
basicLit, ok := expr.(*ast.BasicLit) basicLit, ok := expr.(*ast.BasicLit)
return ok && basicLit.Kind == token.STRING 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 ""
}

View file

@ -11,14 +11,17 @@ import (
"golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis"
) )
func init() {
Config.ConstantsPackage = "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"
}
func TestAnalyzer_negative(t *testing.T) { func TestAnalyzer_negative(t *testing.T) {
const countNegativeCases = 4 const countNegativeCases = 4
_, filename, _, _ := runtime.Caller(0) _, filename, _, _ := runtime.Caller(0)
dir := filepath.Dir(filename) dir := filepath.Dir(filename)
fset := token.NewFileSet() 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 { if err != nil {
t.Fatal(err) 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) _, filename, _, _ := runtime.Caller(0)
dir := filepath.Dir(filename) dir := filepath.Dir(filename)
fset := token.NewFileSet() 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }

View file

@ -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
}

View file

@ -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
}

View file

@ -2,8 +2,11 @@ package src_test
import ( import (
"fmt" "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() { func (c *cfg) info_ok() {
c.log.Info(logs.MSG) //acceptable c.log.Info(logs.MSG) //acceptable
} }

33
main.go
View file

@ -1,6 +1,9 @@
package main package main
import ( import (
"os"
"strings"
noliteral "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral" noliteral "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral"
"github.com/mitchellh/mapstructure" "github.com/mitchellh/mapstructure"
"golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis"
@ -12,21 +15,37 @@ type analyzerPlugin struct{}
// for version ci-lint < '1.5.4'. // for version ci-lint < '1.5.4'.
func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer { func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
return []*analysis.Analyzer{noliteral.LogsAnalyzer} analyzer, _ := New(nil)
return analyzer
} }
// for version ci-lint >= '1.5.4'. // for version ci-lint >= '1.5.4'.
func New(conf any) ([]*analysis.Analyzer, error) { 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 { if confMap, ok := conf.(map[string]any); ok {
err := mapstructure.Decode(confMap, &config) for k, v := range confMap {
if err != nil { configMap[k] = v
return nil, err
} }
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 return []*analysis.Analyzer{noliteral.LogsAnalyzer}, nil
} }