[#10] linters: Add useStrconv linter

Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
This commit is contained in:
Alexander Chuprov 2023-08-16 17:46:20 +03:00
parent cb737e3a3e
commit a2983f6cb8
6 changed files with 243 additions and 27 deletions

View file

@ -1,25 +1,25 @@
# linters # linters
## Overview ## 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 ## Usage
Add to .golangci.yml Add to .golangci.yml
```yml ```yml
linters-settings: linters-settings:
custom: custom:
custom-linters: truecloudlab-linters:
path: <Path to the directory with libraries> path: <Path to the directory with libraries>
original-url: git.frostfs.info/TrueCloudLab/linters original-url: git.frostfs.info/TrueCloudLab/linters
linters: linters:
enable: enable:
custom-linters truecloudlab-linters
``` ```
@ -33,39 +33,44 @@ Add to .golangci.yml
## Available linters ## Available linters
| Name | Description | | Name | Description |
| ----------------------- | --------------------------------------------------------------------------- | |-------------------------|-------------------------------------------------------------------------------------------------------------------------------|
| [noliteral](#noliteral) | The tool prohibits the use of literal string arguments in logging functions | | [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 ## Linters Configuration
Settings via a configuration file are available if golangci-lint >= 1.5.4 is used Settings via a configuration file are available if golangci-lint >= 1.5.4 is used
### noliteral ### noliteral
##### File Configuration ##### File Configuration
```yml ```yml
linters-settings: linters-settings:
custom: custom:
noliteral: truecloudlab-linters:
path: .bin/external_linters.so path: .bin/external_linters.so
original-url: git.frostfs.info/TrueCloudLab/linters.git original-url: git.frostfs.info/TrueCloudLab/linters.git
settings: settings:
target-methods: ["reportFlushError", "reportError"] # optional. Enabled by default "Debug", "Info", "Warn", "Error" noliteral:
disable-packages: ["pkg1", "pkg2"] # List of packages for which the check should be disabled. enable: true # optional
constants-package: "git.frostfs.info/rep/logs" # if not set, then the check is disabled 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 | ##### File Configuration
| ----------------------------- | ------------------------------------------------------- | ```yml
| `NOLITERAL_TARGET_METHODS` | List of methods to analyze | linters-settings:
| `NOLITERAL_DISABLE_PACKAGES` | List of packages for which the check should be disabled | custom:
| `NOLITERAL_CONSTANTS_PACKAGE` | Path to the package with constants | truecloudlab-linters:
path: .bin/external_linters.so
original-url: git.frostfs.info/TrueCloudLab/linters.git
**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). settings:
useStrconv: # optional
enable: true
```

View file

@ -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]
}

View file

@ -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)
}
}

View file

@ -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)
}
}

View file

@ -0,0 +1,11 @@
package src_test
import (
"fmt"
)
func main(){
for i := 0; i < b.N; i++ {
s := fmt.Sprintf("%d ", height)
}
}

11
main.go
View file

@ -2,6 +2,7 @@ package main
import ( import (
"git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral" "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral"
useStrconv "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/use-strconv"
"golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis"
) )
@ -19,14 +20,20 @@ func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
func New(conf any) ([]*analysis.Analyzer, error) { func New(conf any) ([]*analysis.Analyzer, error) {
confMap, ok := conf.(map[string]any) confMap, ok := conf.(map[string]any)
var noliteralConfig any var noliteralConfig, useStrconvConfig any
if ok { if ok {
noliteralConfig = confMap["noliteral"] noliteralConfig = confMap["noliteral"]
useStrconvConfig = confMap["useStrconv"]
} }
noliteral, err := noliteral.New(noliteralConfig) noliteral, err := noliteral.New(noliteralConfig)
if err != nil { if err != nil {
return nil, err 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
} }