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

bytes, strings: add ContainsFunc #54386

Closed
itchyny opened this issue Aug 11, 2022 · 5 comments
Closed

bytes, strings: add ContainsFunc #54386

itchyny opened this issue Aug 11, 2022 · 5 comments

Comments

@itchyny
Copy link
Contributor

itchyny commented Aug 11, 2022

I propose to add ContainsFunc to package strings and bytes.

// ContainsFunc reports whether any Unicode code points c within s satisfy f(c).
func ContainsFunc(s string, f func(rune) bool) bool {
	return IndexFunc(s, f) >= 0
}

The {bytes,strings}.ContainsFunc functions replace codes which only check whether the string contains a Unicode code point satisfying the condition or not using {bytes,strings}.IndexFunc.

In the Go repository, there are 6 codes (out of 9 using $PKG.IndexFunc(...)) to be replaced by the proposed ContainsFunc functions.

 $ semgrep --quiet --lang=go --pattern='$PKG.IndexFunc(...) >= 0'

Findings:

  src/archive/tar/reader.go
        395if bytes.IndexFunc(tr.blk[:], notASCII) >= 0 {


  src/testing/benchmark.go
        353if strings.IndexFunc(unit, unicode.IsSpace) >= 0 {

 $ semgrep --quiet --lang=go --pattern='$PKG.IndexFunc(...) < 0'

Findings:

  src/mime/grammar.go
         31return strings.IndexFunc(s, isNotTokenChar) < 0


  src/net/http/cookie.go
        465return strings.IndexFunc(raw, isNotToken) < 0


  src/text/template/funcs.go
        725if strings.IndexFunc(s, jsIsSpecial) < 0 {

 $ semgrep --quiet --lang=go --pattern='$PKG.IndexFunc(...) == -1'

Findings:

  src/net/http/request.go
        831return len(method) > 0 && strings.IndexFunc(method, isNotToken) == -1

In the Kubernetes repository, I find 2 codes (out of 3 using $PKG.IndexFunc(...)).

 $ semgrep --quiet --lang=go --pattern='$PKG.IndexFunc(...) != -1'

Findings:

  staging/src/k8s.io/client-go/rest/request.go
       1068if bytes.IndexFunc(body, func(r rune) bool {
       1069return r < 0x0a
       1070┆ }) != -1 {


  staging/src/k8s.io/kubectl/pkg/describe/describe.go
        349if strings.IndexFunc(field, func(r rune) bool {
        350return !unicode.IsLetter(r) && r != '-'
        351┆ }) != -1 {

Using ContainsFunc in these if statements will definitely improve the code readability. Also, I believe API symmetry reduces the frustration of coding (I cannot explain why there are Contains, ContainsAny, ContainsRune but not ContainsFunc to Go newbies). Currently, we need to translate what we actually want to check into comparison of returned index against numbers (which may lead to pointless discussion of which is better >= 0 or != -1).

@gopherbot gopherbot added this to the Proposal milestone Aug 11, 2022
@earthboundkid
Copy link
Contributor

Cf #53983

@rsc
Copy link
Contributor

rsc commented Nov 30, 2022

This was waiting on slices.ContainsFunc. Now that we've accepted that proposal, it seems fine to also have bytes.ContainsFunc and strings.ContainsFunc.

@rsc
Copy link
Contributor

rsc commented Nov 30, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Dec 7, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: bytes, strings: add ContainsFunc bytes, strings: add ContainsFunc Dec 7, 2022
@rsc rsc modified the milestones: Proposal, Backlog Dec 7, 2022
@gopherbot
Copy link

Change https://go.dev/cl/460216 mentions this issue: bytes, strings: add ContainsFunc

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

No branches or pull requests

5 participants