Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

analyzer: add options to cover all cases #42

Merged
merged 2 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,23 @@ go install github.com/catenacyber/perfsprint@latest
perfsprint --fix ./...
```

To disable int/uint cast, you can use the flag `-int-conversion=false`
## Options

The 5 following options cover all optimizations proposed by the linter.

Some have suboptions for specific cases.

- integer-format (formatting integer with the package `strconv`)
- int-conversion : disable when the optimization adds a int/uint cast (readability)
- error-format (formatting errors)
- errorf : known behavior change avoiding panic
- err-error : known behavior change panicking for nil errors
- string-format (formatting strings)
- sprintf1 : known behavior change avoiding panic
- strconcat : disable turning some `fmt.Sprintf` to a string concatenation (readability)
- bool-format (formatting bool with `strconv.FormatBool`)
- hex-format (formatting slices with `hex.EncodeToString`)
catenacyber marked this conversation as resolved.
Show resolved Hide resolved


To disable `fmt.Errorf` optimization, you can use the flag `-errorf=false`
This optimization is not always equivalent.
Expand Down
97 changes: 67 additions & 30 deletions analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,41 @@ import (
"golang.org/x/tools/go/analysis"
)

type optionInt struct {
enabled bool
intConv bool
}

type optionErr struct {
enabled bool
errError bool
errorf bool
}

type optionStr struct {
enabled bool
sprintf1 bool
strconcat bool
}
catenacyber marked this conversation as resolved.
Show resolved Hide resolved

type perfSprint struct {
intConv bool
errError bool
errorf bool
sprintf1 bool
intFormat optionInt
errFormat optionErr
strFormat optionStr

boolFormat bool
hexFormat bool
fiximports bool
strconcat bool
}

func newPerfSprint() *perfSprint {
return &perfSprint{
intConv: true,
errError: false,
errorf: true,
sprintf1: true,
intFormat: optionInt{enabled: true, intConv: true},
errFormat: optionErr{enabled: true, errError: false, errorf: true},
strFormat: optionStr{enabled: true, sprintf1: true, strconcat: true},
boolFormat: true,
hexFormat: true,
fiximports: true,
strconcat: true,
}
}

Expand All @@ -44,12 +62,31 @@ func New() *analysis.Analyzer {
Run: n.run,
Requires: []*analysis.Analyzer{inspect.Analyzer},
}
r.Flags.BoolVar(&n.intConv, "int-conversion", true, "optimizes even if it requires an int or uint type cast")
r.Flags.BoolVar(&n.errError, "err-error", false, "optimizes into err.Error() even if it is only equivalent for non-nil errors")
r.Flags.BoolVar(&n.errorf, "errorf", true, "optimizes fmt.Errorf")
r.Flags.BoolVar(&n.sprintf1, "sprintf1", true, "optimizes fmt.Sprintf with only one argument")
r.Flags.BoolVar(&n.intFormat.enabled, "integer-format", true, "enable/disable optimization of integer formatting")
r.Flags.BoolVar(&n.intFormat.intConv, "int-conversion", true, "optimizes even if it requires an int or uint type cast")
r.Flags.BoolVar(&n.errFormat.enabled, "error-format", true, "enable/disable optimization of error formatting")
r.Flags.BoolVar(&n.errFormat.errError, "err-error", false, "optimizes into err.Error() even if it is only equivalent for non-nil errors")
r.Flags.BoolVar(&n.errFormat.errorf, "errorf", true, "optimizes fmt.Errorf")
r.Flags.BoolVar(&n.boolFormat, "bool-format", true, "enable/disable optimization of bool formatting")
r.Flags.BoolVar(&n.hexFormat, "hex-format", true, "enable/disable optimization of hex formatting")
r.Flags.BoolVar(&n.strFormat.enabled, "string-format", true, "enable/disable optimization of string formatting")
r.Flags.BoolVar(&n.strFormat.sprintf1, "sprintf1", true, "optimizes fmt.Sprintf with only one argument")
r.Flags.BoolVar(&n.strFormat.strconcat, "strconcat", true, "optimizes into strings concatenation")
r.Flags.BoolVar(&n.fiximports, "fiximports", true, "fix needed imports from other fixes")
Comment on lines +65 to 75
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
r.Flags.BoolVar(&n.intFormat.enabled, "integer-format", true, "enable/disable optimization of integer formatting")
r.Flags.BoolVar(&n.intFormat.intConv, "int-conversion", true, "optimizes even if it requires an int or uint type cast")
r.Flags.BoolVar(&n.errFormat.enabled, "error-format", true, "enable/disable optimization of error formatting")
r.Flags.BoolVar(&n.errFormat.errError, "err-error", false, "optimizes into err.Error() even if it is only equivalent for non-nil errors")
r.Flags.BoolVar(&n.errFormat.errorf, "errorf", true, "optimizes fmt.Errorf")
r.Flags.BoolVar(&n.boolFormat, "bool-format", true, "enable/disable optimization of bool formatting")
r.Flags.BoolVar(&n.hexFormat, "hex-format", true, "enable/disable optimization of hex formatting")
r.Flags.BoolVar(&n.strFormat.enabled, "string-format", true, "enable/disable optimization of string formatting")
r.Flags.BoolVar(&n.strFormat.sprintf1, "sprintf1", true, "optimizes fmt.Sprintf with only one argument")
r.Flags.BoolVar(&n.strFormat.strconcat, "strconcat", true, "optimizes into strings concatenation")
r.Flags.BoolVar(&n.fiximports, "fiximports", true, "fix needed imports from other fixes")
r.Flags.BoolVar(&n.intFormat.activated, "integer-format", true, "activate/deactivate optimization of integer formatting")
r.Flags.BoolVar(&n.intFormat.intConv, "int-conversion", true, "optimizes even if it requires an int or uint type cast")
r.Flags.BoolVar(&n.errFormat.activated, "error-format", true, "activate/deactivate optimization of error formatting")
r.Flags.BoolVar(&n.errFormat.errError, "err-error", false, "optimizes into err.Error() even if it is only equivalent for non-nil errors")
r.Flags.BoolVar(&n.errFormat.errorf, "errorf", true, "optimizes fmt.Errorf")
r.Flags.BoolVar(&n.boolFormat, "bool-format", true, "activate/deactivate optimization of bool formatting")
r.Flags.BoolVar(&n.hexFormat, "hex-format", true, "activate/deactivate optimization of hex formatting")
r.Flags.BoolVar(&n.strFormat.activated, "string-format", true, "activate/deactivate optimization of string formatting")
r.Flags.BoolVar(&n.strFormat.sprintf1, "sprintf1", true, "optimizes fmt.Sprintf with only one argument")
r.Flags.BoolVar(&n.strFormat.strconcat, "strconcat", true, "optimizes into strings concatenation")
r.Flags.BoolVar(&n.fiximports, "fiximports", true, "fix needed imports from other fixes")

r.Flags.BoolVar(&n.strconcat, "strconcat", true, "optimizes into strings concatenation")

if !n.intFormat.enabled {
n.intFormat.intConv = false
}

if !n.errFormat.enabled {
n.errFormat.errError = false
n.errFormat.errorf = false
}
if !n.strFormat.enabled {
n.strFormat.sprintf1 = false
n.strFormat.strconcat = false
}

return r
}

Expand Down Expand Up @@ -102,7 +139,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
err error
)
switch {
case calledObj == fmtErrorfObj && len(call.Args) == 1 && n.errorf:
case calledObj == fmtErrorfObj && len(call.Args) == 1 && n.errFormat.errorf:
fn = "fmt.Errorf"
verb = "%s"
value = call.Args[0]
Expand All @@ -112,7 +149,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
verb = "%v"
value = call.Args[0]

case calledObj == fmtSprintfObj && len(call.Args) == 1 && n.sprintf1:
case calledObj == fmtSprintfObj && len(call.Args) == 1 && n.strFormat.sprintf1:
fn = "fmt.Sprintf"
verb = "%s"
value = call.Args[0]
Expand Down Expand Up @@ -141,7 +178,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {

switch verb {
default:
if fn == "fmt.Sprintf" && isConcatable(verb) && n.strconcat {
if fn == "fmt.Sprintf" && isConcatable(verb) && n.strFormat.strconcat {
break
}
return
Expand All @@ -160,7 +197,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
neededPackages[fname] = make(map[string]struct{})
}
removedFmtUsages[fname]++
if fn == "fmt.Errorf" {
if fn == "fmt.Errorf" && n.errFormat.enabled {
neededPackages[fname]["errors"] = struct{}{}
d = newAnalysisDiagnostic(
"", // TODO: precise checker
Expand All @@ -177,7 +214,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
},
)
} else {
} else if fn != "fmt.Errorf" && n.strFormat.enabled {
d = newAnalysisDiagnostic(
"", // TODO: precise checker
call,
Expand All @@ -194,7 +231,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
)
}
case types.Implements(valueType, errIface) && oneOf(verb, "%v", "%s") && n.errError:
case types.Implements(valueType, errIface) && oneOf(verb, "%v", "%s") && n.errFormat.errError:
// known false positive if this error is nil
// fmt.Sprint(nil) does not panic like nil.Error() does
errMethodCall := formatNode(pass.Fset, value) + ".Error()"
Expand All @@ -216,7 +253,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
)

case isBasicType(valueType, types.Bool) && oneOf(verb, "%v", "%t"):
case isBasicType(valueType, types.Bool) && oneOf(verb, "%v", "%t") && n.boolFormat:
fname := pass.Fset.File(call.Pos()).Name()
removedFmtUsages[fname]++
if _, ok := neededPackages[fname]; !ok {
Expand All @@ -239,7 +276,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
)

case isArray && isBasicType(a.Elem(), types.Uint8) && oneOf(verb, "%x"):
case isArray && isBasicType(a.Elem(), types.Uint8) && oneOf(verb, "%x") && n.hexFormat:
if _, ok := value.(*ast.Ident); !ok {
// Doesn't support array literals.
return
Expand Down Expand Up @@ -273,7 +310,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
},
)
case isSlice && isBasicType(s.Elem(), types.Uint8) && oneOf(verb, "%x"):
case isSlice && isBasicType(s.Elem(), types.Uint8) && oneOf(verb, "%x") && n.hexFormat:
fname := pass.Fset.File(call.Pos()).Name()
removedFmtUsages[fname]++
if _, ok := neededPackages[fname]; !ok {
Expand All @@ -296,7 +333,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
)

case isBasicType(valueType, types.Int8, types.Int16, types.Int32) && oneOf(verb, "%v", "%d") && n.intConv:
case isBasicType(valueType, types.Int8, types.Int16, types.Int32) && oneOf(verb, "%v", "%d") && n.intFormat.intConv:
fname := pass.Fset.File(call.Pos()).Name()
removedFmtUsages[fname]++
if _, ok := neededPackages[fname]; !ok {
Expand Down Expand Up @@ -325,7 +362,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
},
)
case isBasicType(valueType, types.Int) && oneOf(verb, "%v", "%d"):
case isBasicType(valueType, types.Int) && oneOf(verb, "%v", "%d") && n.intFormat.enabled:
fname := pass.Fset.File(call.Pos()).Name()
removedFmtUsages[fname]++
if _, ok := neededPackages[fname]; !ok {
Expand All @@ -347,7 +384,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
},
)
case isBasicType(valueType, types.Int64) && oneOf(verb, "%v", "%d"):
case isBasicType(valueType, types.Int64) && oneOf(verb, "%v", "%d") && n.intFormat.enabled:
fname := pass.Fset.File(call.Pos()).Name()
removedFmtUsages[fname]++
if _, ok := neededPackages[fname]; !ok {
Expand Down Expand Up @@ -377,7 +414,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
)

case isBasicType(valueType, types.Uint8, types.Uint16, types.Uint32, types.Uint) && oneOf(verb, "%v", "%d", "%x") && n.intConv:
case isBasicType(valueType, types.Uint8, types.Uint16, types.Uint32, types.Uint) && oneOf(verb, "%v", "%d", "%x") && n.intFormat.intConv:
base := []byte("), 10")
if verb == "%x" {
base = []byte("), 16")
Expand Down Expand Up @@ -410,7 +447,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
},
)
case isBasicType(valueType, types.Uint64) && oneOf(verb, "%v", "%d", "%x"):
case isBasicType(valueType, types.Uint64) && oneOf(verb, "%v", "%d", "%x") && n.intFormat.enabled:
base := []byte(", 10")
if verb == "%x" {
base = []byte(", 16")
Expand Down Expand Up @@ -443,7 +480,7 @@ func (n *perfSprint) run(pass *analysis.Pass) (interface{}, error) {
},
},
)
case isBasicType(valueType, types.String) && fn == "fmt.Sprintf" && isConcatable(verb):
case isBasicType(valueType, types.String) && fn == "fmt.Sprintf" && isConcatable(verb) && n.strFormat.enabled:
var fix string
if strings.HasSuffix(verb, "%s") {
fix = strconv.Quote(verb[:len(verb)-2]) + "+" + formatNode(pass.Fset, value)
Expand Down
47 changes: 23 additions & 24 deletions analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package analyzer_test

import (
"encoding/hex"
"flag"
"fmt"
"io"
"strconv"
Expand All @@ -14,31 +15,29 @@ import (
func TestAnalyzer(t *testing.T) {
t.Parallel()
a := analyzer.New()
err := a.Flags.Set("err-error", "true")
if err != nil {
t.Fatalf("failed to set err-error flag")
}
analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, "p")
}

func TestAnalyzerNoErrError(t *testing.T) {
t.Parallel()
a := analyzer.New()
err := a.Flags.Set("err-error", "false")
if err != nil {
t.Fatalf("failed to set err-error flag")
}
analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, "nilerror")
}
analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, "default")
a.Flags.VisitAll(func(f *flag.Flag) {
if f.Name != "fiximports" {
catenacyber marked this conversation as resolved.
Show resolved Hide resolved
t.Run(f.Name, func(t *testing.T) {
changedVal := "false"
if f.DefValue == "false" {
changedVal = "true"
} else if f.DefValue != "true" {
t.Fatalf("default value neither false or true")
}
err := a.Flags.Set(f.Name, changedVal)
if err != nil {
t.Fatalf("failed to set err-error flag")
}
analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, f.Name)
err = a.Flags.Set(f.Name, f.DefValue)
if err != nil {
t.Fatalf("failed to set err-error flag")
catenacyber marked this conversation as resolved.
Show resolved Hide resolved
}
})
}
})

func TestAnalyzerNoConv(t *testing.T) {
t.Parallel()
a := analyzer.New()
err := a.Flags.Set("int-conversion", "false")
if err != nil {
t.Fatalf("failed to set int-conversion flag")
}
analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), a, "noconv")
}

func TestReplacements(t *testing.T) {
Expand Down
Loading