catenacyber / perfsprint

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

Incorrect detection of strings with multiple %[1] #25

Closed percivalalb closed 7 months ago

percivalalb commented 7 months ago

https://github.com/catenacyber/perfsprint/pull/16 fails to consider cases where the one arguments is referenced multiple times:

package main

import "fmt"

func main() {
         _ = fmt.Sprintf("You say %[1]s, I also say %[1]s", "hello")
}

Using the version before the PR mentioned above, there is no change as positional arguments weren't supported.

$ go install github.com/catenacyber/perfsprint@v0.4.0
$ perfsprint --fix ./...

Using a the latest version (i.e. after the PR was merged) it detects and gives an inccorrect suggested "fix":

$ go install github.com/catenacyber/perfsprint@v0.7.0
$ perfsprint --fix ./...
test/main.go:6:7: fmt.Sprintf can be replaced with string concatenation
test/main.go:3:8: Fix imports

The fix is:

package main

import

func main() {
         _ = "You say %[1]s, I also say "+"hello"
}

Given this is not a simple concatenation I would expect this to just not be flagged by perfsprint.

percivalalb commented 7 months ago

I've just noticed this highlights another issue - fmt was the only import and after a syntactically invalid "import" list is left behind

catenacyber commented 7 months ago

Thanks @percivalalb for they report, could you review #26 ?

percivalalb commented 7 months ago

Ty