From a1ae2692e18e3bc38ea4e8e7a66167b34dec598b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Thu, 14 Aug 2025 16:01:06 +0200 Subject: [PATCH] chore: modernize Go code --- caddy/admin_test.go | 8 ++++---- caddy/app.go | 2 +- caddy/caddy_test.go | 28 ++++++++++++++-------------- caddy/module.go | 9 +++------ context.go | 4 ++-- frankenphp_test.go | 13 +++++-------- internal/extgen/classparser.go | 4 ++-- internal/extgen/docs_test.go | 3 +-- internal/extgen/funcparser.go | 4 ++-- internal/extgen/srcanalyzer_test.go | 6 ++---- internal/extgen/utils_test.go | 2 +- internal/extgen/validator.go | 15 +++------------ internal/watcher/watch_pattern.go | 2 +- phpmainthread_test.go | 6 +++--- recorder_test.go | 2 +- state_test.go | 4 +--- watcher_test.go | 2 +- 17 files changed, 47 insertions(+), 67 deletions(-) diff --git a/caddy/admin_test.go b/caddy/admin_test.go index 1318a727..15060aaf 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -112,9 +112,9 @@ func TestAutoScaleWorkerThreads(t *testing.T) { amountOfThreads := len(getDebugState(t, tester).ThreadDebugStates) // try to spawn the additional threads by spamming the server - for tries := 0; tries < maxTries; tries++ { + for range maxTries { wg.Add(requestsPerTry) - for i := 0; i < requestsPerTry; i++ { + for range requestsPerTry { go func() { tester.AssertGetResponse(endpoint, http.StatusOK, "slept for 2 ms and worked for 1000 iterations") wg.Done() @@ -164,9 +164,9 @@ func TestAutoScaleRegularThreadsOnAutomaticThreadLimit(t *testing.T) { amountOfThreads := len(getDebugState(t, tester).ThreadDebugStates) // try to spawn the additional threads by spamming the server - for tries := 0; tries < maxTries; tries++ { + for range maxTries { wg.Add(requestsPerTry) - for i := 0; i < requestsPerTry; i++ { + for range requestsPerTry { go func() { tester.AssertGetResponse(endpoint, http.StatusOK, "slept for 2 ms and worked for 1000 iterations") wg.Done() diff --git a/caddy/app.go b/caddy/app.go index 9e02c0bf..01831c53 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -267,7 +267,7 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } -func parseGlobalOption(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) { +func parseGlobalOption(d *caddyfile.Dispenser, _ any) (any, error) { app := &FrankenPHPApp{} if err := app.UnmarshalCaddyfile(d); err != nil { return nil, err diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 47854c35..29678b8e 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -42,7 +42,7 @@ func TestPHP(t *testing.T) { } `, "caddyfile") - for i := 0; i < 100; i++ { + for i := range 100 { wg.Add(1) go func(i int) { @@ -105,7 +105,7 @@ func TestWorker(t *testing.T) { } `, "caddyfile") - for i := 0; i < 100; i++ { + for i := range 100 { wg.Add(1) go func(i int) { @@ -157,7 +157,7 @@ func TestGlobalAndModuleWorker(t *testing.T) { } `, "caddyfile") - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { @@ -209,7 +209,7 @@ func TestNamedModuleWorkers(t *testing.T) { } `, "caddyfile") - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { @@ -456,7 +456,7 @@ func TestMetrics(t *testing.T) { `, "caddyfile") // Make some requests - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) @@ -529,7 +529,7 @@ func TestWorkerMetrics(t *testing.T) { workerName, _ := fastabs.FastAbs("../testdata/index.php") // Make some requests - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) @@ -621,7 +621,7 @@ func TestNamedWorkerMetrics(t *testing.T) { `, "caddyfile") // Make some requests - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) @@ -712,7 +712,7 @@ func TestAutoWorkerConfig(t *testing.T) { workerName, _ := fastabs.FastAbs("../testdata/index.php") // Make some requests - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) @@ -872,7 +872,7 @@ func TestPHPIniBlockConfiguration(t *testing.T) { func testSingleIniConfiguration(tester *caddytest.Tester, key string, value string) { // test twice to ensure the ini setting is not lost - for i := 0; i < 2; i++ { + for range 2 { tester.AssertGetResponse( "http://localhost:"+testPort+"/ini.php?key="+key, http.StatusOK, @@ -940,7 +940,7 @@ func TestMaxWaitTime(t *testing.T) { wg := sync.WaitGroup{} success := atomic.Bool{} wg.Add(10) - for i := 0; i < 10; i++ { + for range 10 { go func() { statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10", t) if statusCode == http.StatusGatewayTimeout { @@ -987,7 +987,7 @@ func TestMaxWaitTimeWorker(t *testing.T) { wg := sync.WaitGroup{} success := atomic.Bool{} wg.Add(10) - for i := 0; i < 10; i++ { + for range 10 { go func() { statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10000&iteration=1", t) if statusCode == http.StatusGatewayTimeout { @@ -1074,7 +1074,7 @@ func TestMultiWorkersMetrics(t *testing.T) { `, "caddyfile") // Make some requests - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) @@ -1180,7 +1180,7 @@ func TestDisabledMetrics(t *testing.T) { `, "caddyfile") // Make some requests - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) @@ -1285,7 +1285,7 @@ func TestWorkerRestart(t *testing.T) { )) // Make some requests - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(i int) { tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/worker-restart.php?i=%d", i), http.StatusOK, fmt.Sprintf("Counter (%d)", i)) diff --git a/caddy/module.go b/caddy/module.go index 50056ead..dfd78a0a 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -6,6 +6,7 @@ import ( "log/slog" "net/http" "path/filepath" + "slices" "strconv" "strings" @@ -450,12 +451,8 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) tryPolicy = "" } - for _, tf := range tryFiles { - if tf == dirIndex { - dirRedir = true - - break - } + if slices.Contains(tryFiles, dirIndex) { + dirRedir = true } } diff --git a/context.go b/context.go index 6de140da..81f7593a 100644 --- a/context.go +++ b/context.go @@ -29,7 +29,7 @@ type frankenPHPContext struct { responseWriter http.ResponseWriter - done chan interface{} + done chan any startedAt time.Time } @@ -42,7 +42,7 @@ func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) { // NewRequestWithContext creates a new FrankenPHP request context. func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) { fc := &frankenPHPContext{ - done: make(chan interface{}), + done: make(chan any), startedAt: time.Now(), request: r, } diff --git a/frankenphp_test.go b/frankenphp_test.go index 4c36e0c3..7b4b44db 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -522,7 +522,7 @@ func testFlush(t *testing.T, opts *testOptions) { if j == 0 { assert.Equal(t, []byte("He"), buf) } else { - assert.Equal(t, []byte(fmt.Sprintf("llo %d", i)), buf) + assert.Equal(t, fmt.Appendf(nil, "llo %d", i), buf) } j++ @@ -663,7 +663,7 @@ func TestEnvIsNotResetInWorkerMode(t *testing.T) { // reproduction of https://github.com/php/frankenphp/issues/1061 func TestModificationsToEnvPersistAcrossRequests(t *testing.T) { runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { - for j := 0; j < 3; j++ { + for range 3 { result, _ := testGet("http://example.com/env/overwrite-env.php", handler, t) assert.Equal(t, "custom_value", result, "a var directly added to $_ENV should persist") } @@ -780,8 +780,7 @@ func BenchmarkHelloWorld(b *testing.B) { req := httptest.NewRequest("GET", "http://example.com/index.php", nil) w := httptest.NewRecorder() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { handler(w, req) } } @@ -846,8 +845,7 @@ func BenchmarkEcho(b *testing.B) { req := httptest.NewRequest("POST", "http://example.com/echo.php", r) w := httptest.NewRecorder() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { r.Reset(body) handler(w, req) } @@ -917,8 +915,7 @@ func BenchmarkServerSuperGlobal(b *testing.B) { req := httptest.NewRequest("GET", "http://example.com/server-variable.php", nil) w := httptest.NewRecorder() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { handler(w, req) } } diff --git a/internal/extgen/classparser.go b/internal/extgen/classparser.go index b3bb60b3..273df6a1 100644 --- a/internal/extgen/classparser.go +++ b/internal/extgen/classparser.go @@ -306,8 +306,8 @@ func (cp *classParser) parseMethodSignature(className, signature string) (*phpCl var params []phpParameter if paramsStr != "" { - paramParts := strings.Split(paramsStr, ",") - for _, part := range paramParts { + paramParts := strings.SplitSeq(paramsStr, ",") + for part := range paramParts { param, err := cp.parseMethodParameter(strings.TrimSpace(part)) if err != nil { return nil, fmt.Errorf("parsing parameter '%s': %w", part, err) diff --git a/internal/extgen/docs_test.go b/internal/extgen/docs_test.go index 07ac0c22..da7479ff 100644 --- a/internal/extgen/docs_test.go +++ b/internal/extgen/docs_test.go @@ -378,8 +378,7 @@ func BenchmarkDocumentationGenerator_GenerateMarkdown(b *testing.B) { generator: generator, } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, err := docGen.generateMarkdown() assert.NoError(b, err) } diff --git a/internal/extgen/funcparser.go b/internal/extgen/funcparser.go index e3e74208..b2c9f3ec 100644 --- a/internal/extgen/funcparser.go +++ b/internal/extgen/funcparser.go @@ -128,8 +128,8 @@ func (fp *FuncParser) parseSignature(signature string) (*phpFunction, error) { var params []phpParameter if paramsStr != "" { - paramParts := strings.Split(paramsStr, ",") - for _, part := range paramParts { + paramParts := strings.SplitSeq(paramsStr, ",") + for part := range paramParts { param, err := fp.parseParameter(strings.TrimSpace(part)) if err != nil { return nil, fmt.Errorf("parsing parameter '%s': %w", part, err) diff --git a/internal/extgen/srcanalyzer_test.go b/internal/extgen/srcanalyzer_test.go index fc649c04..3efdd73d 100644 --- a/internal/extgen/srcanalyzer_test.go +++ b/internal/extgen/srcanalyzer_test.go @@ -371,8 +371,7 @@ func internalTwo() { analyzer := &SourceAnalyzer{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _, err := analyzer.analyze(filename) require.NoError(b, err) } @@ -391,8 +390,7 @@ func test3() { analyzer := &SourceAnalyzer{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { analyzer.extractInternalFunctions(content) } } diff --git a/internal/extgen/utils_test.go b/internal/extgen/utils_test.go index db77b9de..e379b245 100644 --- a/internal/extgen/utils_test.go +++ b/internal/extgen/utils_test.go @@ -234,7 +234,7 @@ func BenchmarkSanitizePackageName(b *testing.B) { for _, tc := range testCases { b.Run(tc, func(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { SanitizePackageName(tc) } }) diff --git a/internal/extgen/validator.go b/internal/extgen/validator.go index 04f97086..777dd0d6 100644 --- a/internal/extgen/validator.go +++ b/internal/extgen/validator.go @@ -6,6 +6,7 @@ import ( "go/parser" "go/token" "regexp" + "slices" "strings" ) @@ -115,12 +116,7 @@ func (v *Validator) validateClassProperty(prop phpClassProperty) error { } func (v *Validator) isValidPHPType(phpType phpType, validTypes []phpType) bool { - for _, valid := range validTypes { - if phpType == valid { - return true - } - } - return false + return slices.Contains(validTypes, phpType) } // validateScalarTypes checks if PHP signature contains only supported scalar types @@ -141,12 +137,7 @@ func (v *Validator) validateScalarTypes(fn phpFunction) error { } func (v *Validator) isScalarPHPType(phpType phpType, supportedTypes []phpType) bool { - for _, supported := range supportedTypes { - if phpType == supported { - return true - } - } - return false + return slices.Contains(supportedTypes, phpType) } // validateGoFunctionSignatureWithOptions validates with option for method vs function diff --git a/internal/watcher/watch_pattern.go b/internal/watcher/watch_pattern.go index 62162af8..5d9f2b63 100644 --- a/internal/watcher/watch_pattern.go +++ b/internal/watcher/watch_pattern.go @@ -150,7 +150,7 @@ func matchBracketPattern(pattern string, fileName string) bool { // all bracket entries are checked individually, only one needs to match // *.{php,twig,yaml} -> *.php, *.twig, *.yaml - for _, pattern := range strings.Split(betweenTheBrackets, ",") { + for pattern := range strings.SplitSeq(betweenTheBrackets, ",") { if matchPattern(beforeTheBrackets+pattern+afterTheBrackets, fileName) { return true } diff --git a/phpmainthread_test.go b/phpmainthread_test.go index db348b54..18012a41 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -111,7 +111,7 @@ func TestTransitionThreadsWhileDoingRequests(t *testing.T) { // try all possible permutations of transition, transition every ms transitions := allPossibleTransitions(worker1Path, worker2Path) - for i := 0; i < numThreads; i++ { + for i := range numThreads { go func(thread *phpThread, start int) { for { for j := start; j < len(transitions); j++ { @@ -128,9 +128,9 @@ func TestTransitionThreadsWhileDoingRequests(t *testing.T) { // randomly do requests to the 3 endpoints wg.Add(numThreads) - for i := 0; i < numThreads; i++ { + for i := range numThreads { go func(i int) { - for j := 0; j < numRequestsPerThread; j++ { + for range numRequestsPerThread { switch rand.IntN(3) { case 0: assertRequestBody(t, "http://localhost/transition-worker-1.php", "Hello from worker 1") diff --git a/recorder_test.go b/recorder_test.go index f428ef91..dffe46f3 100644 --- a/recorder_test.go +++ b/recorder_test.go @@ -227,7 +227,7 @@ func (rw *ResponseRecorder) Result() *http.Response { if trailers, ok := rw.snapHeader["Trailer"]; ok { res.Trailer = make(http.Header, len(trailers)) for _, k := range trailers { - for _, k := range strings.Split(k, ",") { + for k := range strings.SplitSeq(k, ",") { k = http.CanonicalHeaderKey(textproto.TrimString(k)) if !httpguts.ValidTrailerHeader(k) { // Ignore since forbidden by RFC 7230, section 4.1.2. diff --git a/state_test.go b/state_test.go index 29a10c34..7055d35f 100644 --- a/state_test.go +++ b/state_test.go @@ -39,9 +39,7 @@ func TestStateShouldHaveCorrectAmountOfSubscribers(t *testing.T) { } func assertNumberOfSubscribers(t *testing.T, threadState *threadState, expected int) { - maxWaits := 10_000 // wait for 1 second max - - for i := 0; i < maxWaits; i++ { + for range 10_000 { // wait for 1 second max time.Sleep(100 * time.Microsecond) threadState.mu.RLock() if len(threadState.subscribers) == expected { diff --git a/watcher_test.go b/watcher_test.go index ea5134a9..f3b2efc6 100644 --- a/watcher_test.go +++ b/watcher_test.go @@ -46,7 +46,7 @@ func pollForWorkerReset(t *testing.T, handler func(http.ResponseWriter, *http.Re assert.Equal(t, "requests:1", body) // now we spam file updates and check if the request counter resets - for i := 0; i < limit; i++ { + for range limit { updateTestFile("./testdata/files/test.txt", "updated", t) time.Sleep(pollingTime * time.Millisecond) body, _ := testGet("http://example.com/worker-with-counter.php", handler, t)