From 4ad1e47610dca5a0d8c5f3ac184d1536e81fc0db Mon Sep 17 00:00:00 2001 From: Alexander Chuprov Date: Tue, 18 Jul 2023 11:16:27 +0300 Subject: [PATCH] [#1] linters: add logs linter --- Makefile | 13 + README.md | 41 +++ go.mod | 10 + go.sum | 10 + internal/analyzers/no-literal/linter.go | 88 ++++++ 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/b_negative._go | 36 +++ .../no-literal/test-case/src/b_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 +++ main.go | 22 ++ 15 files changed, 776 insertions(+) create mode 100644 Makefile create mode 100644 README.md create mode 100644 go.mod create mode 100644 go.sum create mode 100644 internal/analyzers/no-literal/linter.go create mode 100644 internal/analyzers/no-literal/linter_test.go create mode 100644 internal/analyzers/no-literal/test-case/src/a_negative._go create mode 100644 internal/analyzers/no-literal/test-case/src/a_positive._go create mode 100644 internal/analyzers/no-literal/test-case/src/b_negative._go create mode 100644 internal/analyzers/no-literal/test-case/src/b_positive._go create mode 100644 internal/analyzers/no-literal/test-case/src/c_negative._go create mode 100644 internal/analyzers/no-literal/test-case/src/c_positive._go create mode 100644 internal/analyzers/no-literal/test-case/src/d_negative._go create mode 100644 internal/analyzers/no-literal/test-case/src/d_positive._go create mode 100644 main.go diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..3ed3dc2 --- /dev/null +++ b/Makefile @@ -0,0 +1,13 @@ +OUT_DIR?=./.bin +PLUGIN_SOURCE?=main.go + +test: + @go test -v ./... + +lib: + @mkdir -pv $(OUT_DIR) + @go build -buildmode=plugin -o $(OUT_DIR)/external_linters.so $(PLUGIN_SOURCE) + +lint: + @golangci-lint run + diff --git a/README.md b/README.md new file mode 100644 index 0000000..7421ae7 --- /dev/null +++ b/README.md @@ -0,0 +1,41 @@ +# linters + +## Overview + + +`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 | The tool prohibits the use of literal string arguments in logging functions | + + +## Installation + +```bash + git clone git.frostfs.info/TrueCloudLab/linters + cd linters + make lib OUT_DIR= +``` + + + +## Usage + +Add to .golagci.yml + +```yml + + linters-settings: + custom: + custom-linters: + path: + original-url: git.frostfs.info/TrueCloudLab/linters + + linters: + enable: + custom-linters +``` \ No newline at end of file diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..2a28372 --- /dev/null +++ b/go.mod @@ -0,0 +1,10 @@ +module git.frostfs.info/TrueCloudLab/linters + +go 1.19 + +require golang.org/x/tools v0.9.3 + +require ( + golang.org/x/mod v0.12.0 // indirect + golang.org/x/sys v0.10.0 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..f374fc1 --- /dev/null +++ b/go.sum @@ -0,0 +1,10 @@ +golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc= +golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA= +golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/tools v0.9.3 h1:Gn1I8+64MsuTb/HpH+LmQtNas23LhUVr3rYZ0eKuaMM= +golang.org/x/tools v0.9.3/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= +golang.org/x/tools v0.10.0 h1:tvDr/iQoUqNdohiYm0LmmKcBk+q86lb9EprIUFhHHGg= +golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM= +golang.org/x/tools v0.11.0 h1:EMCa6U9S2LtZXLAMoWiR/R8dAQFRqbAitmbJ2UKhoi8= +golang.org/x/tools v0.11.0/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= diff --git a/internal/analyzers/no-literal/linter.go b/internal/analyzers/no-literal/linter.go new file mode 100644 index 0000000..22573ae --- /dev/null +++ b/internal/analyzers/no-literal/linter.go @@ -0,0 +1,88 @@ +package noliteral + +import ( + "go/ast" + "go/token" + "strings" + "sync" + + "golang.org/x/tools/go/analysis" +) + +const LinterName = "noliteral" + +var LogsAnalyzer = &analysis.Analyzer{ + Name: LinterName, + Doc: LinterName + " is a helper tool that ensures logging messages in Go code are structured and not written as simple text.", + Run: run, +} + +var ( + methodsToSearchOnce = &sync.Once{} + methodsToSearch = []string{"Debug", "Info", "Warn", "Error"} + customLogs = "reportFlushError, reportError" +) + +func run(pass *analysis.Pass) (interface{}, error) { + for _, file := range pass.Files { + ast.Inspect(file, func(n ast.Node) bool { + expr, ok := n.(*ast.CallExpr) + if !ok { + return true + } + + 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 + } + return true + }) + } + + return nil, nil +} + +func isLogDot(expr ast.Expr) (bool, string) { + sel, ok := expr.(*ast.SelectorExpr) + 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) { + return true, method + } + } + return false, "" +} + +func isIdent(expr ast.Expr, ident string) bool { + id, ok := expr.(*ast.Ident) + if !ok { + return false + } + if id.Name == ident { + return true + } + return false +} + +func isStringValue(expr ast.Expr) bool { + basicLit, ok := expr.(*ast.BasicLit) + return ok && basicLit.Kind == token.STRING +} diff --git a/internal/analyzers/no-literal/linter_test.go b/internal/analyzers/no-literal/linter_test.go new file mode 100644 index 0000000..69b885f --- /dev/null +++ b/internal/analyzers/no-literal/linter_test.go @@ -0,0 +1,264 @@ +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 new file mode 100644 index 0000000..39674bc --- /dev/null +++ b/internal/analyzers/no-literal/test-case/src/a_negative._go @@ -0,0 +1,36 @@ +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 new file mode 100644 index 0000000..2920216 --- /dev/null +++ b/internal/analyzers/no-literal/test-case/src/a_positive._go @@ -0,0 +1,46 @@ +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/b_negative._go b/internal/analyzers/no-literal/test-case/src/b_negative._go new file mode 100644 index 0000000..111369d --- /dev/null +++ b/internal/analyzers/no-literal/test-case/src/b_negative._go @@ -0,0 +1,36 @@ +package src_test + +func (c *cfg) f1_n() { + c.log.Debug("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/b_positive._go b/internal/analyzers/no-literal/test-case/src/b_positive._go new file mode 100644 index 0000000..6ad38e8 --- /dev/null +++ b/internal/analyzers/no-literal/test-case/src/b_positive._go @@ -0,0 +1,46 @@ +package src_test + +import ( + "fmt" +) + +func (c *cfg) f1_ok() { + c.log.Debug(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 new file mode 100644 index 0000000..f3cdccb --- /dev/null +++ b/internal/analyzers/no-literal/test-case/src/c_negative._go @@ -0,0 +1,36 @@ +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 new file mode 100644 index 0000000..77a8fd7 --- /dev/null +++ b/internal/analyzers/no-literal/test-case/src/c_positive._go @@ -0,0 +1,46 @@ +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 new file mode 100644 index 0000000..56c5024 --- /dev/null +++ b/internal/analyzers/no-literal/test-case/src/d_negative._go @@ -0,0 +1,36 @@ +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 new file mode 100644 index 0000000..5ed4b82 --- /dev/null +++ b/internal/analyzers/no-literal/test-case/src/d_positive._go @@ -0,0 +1,46 @@ +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/main.go b/main.go new file mode 100644 index 0000000..7c4ae5d --- /dev/null +++ b/main.go @@ -0,0 +1,22 @@ +package main + +import ( + noliteral "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/no-literal" + "golang.org/x/tools/go/analysis" +) + +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 +} +*/