catenacyber / perfsprint

Golang linter to use strconv
MIT License
21 stars 2 forks source link

"Fix imports" reports #19

Closed ldez closed 8 months ago

ldez commented 8 months ago

Hello,

The "Fix imports" reports seem offtopic for this linter and they are annoying.

Related to #14

Note: I'm using the current dev version of golangci-lint.

package main

import (
    "fmt"
    "testing"
)

func TestName(t *testing.T) {
    data := "bar"
    s := fmt.Sprintf("foo %s", data)
    println(s)
}
$ ./golangci-lint run --no-config --disable-all -Eperfsprint
main_test.go:10:7: fmt.Sprintf can be replaced with string addition (perfsprint)
        s := fmt.Sprintf("foo %s", data)
             ^
main_test.go:4:2: Fix imports (perfsprint)
        "fmt"
        ^

In this context, your linter has detected that fmt is only related to fmt.Sprintf, so you are suggesting to remove the import. I understand why (because you want to "autofix" the problem), but the import management is not something that your linter should handle (at least inside golangci-lint) because other linters will handle that.

Another problem, if I want to ignore some reports then the reports Fix imports will always happen.

linters:
  disable-all: true
  enable:
    - perfsprint

issues:
  exclude:
    - 'fmt.Sprintf can be replaced with string addition'
$ ./golangci-lint run
main_test.go:4:2: Fix imports (perfsprint)
        "fmt"

It's the same thing if I use a nolint directive (without exclude configuration):

package main

import (
    "fmt"
    "testing"
)

func TestName(t *testing.T) {
    data := "bar"
    s := fmt.Sprintf("foo %s", data) //nolint:perfsprint
    println(s)
}
$ ./golangci-lint run --no-config --disable-all -Eperfsprint
main_test.go:4:2: Fix imports (perfsprint)
        "fmt"

So I suggest removing those reports, at least for golangci-lint.

catenacyber commented 8 months ago

Thank you @ldez

I understand why (because you want to "autofix" the problem), but the import management is not something that your linter should handle (at least inside golangci-lint) because other linters will handle that.

I want to keep this feature for perfsprint outside of golangci-lint cf #14

Should I just add a bool flag to configure this behavior ?

ldez commented 8 months ago

Adding a flag is a good way to handle the problem. Another way is to append the suggested fix ([]analysis.SuggestedFix) to the other reports instead of creating a new report.