[#1] linters: add logs linter #2

Merged
dstepanov-yadro merged 1 commit from achuprov/linters:master into master 2023-07-26 21:08:06 +00:00
14 changed files with 146 additions and 556 deletions
Showing only changes of commit 53d601603d - Show all commits

View file

@ -2,7 +2,7 @@ OUT_DIR?=./.bin
PLUGIN_SOURCE ?= main.go PLUGIN_SOURCE ?= main.go
test: test:
@go test -v ./... @go test -v ./... -count=1
lib: lib:
@mkdir -pv $(OUT_DIR) @mkdir -pv $(OUT_DIR)
@ -11,3 +11,8 @@ lib:
lint: lint:
@golangci-lint run @golangci-lint run
dstepanov-yadro marked this conversation as resolved
Review

Please add .golangci.yml file with linter config like here: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/.golangci.yml

Please add `.golangci.yml` file with linter config like here: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/.golangci.yml
Review

The empty repository template already has a set of minimally necessary linters, therefore .golangci.yml is not present in the PR

The empty repository template already has a set of minimally necessary linters, therefore .golangci.yml is not present in the PR
staticcheck-install:
@go install honnef.co/go/tools/cmd/staticcheck@latest
staticcheck-run:
@staticcheck ./...

View file

@ -9,7 +9,7 @@
## Available linters ## Available linters
| Name | Description | | Name | Description |
| ----------- | ----------- | | --------- | --------------------------------------------------------------------------- |
| noliteral | The tool prohibits the use of literal string arguments in logging functions | | noliteral | The tool prohibits the use of literal string arguments in logging functions |
@ -25,7 +25,7 @@
## Usage ## Usage
Add to .golagci.yml Add to .golangci.yml
```yml ```yml

View file

@ -1,264 +0,0 @@
package noliteral
import (
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"runtime"
"testing"
"golang.org/x/tools/go/analysis"
)
func TestAnalyzerA_n(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/a_negative._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) {
t.Log(diag.Message)
Flag = true
},
}
_, err = run(pass)
if !Flag {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}
func TestAnalyzerA_p(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/a_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) {
t.Log(diag.Message)
flag = true
},
}
_, err = run(pass)
if flag {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}
func TestAnalyzerB_n(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/b_negative._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) {
t.Log(diag.Message)
Flag = true
},
}
_, err = run(pass)
if !Flag {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}
func TestAnalyzerB_p(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/b_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) {
t.Log(diag.Message)
flag = true
},
}
_, err = run(pass)
if flag {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}
func TestAnalyzerC_n(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/c_negative._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) {
t.Log(diag.Message)
Flag = true
},
}
_, err = run(pass)
if !Flag {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}
func TestAnalyzerC_p(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/c_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) {
t.Log(diag.Message)
flag = true
},
}
_, err = run(pass)
if flag {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}
func TestAnalyzerD_n(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/d_negative._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) {
t.Log(diag.Message)
Flag = true
},
}
_, err = run(pass)
if Flag {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}
func TestAnalyzerD_p(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/d_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) {
t.Log(diag.Message)
flag = true
},
}
_, err = run(pass)
if flag {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}

View file

@ -1,36 +0,0 @@
package src_test
func (c *cfg) f1_n() {
c.log.Info("logs.MSG") //unacceptable
}
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

@ -1,46 +0,0 @@
package src_test
import (
"fmt"
)
func (c *cfg) f1_ok() {
c.log.Info(logs.MSG) //acceptable
}
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

@ -1,36 +0,0 @@
package src_test
func (c *cfg) f1_n() {
c.log.Error("logs.MSG") //unacceptable
}
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

@ -1,46 +0,0 @@
package src_test
import (
"fmt"
)
func (c *cfg) f1_ok() {
c.log.Error(logs.MSG) //acceptable
}
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

@ -1,36 +0,0 @@
package src_test
func (c *cfg) f1_n() {
c.log.Abyr("logs.MSG") //unacceptable
}
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

@ -1,46 +0,0 @@
package src_test
import (
"fmt"
)
func (c *cfg) f1_ok() {
c.log.Abyr(logs.MSG) //acceptable
}
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

