From df29d0882173e9e6142dd99621816eb6c8a0e2a0 Mon Sep 17 00:00:00 2001 From: Alexander Chuprov Date: Mon, 31 Jul 2023 12:59:51 +0300 Subject: [PATCH] [#5] linters: refactoring Signed-off-by: Alexander Chuprov --- Makefile | 11 +- README.md | 6 +- internal/analyzers/no-literal/linter_test.go | 264 ------------------ .../no-literal/test-case/src/a_negative._go | 36 --- .../no-literal/test-case/src/a_positive._go | 46 --- .../no-literal/test-case/src/c_negative._go | 36 --- .../no-literal/test-case/src/c_positive._go | 46 --- .../no-literal/test-case/src/d_negative._go | 36 --- .../no-literal/test-case/src/d_positive._go | 46 --- .../{no-literal => noliteral}/linter.go | 48 ++-- internal/analyzers/noliteral/linter_test.go | 70 +++++ .../test-case/src/negative._go} | 25 +- .../test-case/src/positive._go} | 22 +- main.go | 10 +- 14 files changed, 146 insertions(+), 556 deletions(-) delete mode 100644 internal/analyzers/no-literal/linter_test.go delete mode 100644 internal/analyzers/no-literal/test-case/src/a_negative._go delete mode 100644 internal/analyzers/no-literal/test-case/src/a_positive._go delete mode 100644 internal/analyzers/no-literal/test-case/src/c_negative._go delete mode 100644 internal/analyzers/no-literal/test-case/src/c_positive._go delete mode 100644 internal/analyzers/no-literal/test-case/src/d_negative._go delete mode 100644 internal/analyzers/no-literal/test-case/src/d_positive._go rename internal/analyzers/{no-literal => noliteral}/linter.go (58%) create mode 100644 internal/analyzers/noliteral/linter_test.go rename internal/analyzers/{no-literal/test-case/src/b_negative._go => noliteral/test-case/src/negative._go} (55%) rename internal/analyzers/{no-literal/test-case/src/b_positive._go => noliteral/test-case/src/positive._go} (62%) diff --git a/Makefile b/Makefile index 3ed3dc2..8667f50 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,8 @@ -OUT_DIR?=./.bin -PLUGIN_SOURCE?=main.go +OUT_DIR ?= ./bin +PLUGIN_SOURCE ?= main.go test: - @go test -v ./... + @go test -v ./... -count=1 lib: @mkdir -pv $(OUT_DIR) @@ -11,3 +11,8 @@ lib: lint: @golangci-lint run +staticcheck-install: + @go install honnef.co/go/tools/cmd/staticcheck@latest + +staticcheck-run: + @staticcheck ./... diff --git a/README.md b/README.md index 7421ae7..5dd6f7c 100644 --- a/README.md +++ b/README.md @@ -8,8 +8,8 @@ ## Available linters -| Name | Description | -| ----------- | ----------- | +| Name | Description | +| --------- | --------------------------------------------------------------------------- | | noliteral | The tool prohibits the use of literal string arguments in logging functions | @@ -25,7 +25,7 @@ ## Usage -Add to .golagci.yml +Add to .golangci.yml ```yml diff --git a/internal/analyzers/no-literal/linter_test.go b/internal/analyzers/no-literal/linter_test.go deleted file mode 100644 index 69b885f..0000000 --- a/internal/analyzers/no-literal/linter_test.go +++ /dev/null @@ -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) - } -} diff --git a/internal/analyzers/no-literal/test-case/src/a_negative._go b/internal/analyzers/no-literal/test-case/src/a_negative._go deleted file mode 100644 index 39674bc..0000000 --- a/internal/analyzers/no-literal/test-case/src/a_negative._go +++ /dev/null @@ -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 -} diff --git a/internal/analyzers/no-literal/test-case/src/a_positive._go b/internal/analyzers/no-literal/test-case/src/a_positive._go deleted file mode 100644 index 2920216..0000000 --- a/internal/analyzers/no-literal/test-case/src/a_positive._go +++ /dev/null @@ -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 -} diff --git a/internal/analyzers/no-literal/test-case/src/c_negative._go b/internal/analyzers/no-literal/test-case/src/c_negative._go deleted file mode 100644 index f3cdccb..0000000 --- a/internal/analyzers/no-literal/test-case/src/c_negative._go +++ /dev/null @@ -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 -} diff --git a/internal/analyzers/no-literal/test-case/src/c_positive._go b/internal/analyzers/no-literal/test-case/src/c_positive._go deleted file mode 100644 index 77a8fd7..0000000 --- a/internal/analyzers/no-literal/test-case/src/c_positive._go +++ /dev/null @@ -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 -} diff --git a/internal/analyzers/no-literal/test-case/src/d_negative._go b/internal/analyzers/no-literal/test-case/src/d_negative._go deleted file mode 100644 index 56c5024..0000000 --- a/internal/analyzers/no-literal/test-case/src/d_negative._go +++ /dev/null @@ -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 -} diff --git a/internal/analyzers/no-literal/test-case/src/d_positive._go b/internal/analyzers/no-literal/test-case/src/d_positive._go deleted file mode 100644 index 5ed4b82..0000000 --- a/internal/analyzers/no-literal/test-case/src/d_positive._go +++ /dev/null @@ -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 -} diff --git a/internal/analyzers/no-literal/linter.go b/internal/analyzers/noliteral/linter.go similarity index 58% rename from internal/analyzers/no-literal/linter.go rename to internal/analyzers/noliteral/linter.go index 22573ae..6e762b5 100644 --- a/internal/analyzers/no-literal/linter.go +++ b/internal/analyzers/noliteral/linter.go @@ -3,8 +3,6 @@ package noliteral import ( "go/ast" "go/token" - "strings" - "sync" "golang.org/x/tools/go/analysis" ) @@ -17,11 +15,7 @@ var LogsAnalyzer = &analysis.Analyzer{ Run: run, } -var ( - methodsToSearchOnce = &sync.Once{} - methodsToSearch = []string{"Debug", "Info", "Warn", "Error"} - customLogs = "reportFlushError, reportError" -) +var methodsToSearch = []string{"Debug", "Info", "Warn", "Error", "reportFlushError", "reportError"} func run(pass *analysis.Pass) (interface{}, error) { for _, file := range pass.Files { @@ -32,17 +26,23 @@ func run(pass *analysis.Pass) (interface{}, error) { } isLog, _ := isLogDot(expr.Fun) - if isLog && len(expr.Args) > 0 && isStringValue(expr.Args[0]) { - pass.Report(analysis.Diagnostic{ - Pos: expr.Pos(), - End: 0, - Category: LinterName, - Message: "Literals are not allowed in the body of the logger", - SuggestedFixes: nil, - }) - return false + if !isLog { + return true } - return true + + if len(expr.Args) == 0 || !isStringValue(expr.Args[0]) { + return true + } + + pass.Report(analysis.Diagnostic{ + Pos: expr.Pos(), + End: expr.End(), + Category: LinterName, + Message: "Literals are not allowed in the body of the logger", + SuggestedFixes: nil, + }) + + return false }) } @@ -54,14 +54,6 @@ func isLogDot(expr ast.Expr) (bool, string) { if !ok { 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 { if isIdent(sel.Sel, method) { @@ -73,12 +65,10 @@ func isLogDot(expr ast.Expr) (bool, string) { func isIdent(expr ast.Expr, ident string) bool { id, ok := expr.(*ast.Ident) - if !ok { - return false - } - if id.Name == ident { + if ok && id.Name == ident { return true } + return false } diff --git a/internal/analyzers/noliteral/linter_test.go b/internal/analyzers/noliteral/linter_test.go new file mode 100644 index 0000000..b82b05e --- /dev/null +++ b/internal/analyzers/noliteral/linter_test.go @@ -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) + } +} diff --git a/internal/analyzers/no-literal/test-case/src/b_negative._go b/internal/analyzers/noliteral/test-case/src/negative._go similarity index 55% rename from internal/analyzers/no-literal/test-case/src/b_negative._go rename to internal/analyzers/noliteral/test-case/src/negative._go index 111369d..d0ffe85 100644 --- a/internal/analyzers/no-literal/test-case/src/b_negative._go +++ b/internal/analyzers/noliteral/test-case/src/negative._go @@ -1,9 +1,32 @@ 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 } +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 { Debug(msg string) Info(msg string) diff --git a/internal/analyzers/no-literal/test-case/src/b_positive._go b/internal/analyzers/noliteral/test-case/src/positive._go similarity index 62% rename from internal/analyzers/no-literal/test-case/src/b_positive._go rename to internal/analyzers/noliteral/test-case/src/positive._go index 6ad38e8..b23bd24 100644 --- a/internal/analyzers/no-literal/test-case/src/b_positive._go +++ b/internal/analyzers/noliteral/test-case/src/positive._go @@ -4,10 +4,30 @@ import ( "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 } +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 { Debug(msg string) Info(msg string) diff --git a/main.go b/main.go index 7c4ae5d..a7b4710 100644 --- a/main.go +++ b/main.go @@ -1,7 +1,7 @@ package main 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" ) @@ -9,14 +9,6 @@ var AnalyzerPlugin analyzerPlugin type analyzerPlugin struct{} -// for version ci-lint < '1.5.4'. func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer { 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 -} -*/