generated from TrueCloudLab/basic
[#10] linters: add useStrconv linter #10
9 changed files with 389 additions and 141 deletions
37
README.md
37
README.md
|
@ -12,14 +12,14 @@ Add to .golangci.yml
|
|||
|
||||
linters-settings:
|
||||
custom:
|
||||
custom-linters:
|
||||
truecloudlab-linters:
|
||||
path: <Path to the directory with libraries>
|
||||
original-url: git.frostfs.info/TrueCloudLab/linters
|
||||
|
||||
|
||||
linters:
|
||||
enable:
|
||||
custom-linters
|
||||
truecloudlab-linters
|
||||
```
|
||||
|
||||
|
||||
|
@ -34,8 +34,10 @@ Add to .golangci.yml
|
|||
## Available linters
|
||||
|
||||
| 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
|
||||
|
||||
|
@ -48,24 +50,27 @@ Settings via a configuration file are available if golangci-lint >= 1.5.4 is use
|
|||
```yml
|
||||
linters-settings:
|
||||
custom:
|
||||
noliteral:
|
||||
truecloudlab-linters:
|
||||
path: .bin/external_linters.so
|
||||
original-url: git.frostfs.info/TrueCloudLab/linters.git
|
||||
settings:
|
||||
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
|
||||
```
|
||||
|
|
|
@ -2,10 +2,9 @@ package noliteral
|
|||
|
||||
import (
|
||||
"go/ast"
|
||||
"go/token"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
astutils "git.frostfs.info/TrueCloudLab/linters/pkg/ast-utils"
|
||||
"github.com/mitchellh/mapstructure"
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
||||
|
@ -17,21 +16,39 @@ var LogsAnalyzer = &analysis.Analyzer{
|
|||
Run: run,
|
||||
}
|
||||
|
||||
var (
|
||||
aliasCache = sync.Map{}
|
||||
)
|
||||
|
||||
type Configuration struct {
|
||||
TargetMethods []string `mapstructure:"target-methods"`
|
||||
DisablePackages []string `mapstructure:"disable-packages"`
|
||||
ConstantsPackage string `mapstructure:"constants-package"`
|
||||
Enable bool `mapstructure:"enable"`
|
||||
}
|
||||
|
||||
var Config = Configuration{
|
||||
TargetMethods: []string{"Debug", "Info", "Warn", "Error"},
|
||||
}
|
||||
|
||||
func New(conf any) (*analysis.Analyzer, error) {
|
||||
var config Configuration
|
||||
config.Enable = true
|
||||
|
||||
err := mapstructure.Decode(conf, &config)
|
||||
if err != nil {
|
||||
Config.Enable = true
|
||||
return LogsAnalyzer, nil
|
||||
}
|
||||
|
||||
Config.TargetMethods = append(Config.TargetMethods, config.TargetMethods...)
|
||||
Config.ConstantsPackage = config.ConstantsPackage
|
||||
Config.DisablePackages = config.DisablePackages
|
||||
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)
|
||||
|
@ -39,19 +56,19 @@ func run(pass *analysis.Pass) (interface{}, error) {
|
|||
return true
|
||||
}
|
||||
|
||||
isLog, _ := isLogDot(expr.Fun)
|
||||
if !isLog || len(expr.Args) == 0 {
|
||||
isLog, _ := astutils.IsTargetMethod(expr.Fun, Config.TargetMethods)
|
||||
if !isLog || len(expr.Args) == 0 || astutils.HasNoLintComment(pass, expr.Pos()) {
|
||||
return true
|
||||
}
|
||||
|
||||
if !isStringValue(expr.Args[0]) {
|
||||
alias, _ := getAliasByPkgName(file, Config.ConstantsPackage)
|
||||
if Config.ConstantsPackage == "" || getPackageName(expr.Args[0]) == alias || getPackageName(expr.Args[0]) == "" {
|
||||
if !astutils.IsStringValue(expr.Args[0]) {
|
||||
alias, _ := astutils.GetAliasByPkgName(file, Config.ConstantsPackage)
|
||||
if Config.ConstantsPackage == "" || astutils.GetPackageName(expr.Args[0]) == alias || astutils.GetPackageName(expr.Args[0]) == "" {
|
||||
return true
|
||||
}
|
||||
|
||||
for _, pkgName := range Config.DisablePackages {
|
||||
if pkgName == getPackageName(expr.Args[0]) {
|
||||
if pkgName == astutils.GetPackageName(expr.Args[0]) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
@ -80,81 +97,3 @@ func run(pass *analysis.Pass) (interface{}, error) {
|
|||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func isLogDot(expr ast.Expr) (bool, string) {
|
||||
sel, ok := expr.(*ast.SelectorExpr)
|
||||
if !ok {
|
||||
return false, ""
|
||||
}
|
||||
|
||||
for _, method := range Config.TargetMethods {
|
||||
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 && id.Name == ident {
|
||||
return true
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func isStringValue(expr ast.Expr) bool {
|
||||
basicLit, ok := expr.(*ast.BasicLit)
|
||||
return ok && basicLit.Kind == token.STRING
|
||||
}
|
||||
|
||||
func getAliasByPkgName(file *ast.File, pkgName string) (string, error) {
|
||||
if alias, ok := aliasCache.Load(file); ok {
|
||||
return alias.(string), nil
|
||||
}
|
||||
|
||||
var alias string
|
||||
specs := file.Imports
|
||||
|
||||
for _, spec := range specs {
|
||||
alias = getAliasFromImportSpec(spec, pkgName)
|
||||
if alias != "" {
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
aliasCache.Store(file, alias)
|
||||
return alias, nil
|
||||
}
|
||||
|
||||
func getAliasFromImportSpec(spec *ast.ImportSpec, pkgName string) string {
|
||||
if spec == nil {
|
||||
return ""
|
||||
}
|
||||
importName := strings.Replace(spec.Path.Value, "\"", "", -1)
|
||||
if importName != pkgName {
|
||||
return ""
|
||||
}
|
||||
|
||||
split := strings.Split(importName, "/")
|
||||
|
||||
if len(split) == 0 {
|
||||
return ""
|
||||
}
|
||||
alias := split[len(split)-1]
|
||||
if spec.Name != nil {
|
||||
alias = spec.Name.Name
|
||||
}
|
||||
|
||||
return alias
|
||||
}
|
||||
|
||||
func getPackageName(expr ast.Expr) string {
|
||||
if selectorExpr, ok := expr.(*ast.SelectorExpr); ok {
|
||||
if ident, ok := selectorExpr.X.(*ast.Ident); ok {
|
||||
return ident.Name
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
|
|
@ -13,6 +13,7 @@ import (
|
|||
|
||||
func init() {
|
||||
Config.ConstantsPackage = "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs"
|
||||
Config.Enable = true
|
||||
}
|
||||
|
||||
func TestAnalyzer_negative(t *testing.T) {
|
||||
|
|
104
internal/analyzers/use-strconv/linter.go
Normal file
104
internal/analyzers/use-strconv/linter.go
Normal 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 || astutils.HasNoLintComment(pass, expr.Pos()) {
|
||||
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]
|
||||
}
|
74
internal/analyzers/use-strconv/linter_test.go
Normal file
74
internal/analyzers/use-strconv/linter_test.go
Normal 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)
|
||||
}
|
||||
}
|
15
internal/analyzers/use-strconv/test-case/negative._go
Normal file
15
internal/analyzers/use-strconv/test-case/negative._go
Normal 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)
|
||||
|
||||
}
|
||||
}
|
11
internal/analyzers/use-strconv/test-case/positive._go
Normal file
11
internal/analyzers/use-strconv/test-case/positive._go
Normal file
|
@ -0,0 +1,11 @@
|
|||
package src_test
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
)
|
||||
|
||||
func main(){
|
||||
for i := 0; i < b.N; i++ {
|
||||
s := fmt.Sprintf("%d ", height)
|
||||
}
|
||||
}
|
38
main.go
38
main.go
|
@ -1,11 +1,8 @@
|
|||
package main
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strings"
|
||||
|
||||
noliteral "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral"
|
||||
"github.com/mitchellh/mapstructure"
|
||||
"git.frostfs.info/TrueCloudLab/linters/internal/analyzers/noliteral"
|
||||
useStrconv "git.frostfs.info/TrueCloudLab/linters/internal/analyzers/use-strconv"
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
||||
|
@ -21,31 +18,22 @@ func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer {
|
|||
|
||||
// for version ci-lint >= '1.5.4'.
|
||||
func New(conf any) ([]*analysis.Analyzer, error) {
|
||||
targetMethods := strings.Split(os.Getenv("NOLITERAL_TARGET_METHODS"), ",")
|
||||
disablePackages := strings.Split(os.Getenv("NOLITERAL_DISABLE_PACKAGES"), ",")
|
||||
constantsPackage := os.Getenv("NOLITERAL_CONSTANTS_PACKAGE")
|
||||
confMap, ok := conf.(map[string]any)
|
||||
|
||||
configMap := map[string]any{
|
||||
"target-methods": targetMethods,
|
||||
"constants-package": constantsPackage,
|
||||
"disable-packages": disablePackages,
|
||||
var noliteralConfig, useStrconvConfig any
|
||||
if ok {
|
||||
noliteralConfig = confMap["noliteral"]
|
||||
useStrconvConfig = confMap["useStrconv"]
|
||||
}
|
||||
|
||||
if confMap, ok := conf.(map[string]any); ok {
|
||||
for k, v := range confMap {
|
||||
configMap[k] = v
|
||||
}
|
||||
}
|
||||
|
||||
var config noliteral.Configuration
|
||||
err := mapstructure.Decode(configMap, &config)
|
||||
noliteral, err := noliteral.New(noliteralConfig)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
noliteral.Config.TargetMethods = append(noliteral.Config.TargetMethods, config.TargetMethods...)
|
||||
noliteral.Config.ConstantsPackage = config.ConstantsPackage
|
||||
noliteral.Config.DisablePackages = config.DisablePackages
|
||||
useStrconv, err := useStrconv.New(useStrconvConfig)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return []*analysis.Analyzer{noliteral.LogsAnalyzer}, nil
|
||||
return []*analysis.Analyzer{noliteral, useStrconv}, nil
|
||||
}
|
||||
|
|
111
pkg/ast-utils/ast.go
Normal file
111
pkg/ast-utils/ast.go
Normal file
|
@ -0,0 +1,111 @@
|
|||
package astutils
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"go/token"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
||||
type aliasCacheKey struct {
|
||||
File *ast.File
|
||||
PkgName string
|
||||
}
|
||||
|
||||
var (
|
||||
aliasCache = sync.Map{}
|
||||
)
|
||||
|
||||
func IsTargetMethod(expr ast.Expr, targetMethods []string) (bool, string) {
|
||||
sel, ok := expr.(*ast.SelectorExpr)
|
||||
if !ok {
|
||||
return false, ""
|
||||
}
|
||||
|
||||
for _, method := range targetMethods {
|
||||
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 && id.Name == ident {
|
||||
return true
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func IsStringValue(expr ast.Expr) bool {
|
||||
basicLit, ok := expr.(*ast.BasicLit)
|
||||
return ok && basicLit.Kind == token.STRING
|
||||
}
|
||||
|
||||
func GetAliasByPkgName(file *ast.File, pkgName string) (string, error) {
|
||||
key := aliasCacheKey{File: file, PkgName: pkgName}
|
||||
if alias, ok := aliasCache.Load(key); ok {
|
||||
return alias.(string), nil
|
||||
}
|
||||
|
||||
var alias string
|
||||
specs := file.Imports
|
||||
|
||||
for _, spec := range specs {
|
||||
alias = GetAliasFromImportSpec(spec, pkgName)
|
||||
if alias != "" {
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
aliasCache.Store(key, alias)
|
||||
return alias, nil
|
||||
}
|
||||
|
||||
func GetAliasFromImportSpec(spec *ast.ImportSpec, pkgName string) string {
|
||||
if spec == nil {
|
||||
return ""
|
||||
}
|
||||
importName := strings.Replace(spec.Path.Value, "\"", "", -1)
|
||||
if importName != pkgName {
|
||||
return ""
|
||||
}
|
||||
|
||||
split := strings.Split(importName, "/")
|
||||
|
||||
if len(split) == 0 {
|
||||
return ""
|
||||
}
|
||||
alias := split[len(split)-1]
|
||||
if spec.Name != nil {
|
||||
alias = spec.Name.Name
|
||||
}
|
||||
|
||||
return alias
|
||||
}
|
||||
|
||||
func GetPackageName(expr ast.Expr) string {
|
||||
if selectorExpr, ok := expr.(*ast.SelectorExpr); ok {
|
||||
if ident, ok := selectorExpr.X.(*ast.Ident); ok {
|
||||
return ident.Name
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func HasNoLintComment(pass *analysis.Pass, pos token.Pos) bool {
|
||||
for _, commentGroup := range pass.Files[0].Comments {
|
||||
if commentGroup.End() < pos {
|
||||
for _, comment := range commentGroup.List {
|
||||
if strings.Contains(comment.Text, "nolint") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
Loading…
Reference in a new issue