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

Use ReplaceAll #872

Closed
breml opened this issue Sep 10, 2019 · 6 comments
Closed

Use ReplaceAll #872

breml opened this issue Sep 10, 2019 · 6 comments

Comments

@breml
Copy link
Contributor

breml commented Sep 10, 2019

Since Go version 1.12, strings.ReplaceAll and bytes.ReplaceAll are available. These should be preferred over their predecessors Replace (with n set to -1), because they make the code more readable.

Before:

x := strings.Replace(x, "foo", "bar", -1)
y := bytes.Replace(y, []byte("foo"), []byte("bar"), -1)

After:

x := strings.ReplaceAll(x, "foo", "bar")
y := bytes.ReplaceAll(y, []byte("foo"), []byte("bar"))
@cristaloleg
Copy link
Member

Looks like a subcase of wrapperFunc checker

/*! use strings.ReplaceAll method in `strings.Replace(s, "a", "b", -1)` */

@quasilyte
Copy link
Member

wrapperFunc does that, but unfortunately, it suggests replace all forms even for Go 1.12-.
This is because we have no "target Go version" options right now.

@breml
Copy link
Contributor Author

breml commented Sep 11, 2019

If I get this correctly, this is already included in go-critic, the only thing that could be improved is, the suggestion should only happen, if a version of Go >= 1.12 is used.
But to add version depended suggestions would need a major change, wouldn't it?

@quasilyte
Copy link
Member

But to add version depended suggestions would need a major change, wouldn't it?

I think it's not that hard to add.
But it should be a separate issue probably. As for ReplaceAll, since most users have now go1.12+, the priority is lower than it was before.

@tehsphinx
Copy link

It would be really nice if the go version in go.mod would be used (if present).

@quasilyte
Copy link
Member

We're using the model that most Go linters use right now: target Go version is specified using the -go parameter.
This parameter is also available (or will be available) via golangci-lint.

ScArFaCe2020 pushed a commit to Euno/rosetta-euno that referenced this issue Mar 14, 2022
functional replacement for https://golang.org/pkg/strings/#Replace

to https://golang.org/pkg/strings/#ReplaceAll

since 1.31 golangci-lint gocritic was updated to include replace to replaceall hint.

ReplaceAll was added in Go 1.12

See go-critic/go-critic#872 for more.
blankdots added a commit to CSCfi/sda-filesystem that referenced this issue Nov 30, 2022
- deprecated rReplace (with n set to -1) go-critic/go-critic#872
blankdots added a commit to CSCfi/sda-filesystem that referenced this issue Nov 30, 2022
- deprecated rReplace (with n set to -1) go-critic/go-critic#872
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

No branches or pull requests

4 participants