forked from TrueCloudLab/linters
[#4] linters: Add check for source of constants
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
This commit is contained in:
parent
a1a45f5368
commit
420dd98c24
9 changed files with 276 additions and 14 deletions
|
@ -12,6 +12,7 @@
|
|||
|
||||
## Linters Configuration
|
||||
|
||||
The settings for linters are available if golangci-lint >= 1.5.4 is used.
|
||||
### noliteral
|
||||
|
||||
```yml
|
||||
|
@ -22,6 +23,8 @@ linters-settings:
|
|||
original-url: git.frostfs.info/TrueCloudLab/linters.git
|
||||
settings:
|
||||
target-methods : ["reportFlushError", "reportError"] #optional. Enabled by default "Debug", "Info", "Warn", "Error"
|
||||
constants-package: "git.frostfs.info/rep/logs" #if not set, then the check is disabled
|
||||
|
||||
```
|
||||
|
||||
## Installation
|
||||
|
@ -44,6 +47,7 @@ Add to .golangci.yml
|
|||
path: <Path to the directory with libraries>
|
||||
original-url: git.frostfs.info/TrueCloudLab/linters
|
||||
|
||||
|
||||
linters:
|
||||
enable:
|
||||
custom-linters
|
||||
|
|
|
@ -3,6 +3,8 @@ package noliteral
|
|||
import (
|
||||
"go/ast"
|
||||
"go/token"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
@ -15,8 +17,13 @@ var LogsAnalyzer = &analysis.Analyzer{
|
|||
Run: run,
|
||||
}
|
||||
|
||||
var (
|
||||
aliasCache = sync.Map{}
|
||||
)
|
||||
|
||||
type Configuration struct {
|
||||
TargetMethods []string `mapstructure:"target-methods"`
|
||||
ConstantsPackage string `mapstructure:"constants-package"`
|
||||
}
|
||||
|
||||
var Config = Configuration{
|
||||
|
@ -32,11 +39,23 @@ 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
|
||||
}
|
||||
|
||||
pass.Report(analysis.Diagnostic{
|
||||
Pos: expr.Pos(),
|
||||
End: expr.End(),
|
||||
Category: LinterName,
|
||||
Message: "Wrong package for constants",
|
||||
SuggestedFixes: nil,
|
||||
})
|
||||
return true
|
||||
}
|
||||
|
||||
|
@ -82,3 +101,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 ""
|
||||
}
|
||||
|
|
|
@ -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)
|
||||
}
|
||||
|
|
|
@ -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 (
|
||||
"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
|
||||
}
|
1
main.go
1
main.go
|
@ -26,6 +26,7 @@ func New(conf any) ([]*analysis.Analyzer, error) {
|
|||
}
|
||||
|
||||
noliteral.Config.TargetMethods = append(noliteral.Config.TargetMethods, config.TargetMethods...)
|
||||
noliteral.Config.ConstantsPackage = config.ConstantsPackage
|
||||
}
|
||||
|
||||
return []*analysis.Analyzer{noliteral.LogsAnalyzer}, nil
|
||||
|
|
Loading…
Reference in a new issue