Use better string splitting techniques where possible (#8226)

## What changed?

This PR aims to avoid usage of
[`strings.Split`](https://pkg.go.dev/strings#Split) where possible in
favor of better string splitting techniques, speficially:
[`strings.SplitN`](https://pkg.go.dev/strings#SplitN) and
[`strings.SplitSeq`](https://pkg.go.dev/strings#SplitSeq) where
appropriate.

There was also a [`strings.Fields`](https://pkg.go.dev/strings#Fields)
change I made to use
[`strings.FieldsSeq`](https://pkg.go.dev/strings#FieldsSeq) instead, and
another for S3 to use the [`path`](https://pkg.go.dev/path) package
instead of [`strings.Split`](https://pkg.go.dev/strings#Split).

## Why?

[`strings.SplitN`](https://pkg.go.dev/strings#SplitN) and
[`strings.SplitSeq`](https://pkg.go.dev/strings#SplitSeq) are often
better options in many cases, and can be _partially_ detected using
[`modernize`](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize):
> `stringsseq`: replace Split in "for range strings.Split(...)" by
go1.24's more efficient `SplitSeq`, or `Fields` with `FieldSeq`.

## How did you test it?
- [X] built
- [X] run locally and tested manually
- [x] covered by existing tests
- [x] added new unit test(s)
- [ ] added new functional test(s)

## Potential risks

There are lots of potentially subtle behaviors from the `strings.Split`
(and `strings.Fields`) usage that should be accounted for. If our
existing tests don't cover those subtleties, there's risk for
introducing an unintended bug. More intricate handling/parsing
previously using the `strings` package should get extra attention from
reviewers. I've attempted to break up my changes into logical commit
chunks to aid in review / help spot potentially concerning changes.

Author: picatz

cmd/tools/codegen/helpers.go

@@ -81,17 +81,14 @@ func CamelCaseToSnakeCase(s string) string {
 }
 
 func SnakeCaseToPascalCase(s string) string {
-	// Split the string by underscores
-	words := strings.Split(s, "_")
-
-	// Capitalize the first letter of each word
-	for i, word := range words {
+	var b strings.Builder
+	// Capitalize the first letter of each word split by underscore
+	for word := range strings.SplitSeq(s, "_") {
 		// Convert first rune to upper and the rest to lower case
-		words[i] = cases.Title(language.AmericanEnglish).String(strings.ToLower(word))
+		b.WriteString(cases.Title(language.AmericanEnglish).String(strings.ToLower(word)))
 	}
-
 	// Join them back into a single string
-	return strings.Join(words, "")
+	return b.String()
 }
 
 func IsASCIIUpper(c rune) bool {

cmd/tools/codegen/helpers_test.go

@@ -0,0 +1,33 @@
+package codegen
+
+import "testing"
+
+func TestSnakeCaseToPascalCase(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{name: "empty", in: "", want: ""},
+		{name: "single_lower", in: "a", want: "A"},
+		{name: "single_upper", in: "A", want: "A"},
+		{name: "simple", in: "hello", want: "Hello"},
+		{name: "two_words", in: "hello_world", want: "HelloWorld"},
+		{name: "all_caps", in: "HELLO_WORLD", want: "HelloWorld"},
+		{name: "leading_underscore", in: "_leading", want: "Leading"},
+		{name: "trailing_underscore", in: "trailing_", want: "Trailing"},
+		{name: "double_underscore", in: "a__b", want: "AB"},
+		{name: "only_underscores", in: "__", want: ""},
+		{name: "common_id", in: "user_id", want: "UserId"},
+		{name: "with_digits", in: "http_server_v2", want: "HttpServerV2"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := SnakeCaseToPascalCase(tt.in)
+			if got != tt.want {
+				t.Fatalf("SnakeCaseToPascalCase(%q) = %q, want %q", tt.in, got, tt.want)
+			}
+		})
+	}
+}

cmd/tools/getproto/main.go

@@ -41,7 +41,7 @@ func findProtoImports() []string {
 		if d.Type().IsRegular() && strings.HasSuffix(path, ".proto") {
 			protoFile, err := os.ReadFile(path)
 			fatalIfErr(err)
-			for _, line := range strings.Split(string(protoFile), "\n") {
+			for line := range strings.SplitSeq(string(protoFile), "\n") {
 				if match := matchImport.FindStringSubmatch(line); len(match) > 0 {
 					i := match[1]
 					if strings.HasPrefix(i, "temporal/api/") ||

common/archiver/gcloud/util.go

@@ -152,7 +152,7 @@ func newRunIDPrecondition(runID string) connector.Precondition {
 		}
 
 		if strings.Contains(fileName, runID) {
-			fileNameParts := strings.Split(fileName, "_")
+			fileNameParts := strings.SplitN(fileName, "_", 5)
 			if len(fileNameParts) != 5 {
 				return true
 			}
@@ -176,7 +176,7 @@ func newWorkflowIDPrecondition(workflowID string) connector.Precondition {
 		}
 
 		if strings.Contains(fileName, workflowID) {
-			fileNameParts := strings.Split(fileName, "_")
+			fileNameParts := strings.SplitN(fileName, "_", 5)
 			if len(fileNameParts) != 5 {
 				return true
 			}
@@ -200,7 +200,7 @@ func newWorkflowTypeNamePrecondition(workflowTypeName string) connector.Precondi
 		}
 
 		if strings.Contains(fileName, workflowTypeName) {
-			fileNameParts := strings.Split(fileName, "_")
+			fileNameParts := strings.SplitN(fileName, "_", 5)
 			if len(fileNameParts) != 5 {
 				return true
 			}

common/archiver/s3store/visibility_archiver.go

@@ -2,6 +2,7 @@ package s3store
 
 import (
 	"context"
+	"path"
 	"strings"
 	"time"
 
@@ -223,11 +224,17 @@ func (v *visibilityArchiver) queryAll(
 			nextPageToken: nextPageToken,
 			parsedQuery:   &parsedQuery{},
 		}, saTypeMap, searchPrefix, func(key string) bool {
-			// We only want to return entries for the closeTimeout secondary index, which will always be of the form:
-			// .../closeTimeout/<closeTimeout>/<runID>, so we split the key on "/" and check that the third-to-last
-			// element is "closeTimeout".
-			elements := strings.Split(key, "/")
-			return len(elements) >= 3 && elements[len(elements)-3] == secondaryIndexKeyCloseTimeout
+			// We only want to return entries for the closeTimeout secondary index. Keys for this
+			// index are always of the form:
+			//   .../closeTimeout/<timestamp>/<runID>
+			// Walk from the end instead of splitting the whole string to avoid unnecessary
+			// allocations and to keep the logic clear:
+			//   - drop <runID>
+			//   - drop <timestamp>
+			//   - check the remaining last segment equals "closeTimeout".
+			dir := path.Dir(key) // drop runID
+			dir = path.Dir(dir)  // drop <timestamp>
+			return path.Base(dir) == secondaryIndexKeyCloseTimeout
 		})
 		if err != nil {
 			return nil, err

common/authorization/default_jwt_claim_mapper.go

@@ -125,7 +125,7 @@ func (a *defaultJWTClaimMapper) extractPermissions(permissions []interface{}, cl
 			}
 			parts = []string{match[a.matchNamespaceIndex], match[a.matchRoleIndex]}
 		} else {
-			parts = strings.Split(p, ":")
+			parts = strings.SplitN(p, ":", 2)
 			if len(parts) != 2 {
 				a.logger.Warn(fmt.Sprintf("ignoring permission in unexpected format: %v", permission))
 				continue

common/headers/version_checker.go

@@ -2,7 +2,6 @@ package headers
 
 import (
 	"context"
-	"slices"
 	"strings"
 
 	"github.com/blang/semver/v4"
@@ -152,8 +151,15 @@ func (vc *versionChecker) ClientSupported(ctx context.Context) error {
 // given feature (which should be one of the Feature... constants above).
 func (vc *versionChecker) ClientSupportsFeature(ctx context.Context, feature string) bool {
 	headers := GetValues(ctx, SupportedFeaturesHeaderName)
-	clientFeatures := strings.Split(headers[0], SupportedFeaturesHeaderDelim)
-	return slices.Contains(clientFeatures, feature)
+	if len(headers) == 0 {
+		return false
+	}
+	for clientFeature := range strings.SplitSeq(headers[0], SupportedFeaturesHeaderDelim) {
+		if clientFeature == feature {
+			return true
+		}
+	}
+	return false
 }
 
 func mustParseRanges(ranges map[string]string) map[string]semver.Range {

common/persistence/data_interfaces.go

@@ -1385,15 +1385,16 @@ func BuildHistoryGarbageCleanupInfo(namespaceID, workflowID, runID string) strin
 
 // SplitHistoryGarbageCleanupInfo returns workflow identity information
 func SplitHistoryGarbageCleanupInfo(info string) (namespaceID, workflowID, runID string, err error) {
-	ss := strings.Split(info, ":")
-	// workflowID can contain ":" so len(ss) can be greater than 3
-	if len(ss) < numItemsInGarbageInfo {
-		return "", "", "", fmt.Errorf("not able to split info for  %s", info)
-	}
-	namespaceID = ss[0]
-	runID = ss[len(ss)-1]
-	workflowEnd := len(info) - len(runID) - 1
-	workflowID = info[len(namespaceID)+1 : workflowEnd]
+	// Expect format: namespaceID:workflowID:runID, but workflowID may contain ':' so we
+	// take everything between the first and last ':' as workflowID.
+	first := strings.IndexByte(info, ':')
+	last := strings.LastIndexByte(info, ':')
+	if first < 0 || first == last { // need at least two ':' to have 3 parts
+		return "", "", "", fmt.Errorf("not able to split info for %s", info)
+	}
+	namespaceID = info[:first]
+	workflowID = info[first+1 : last]
+	runID = info[last+1:]
 	return
 }
 

common/persistence/history_task_queue_manager.go

@@ -248,7 +248,7 @@ func GetHistoryTaskQueueName(
 }
 
 func GetHistoryTaskQueueCategoryID(queueName string) (int, error) {
-	fields := strings.Split(queueName, "_")
+	fields := strings.SplitN(queueName, "_", 4)
 	if len(fields) != 4 {
 		return 0, fmt.Errorf("%w: %s", ErrInvalidQueueName, queueName)
 	}

common/persistence/nosql/nosqlplugin/cassandra/gocql/client.go

@@ -173,7 +173,7 @@ func ConfigureCassandraCluster(cfg config.Cassandra, cluster *gocql.ClusterConfi
 // parseHosts returns parses a list of hosts separated by comma
 func parseHosts(input string) []string {
 	var hosts []string
-	for _, h := range strings.Split(input, ",") {
+	for h := range strings.SplitSeq(input, ",") {
 		if host := strings.TrimSpace(h); len(host) > 0 {
 			hosts = append(hosts, host)
 		}