forked from TrueCloudLab/linters
Compare commits
3 commits
master
...
const-chec
Author | SHA1 | Date | |
---|---|---|---|
1c1b1596a8 | |||
7450953e5f | |||
420dd98c24 |
10 changed files with 350 additions and 46 deletions
|
@ -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:
|
||||||
|
|
77
README.md
77
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.
|
`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).
|
||||||
|
|
||||||
|
|
|
@ -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 ""
|
||||||
|
}
|
||||||
|
|
|
@ -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)
|
||||||
}
|
}
|
||||||
|
|
|
@ -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
|
||||||
|
}
|
|
@ -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
|
||||||
|
}
|
|
@ -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
33
main.go
|
@ -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
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue