diff --git a/.golangci.yaml b/.golangci.yaml index dd01c2f6f0..5a72cd2d52 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -12,6 +12,7 @@ linters: - intrange - mirror - misspell + - modernize - perfsprint - prealloc - revive # replacement for golint @@ -60,6 +61,11 @@ linters: - nilness lll: line-length: 200 + modernize: + disable: + # this should be enabled only outside of hot paths, as it's likely + # less performant (and there have been reports of that) + - slicesbackward perfsprint: # only rule disabled by default, but it's a good one err-error: true diff --git a/internal/providers/aws/signing_v4.go b/internal/providers/aws/signing_v4.go index c463ccbff8..cb3f57d77e 100644 --- a/internal/providers/aws/signing_v4.go +++ b/internal/providers/aws/signing_v4.go @@ -151,23 +151,31 @@ func SignV4(headers map[string][]string, method string, theURL *url.URL, body [] // the "canonical request" is the normalized version of the AWS service access // that we're attempting to perform - canonicalReq := method + "\n" // HTTP method - canonicalReq += theURL.EscapedPath() + "\n" // URI-escaped path - canonicalReq += theURL.RawQuery + "\n" // RAW Query String + buf := bytes.NewBufferString(method) + buf.WriteByte('\n') + buf.WriteString(theURL.EscapedPath()) + buf.WriteByte('\n') + buf.WriteString(theURL.RawQuery) + buf.WriteByte('\n') // include the values for the signed headers orderedKeys := util.KeysSorted(headersToSign) for _, k := range orderedKeys { - // TODO: fix later - //nolint:perfsprint - canonicalReq += k + ":" + strings.Join(headersToSign[k], ",") + "\n" + buf.WriteString(k) + buf.WriteByte(':') + buf.WriteString(strings.Join(headersToSign[k], ",")) + buf.WriteByte('\n') } - canonicalReq += "\n" // linefeed to terminate headers + + buf.WriteByte('\n') // linefeed to terminate headers // include the list of the signed headers headerList := strings.Join(orderedKeys, ";") - canonicalReq += headerList + "\n" - canonicalReq += contentSha256 + buf.WriteString(headerList) + buf.WriteByte('\n') + buf.WriteString(contentSha256) + + canonicalReq := buf.String() // the "string to sign" is a time-bounded, scoped request token which // is linked to the "canonical request" by inclusion of its SHA-256 hash diff --git a/util/test/ci_skip.go b/util/test/ci_skip.go index 3380c15925..d5bf46d951 100644 --- a/util/test/ci_skip.go +++ b/util/test/ci_skip.go @@ -1,5 +1,4 @@ //go:build !darwin -// +build !darwin package test diff --git a/v1/ast/parser_bench_test.go b/v1/ast/parser_bench_test.go index ad7ccba85e..87d0a75633 100644 --- a/v1/ast/parser_bench_test.go +++ b/v1/ast/parser_bench_test.go @@ -5,6 +5,7 @@ package ast import ( + "bytes" "fmt" "strconv" "strings" @@ -218,12 +219,18 @@ func runParseStatementBenchmarkWithError(b *testing.B, stmt string) { } func generateModule(numRules int) string { - mod := "package bench\n" + mod := bytes.NewBufferString("package bench\n") for i := range numRules { - //nolint:perfsprint - mod += fmt.Sprintf("p%d if { input.x%d = %d }\n", i, i, i) + is := strconv.Itoa(i) + mod.WriteByte('p') + mod.WriteString(is) + mod.WriteString(" if { input.x") + mod.WriteString(is) + mod.WriteString(" = ") + mod.WriteString(is) + mod.WriteString(" }\n") } - return mod + return mod.String() } func generateArrayStatement(size int) string { diff --git a/v1/ast/parser_test.go b/v1/ast/parser_test.go index eb5bbd3a85..a31a8d9b40 100644 --- a/v1/ast/parser_test.go +++ b/v1/ast/parser_test.go @@ -5626,12 +5626,12 @@ func TestRuleFromBody(t *testing.T) { } // Verify the rule and rule and rule head col/loc values - testModule := "package a.b.c\n\n" + testModule := bytes.NewBufferString("package a.b.c\n\n") for _, tc := range tests { - //nolint:perfsprint - testModule += tc.input + "\n" + testModule.WriteString(tc.input) + testModule.WriteByte('\n') } - module, err := ParseModuleWithOpts("test.rego", testModule, popts) + module, err := ParseModuleWithOpts("test.rego", testModule.String(), popts) if err != nil { t.Fatal(err) } diff --git a/v1/download/config_test.go b/v1/download/config_test.go index 85966a5940..6964a66dfd 100644 --- a/v1/download/config_test.go +++ b/v1/download/config_test.go @@ -136,12 +136,12 @@ func TestConfigValidationUpdate(t *testing.T) { expParsedMax := time.Second * time.Duration(expMax) var config Config - if err := json.Unmarshal([]byte(fmt.Sprintf(`{ + if err := json.Unmarshal(fmt.Appendf(nil, `{ "polling": { "min_delay_seconds": %d, "max_delay_seconds": %d } - }`, expMin, expMax)), &config); err != nil { + }`, expMin, expMax), &config); err != nil { t.Fatal(err) } diff --git a/v1/plugins/bundle/plugin_test.go b/v1/plugins/bundle/plugin_test.go index 4e30a65c44..c2474dfedf 100644 --- a/v1/plugins/bundle/plugin_test.go +++ b/v1/plugins/bundle/plugin_test.go @@ -7371,7 +7371,7 @@ func TestPluginManualTriggerWithServerError(t *testing.T) { } } } else { - t.Fatalf("expected type of error to be %s but got %s", reflect.TypeOf(bundleErrors), reflect.TypeOf(err)) + t.Fatalf("expected type of error to be %s but got %s", reflect.TypeFor[Errors](), reflect.TypeOf(err)) } } diff --git a/v1/plugins/rest/rest_test.go b/v1/plugins/rest/rest_test.go index 3deeb1b069..4c1d6cb78e 100644 --- a/v1/plugins/rest/rest_test.go +++ b/v1/plugins/rest/rest_test.go @@ -1849,15 +1849,14 @@ func TestS3SigningMultiCredentialProvider(t *testing.T) { t.Fatalf("Client config S3 signing credentials setup unexpected") } - awsCredentialServiceChain, ok := awsPlugin.awsCredentialService().(*awsCredentialServiceChain) + chain, ok := awsPlugin.awsCredentialService().(*awsCredentialServiceChain) if !ok { - t.Fatalf("Unexpected AWS credential service:%v is not a chain", - reflect.TypeOf(awsCredentialServiceChain)) + t.Fatalf("Unexpected AWS credential service: %T is not a chain", chain) } - if len(awsCredentialServiceChain.awsCredentialServices) != credentialProviderCount { + if len(chain.awsCredentialServices) != credentialProviderCount { t.Fatalf("Credential provider count mismatch %d != %d", credentialProviderCount, - len(awsCredentialServiceChain.awsCredentialServices)) + len(chain.awsCredentialServices)) } expectedOrder := []awsCredentialService{ @@ -1867,8 +1866,7 @@ func TestS3SigningMultiCredentialProvider(t *testing.T) { &awsMetadataCredentialService{}, } - if !reflect.DeepEqual(awsCredentialServiceChain.awsCredentialServices, - expectedOrder) { + if !reflect.DeepEqual(chain.awsCredentialServices, expectedOrder) { t.Fatalf("Ordering is unexpected") } } diff --git a/v1/tester/reporter.go b/v1/tester/reporter.go index db35f04a84..96e4907ba4 100644 --- a/v1/tester/reporter.go +++ b/v1/tester/reporter.go @@ -245,11 +245,11 @@ func (r PrettyReporter) fmtBenchmark(tr *Result) string { // // This converts the test case name like data.foo.bar.test_auth to be more // like BenchmarkDataFooBarTestAuth. - camelCaseName := "" + var camelCaseName strings.Builder for part := range strings.SplitSeq(strings.ReplaceAll(name, "_", "."), ".") { - camelCaseName += strings.Title(part) //nolint:perfsprint,staticcheck + camelCaseName.WriteString(strings.Title(part)) //nolint:staticcheck } - name = "Benchmark" + camelCaseName + name = "Benchmark" + camelCaseName.String() } result := fmt.Sprintf("%s\t%s", name, tr.BenchmarkResult.String()) diff --git a/v1/topdown/json_bench_test.go b/v1/topdown/json_bench_test.go index e3cdda628d..46bc22478d 100644 --- a/v1/topdown/json_bench_test.go +++ b/v1/topdown/json_bench_test.go @@ -339,13 +339,11 @@ func BenchmarkJSONPatchPathologicalNestedAddChainObject(b *testing.B) { for _, n := range []int{10, 100, 500, 1000, 5000, 10000} { b.Run(strconv.Itoa(n), func(b *testing.B) { patchList := make([]*ast.Term, n) - path := "" for i := range n { - path += "/a" patchList[i] = ast.NewTerm(ast.NewObject( [2]*ast.Term{ast.InternedTerm("op"), ast.InternedTerm("add")}, [2]*ast.Term{ast.InternedTerm("value"), ast.ObjectTerm()}, - [2]*ast.Term{ast.InternedTerm("path"), ast.InternedTerm(path)}, + [2]*ast.Term{ast.InternedTerm("path"), ast.InternedTerm(strings.Repeat("/a", i+1))}, )) } runJSONPatchBenchmarkTest(b, ast.NewObject(), ast.NewArray(patchList...)) @@ -357,13 +355,11 @@ func BenchmarkJSONPatchPathologicalNestedAddChainArray(b *testing.B) { for _, n := range []int{10, 100, 500, 1000, 5000, 10000} { b.Run(strconv.Itoa(n), func(b *testing.B) { patchList := make([]*ast.Term, n) - path := "" for i := range n { - path += "/0" patchList[i] = ast.NewTerm(ast.NewObject( [2]*ast.Term{ast.InternedTerm("op"), ast.InternedTerm("add")}, [2]*ast.Term{ast.InternedTerm("value"), ast.ArrayTerm()}, - [2]*ast.Term{ast.InternedTerm("path"), ast.StringTerm(path)}, + [2]*ast.Term{ast.InternedTerm("path"), ast.StringTerm(strings.Repeat("/0", i+1))}, )) } runJSONPatchBenchmarkTest(b, ast.NewArray(), ast.NewArray(patchList...)) diff --git a/v1/topdown/topdown_bench_test.go b/v1/topdown/topdown_bench_test.go index eae98ff6e8..8b82c10fe2 100644 --- a/v1/topdown/topdown_bench_test.go +++ b/v1/topdown/topdown_bench_test.go @@ -192,7 +192,8 @@ func benchmarkConcurrency(b *testing.B, params []storage.TransactionParams) { for range 1000 / len(params) { txn, err := store.NewTransaction(ctx, param) if err != nil { - b.Fatalf("Unexpected transaction error: %v", err) + b.Errorf("Unexpected transaction error: %v", err) + return } rs, err := NewQuery(body). WithCompiler(compiler). @@ -200,10 +201,9 @@ func benchmarkConcurrency(b *testing.B, params []storage.TransactionParams) { WithTransaction(txn). Run(ctx) if err != nil { - b.Fatalf("Unexpected topdown query error: %v", err) - } - if len(rs) != 1 || !rs[0][ast.Var("x")].Equal(ast.BooleanTerm(true)) { - b.Fatalf("Unexpected undefined/extra/bad result: %v", rs) + b.Errorf("Unexpected topdown query error: %v", err) + } else if len(rs) != 1 || !rs[0][ast.Var("x")].Equal(ast.BooleanTerm(true)) { + b.Errorf("Unexpected undefined/extra/bad result: %v", rs) } store.Abort(ctx, txn) } diff --git a/v1/util/test/benchmark.go b/v1/util/test/benchmark.go index 8ca20cf1d6..dec65e489a 100644 --- a/v1/util/test/benchmark.go +++ b/v1/util/test/benchmark.go @@ -67,6 +67,7 @@ func PartialObjectBenchmarkCrossModule(n int) []string { bazMod += fmt.Sprintf(`rule_%d if { %s }`, idx, ruleBuilder) + //nolint:modernize fooMod += fmt.Sprintf(` final_decision = "allow" if { baz.rule_%d diff --git a/v1/util/test/ci_skip.go b/v1/util/test/ci_skip.go index 3380c15925..d5bf46d951 100644 --- a/v1/util/test/ci_skip.go +++ b/v1/util/test/ci_skip.go @@ -1,5 +1,4 @@ //go:build !darwin -// +build !darwin package test