From a2983f6cb81fafb8562c9593670c35b31c770583 Mon Sep 17 00:00:00 2001 From: Alexander Chuprov Date: Wed, 16 Aug 2023 17:46:20 +0300 Subject: [PATCH] [#10] linters: Add useStrconv linter Signed-off-by: Alexander Chuprov --- README.md | 55 ++++----- internal/analyzers/use-strconv/linter.go | 104 ++++++++++++++++++ internal/analyzers/use-strconv/linter_test.go | 74 +++++++++++++ .../use-strconv/test-case/negative._go | 15 +++ .../use-strconv/test-case/positive._go | 11 ++ main.go | 11 +- 6 files changed, 243 insertions(+), 27 deletions(-) create mode 100644 internal/analyzers/use-strconv/linter.go create mode 100644 internal/analyzers/use-strconv/linter_test.go create mode 100644 internal/analyzers/use-strconv/test-case/negative._go create mode 100644 internal/analyzers/use-strconv/test-case/positive._go diff --git a/README.md b/README.md index ed1c8e3..d19166d 100644 --- a/README.md +++ b/README.md @@ -1,25 +1,25 @@ -# linters +# 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. +`linters` is a project that enables the integration of custom linting rules into the [golangci-lint](https://github.com/golangci/golangci-lint) framework. ## Usage -Add to .golangci.yml +Add to .golangci.yml ```yml linters-settings: custom: - custom-linters: + truecloudlab-linters: path: original-url: git.frostfs.info/TrueCloudLab/linters linters: enable: - custom-linters + truecloudlab-linters ``` @@ -33,39 +33,44 @@ Add to .golangci.yml ## Available linters -| Name | Description | -| ----------------------- | --------------------------------------------------------------------------- | -| [noliteral](#noliteral) | The tool prohibits the use of literal string arguments in logging functions | +| Name | Description | +|-------------------------|-------------------------------------------------------------------------------------------------------------------------------| +| [noliteral](#noliteral) | The tool prohibits the use of literal string arguments in logging functions. | +| [useStrconv](#useStrconv) | The `useStrconv` linter recommends the utilization of `strconv` over `fmt` when performing string conversions of primitive data types. Detailed guidelines can be found in the [official Uber Go Style Guide](https://github.com/uber-go/guide/blob/master/style.md#prefer-strconv-over-fmt). | + ## Linters Configuration Settings via a configuration file are available if golangci-lint >= 1.5.4 is used -### noliteral +### noliteral ##### File Configuration ```yml linters-settings: custom: - noliteral: + truecloudlab-linters: path: .bin/external_linters.so - original-url: git.frostfs.info/TrueCloudLab/linters.git + 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 - + noliteral: + enable: true # optional + 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 +### useStrconv -| 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). - +##### File Configuration +```yml +linters-settings: + custom: + truecloudlab-linters: + path: .bin/external_linters.so + original-url: git.frostfs.info/TrueCloudLab/linters.git + settings: + useStrconv: # optional + enable: true +``` diff --git a/internal/analyzers/use-strconv/linter.go b/internal/analyzers/use-strconv/linter.go new file mode 100644 index 0000000..fe6ca1f --- /dev/null +++ b/internal/analyzers/use-strconv/linter.go @@ -0,0 +1,104 @@ +package usestrconv + +import ( + "go/ast" + + astutils "git.frostfs.info/TrueCloudLab/linters/pkg/ast-utils" + "github.com/mitchellh/mapstructure" + "golang.org/x/tools/go/analysis" +) + +const LinterName = "useStrconv" + +var LogsAnalyzer = &analysis.Analyzer{ + Name: LinterName, + Doc: LinterName + " linter recommends the utilization of `strconv` over `fmt` when performing string conversions of primitive data types.", + Run: run, +} + +type Configuration struct { + Enable bool `mapstructure:"enable"` +} + +var Config = Configuration{ + Enable: true, +} +var modyficators = []string{"%d", "%f", "%t", "%x"} + +func New(conf any) (*analysis.Analyzer, error) { + configMap, ok := conf.(map[string]any) + if !ok { + Config.Enable = true + return LogsAnalyzer, nil + } + + var config Configuration + config.Enable = true + err := mapstructure.Decode(configMap, &config) + if err != nil { + return nil, err + } + + Config.Enable = config.Enable + return LogsAnalyzer, nil +} + +func run(pass *analysis.Pass) (interface{}, error) { + if !Config.Enable { + return nil, nil + } + + for _, file := range pass.Files { + ast.Inspect(file, func(n ast.Node) bool { + expr, ok := n.(*ast.CallExpr) + if !ok { + return true + } + + expectedPackageAlias, err := astutils.GetAliasByPkgName(file, "fmt") + if err != nil { + return false + } + if expectedPackageAlias != astutils.GetPackageName(expr.Fun) { + return true + } + + isTarget, _ := astutils.IsTargetMethod(expr.Fun, []string{"Sprintf"}) + + if !isTarget || !astutils.IsStringValue(expr.Args[0]) { + return true + } + + stringLiteral, ok := expr.Args[0].(*ast.BasicLit) + + if !ok { + return true + } + + modValue := unquote(stringLiteral.Value) + for _, modificator := range modyficators { + if modValue == modificator { + pass.Report(analysis.Diagnostic{ + Pos: expr.Pos(), + End: expr.End(), + Category: LinterName, + Message: `Usage of fmt.Sprintf(` + modificator + `) is not allowed`, + SuggestedFixes: nil, + }) + return false + } + } + + return true + }) + } + + return nil, nil +} + +func unquote(s string) string { + if len(s) < 2 { + return s + } + return s[1 : len(s)-1] +} diff --git a/internal/analyzers/use-strconv/linter_test.go b/internal/analyzers/use-strconv/linter_test.go new file mode 100644 index 0000000..e7b51de --- /dev/null +++ b/internal/analyzers/use-strconv/linter_test.go @@ -0,0 +1,74 @@ +package usestrconv + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "runtime" + "testing" + + "golang.org/x/tools/go/analysis" +) + +func init() { + Config.Enable = true +} +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/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_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/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/use-strconv/test-case/negative._go b/internal/analyzers/use-strconv/test-case/negative._go new file mode 100644 index 0000000..cfe69e0 --- /dev/null +++ b/internal/analyzers/use-strconv/test-case/negative._go @@ -0,0 +1,15 @@ +package src_test + +import ( + "fmt" +) + +func main(){ + for i := 0; i < b.N; i++ { + s := fmt.Sprintf("%d", height) + s := fmt.Sprintf("%x", height) + s := fmt.Sprintf("%f", height) + s := fmt.Sprintf("%t", height) + + } +} diff --git a/internal/analyzers/use-strconv/test-case/positive._go b/internal/analyzers/use-strconv/test-case/positive._go new file mode 100644 index 0000000..17fdca2 --- /dev/null +++ b/internal/analyzers/use-strconv/test-case/positive._go @@ -0,0 +1,11 @@ +package src_test + +import ( + "fmt" +) + +func main(){ + for i := 0; i < b.N; i++ { + s := fmt.Sprintf("%d ", height) + } +} diff --git a/main.go b/main.go index e5136d2..e7d3b03 100644 --- a/main.go +++ b/main.go @@ -2,6 +2,7 @@ package main import ( "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral" + useStrconv "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/use-strconv" "golang.org/x/tools/go/analysis" ) @@ -19,14 +20,20 @@ func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer { func New(conf any) ([]*analysis.Analyzer, error) { confMap, ok := conf.(map[string]any) - var noliteralConfig any + var noliteralConfig, useStrconvConfig any if ok { noliteralConfig = confMap["noliteral"] + useStrconvConfig = confMap["useStrconv"] } noliteral, err := noliteral.New(noliteralConfig) if err != nil { return nil, err } - return []*analysis.Analyzer{noliteral}, nil + useStrconv, err := useStrconv.New(useStrconvConfig) + if err != nil { + return nil, err + } + + return []*analysis.Analyzer{noliteral, useStrconv}, nil }