@ -3,8 +3,6 @@ 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"
) )
@ -17,11 +15,7 @@ var LogsAnalyzer = &analysis.Analyzer{
Run: run, Run: run,
} }
var ( var methodsToSearch = []string{"Debug", "Info", "Warn", "Error", "reportFlushError", "reportError"}
methodsToSearchOnce = &sync.Once{}
methodsToSearch = []string{"Debug", "Info", "Warn", "Error"}
customLogs = "reportFlushError, reportError"
)
func run(pass *analysis.Pass) (interface{}, error) { func run(pass *analysis.Pass) (interface{}, error) {
for _, file := range pass.Files { for _, file := range pass.Files {
@ -32,17 +26,23 @@ func run(pass *analysis.Pass) (interface{}, error) {
} }
isLog, _ := isLogDot(expr.Fun) isLog, _ := isLogDot(expr.Fun)
if isLog && len(expr.Args) > 0 && isStringValue(expr.Args[0]) { if !isLog {
return true
}
if len(expr.Args) == 0 || !isStringValue(expr.Args[0]) {
return true
}
pass.Report(analysis.Diagnostic{ pass.Report(analysis.Diagnostic{
Pos: expr.Pos(), Pos: expr.Pos(),
End: 0, End: expr.End(),
Category: LinterName, Category: LinterName,
Message: "Literals are not allowed in the body of the logger", Message: "Literals are not allowed in the body of the logger",
SuggestedFixes: nil, SuggestedFixes: nil,
}) })
return false return false
}
return true
}) })
} }
@ -54,14 +54,6 @@ func isLogDot(expr ast.Expr) (bool, string) {
if !ok { if !ok {
return false, "" return false, ""
} }
methodsToSearchOnce.Do(func() {
for _, cl := range strings.Split(customLogs, ",") {
cl = strings.Trim(cl, " ")
if len(cl) > 0 {
methodsToSearch = append(methodsToSearch, cl)
}
}
})
for _, method := range methodsToSearch { for _, method := range methodsToSearch {
if isIdent(sel.Sel, method) { if isIdent(sel.Sel, method) {
@ -73,12 +65,10 @@ func isLogDot(expr ast.Expr) (bool, string) {
func isIdent(expr ast.Expr, ident string) bool { func isIdent(expr ast.Expr, ident string) bool {
id, ok := expr.(*ast.Ident) id, ok := expr.(*ast.Ident)
if !ok { if ok && id.Name == ident {
return false
}
if id.Name == ident {
return true return true
} }
return false return false
} }

View file

@ -0,0 +1,70 @@
package noliteral
import (
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"runtime"
"testing"
"golang.org/x/tools/go/analysis"
)
func TestAnalyzer_negative(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/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 != 6 {
t.Fail()
}
if err != nil {
t.Fatal(err)
}
}
func TestAnalyzer_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)
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)
}
}

View file

@ -1,9 +1,32 @@
package src_test package src_test
func (c *cfg) f1_n() {
func (c *cfg) info_n() {
c.log.Info("logs.MSG") //unacceptable
}
func (c *cfg) info_n() {
log.Info("logs.MSG") //unacceptable
}
func (c *cfg) debug_n() {
c.log.Debug("logs.MSG") //unacceptable c.log.Debug("logs.MSG") //unacceptable
} }
func (c *cfg) error_n() {
c.log.Error("logs.MSG") //unacceptable
}
func (c *cfg) reportFlushError_n() {
c.log.reportFlushError("logs.MSG") //unacceptable
}
func (c *cfg) reportError_n() {
c.log.reportError("logs.MSG") //unacceptable
}
type Logger interface { type Logger interface {
Debug(msg string) Debug(msg string)
Info(msg string) Info(msg string)

View file

@ -4,10 +4,30 @@ import (
"fmt" "fmt"
) )
func (c *cfg) f1_ok() { func (c *cfg) info_ok() {
c.log.Info(logs.MSG) //acceptable
}
func (c *cfg) debug_ok() {
c.log.Debug(logs.MSG) //acceptable c.log.Debug(logs.MSG) //acceptable
} }
func (c *cfg) error_ok() {
c.log.Error(logs.MSG) //acceptable
}
func (c *cfg) custom_ok_const() {
c.log.Abyr(logs.MSG) //acceptable
}
func (c *cfg) custom_ok_lit() {
c.log.Abyr("logs.MSG") //acceptable
}
func (c *cfg) custom_ok_lit() {
log.Abyr("logs.MSG") //acceptable
}
type Logger interface { type Logger interface {
Debug(msg string) Debug(msg string)
Info(msg string) Info(msg string)

10
main.go
View file

@ -1,7 +1,7 @@
package main package main
import ( import (
noliteral "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/no-literal" noliteral "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral"
"golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis"
) )
@ -9,14 +9,6 @@ var AnalyzerPlugin analyzerPlugin
type analyzerPlugin struct{} type analyzerPlugin struct{}
// for version ci-lint < '1.5.4'.
func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer { func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
return []*analysis.Analyzer{noliteral.LogsAnalyzer} return []*analysis.Analyzer{noliteral.LogsAnalyzer}
} }
/*
// for version ci-lint >= '1.5.4'.
func New(conf any) ([]*analysis.Analyzer, error) {
return []*analysis.Analyzer{noliteral.LogsAnalyzer}, nil
}
*/