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

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

Closed
percivalalb opened this issue Feb 19, 2024 · 3 comments · Fixed by #26
Closed

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

percivalalb opened this issue Feb 19, 2024 · 3 comments · Fixed by #26

Comments

@percivalalb
Copy link

#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
Copy link
Author

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

@catenacyber
Copy link
Owner

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

@percivalalb
Copy link
Author

Ty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants