chore: modernize Go code

This commit is contained in:
Kévin Dunglas
2025-08-14 16:01:06 +02:00
parent 6ad34b1cb3
commit a1ae2692e1
17 changed files with 47 additions and 67 deletions

View File

@@ -112,9 +112,9 @@ func TestAutoScaleWorkerThreads(t *testing.T) {
amountOfThreads := len(getDebugState(t, tester).ThreadDebugStates) amountOfThreads := len(getDebugState(t, tester).ThreadDebugStates)
// try to spawn the additional threads by spamming the server // try to spawn the additional threads by spamming the server
for tries := 0; tries < maxTries; tries++ { for range maxTries {
wg.Add(requestsPerTry) wg.Add(requestsPerTry)
for i := 0; i < requestsPerTry; i++ { for range requestsPerTry {
go func() { go func() {
tester.AssertGetResponse(endpoint, http.StatusOK, "slept for 2 ms and worked for 1000 iterations") tester.AssertGetResponse(endpoint, http.StatusOK, "slept for 2 ms and worked for 1000 iterations")
wg.Done() wg.Done()
@@ -164,9 +164,9 @@ func TestAutoScaleRegularThreadsOnAutomaticThreadLimit(t *testing.T) {
amountOfThreads := len(getDebugState(t, tester).ThreadDebugStates) amountOfThreads := len(getDebugState(t, tester).ThreadDebugStates)
// try to spawn the additional threads by spamming the server // try to spawn the additional threads by spamming the server
for tries := 0; tries < maxTries; tries++ { for range maxTries {
wg.Add(requestsPerTry) wg.Add(requestsPerTry)
for i := 0; i < requestsPerTry; i++ { for range requestsPerTry {
go func() { go func() {
tester.AssertGetResponse(endpoint, http.StatusOK, "slept for 2 ms and worked for 1000 iterations") tester.AssertGetResponse(endpoint, http.StatusOK, "slept for 2 ms and worked for 1000 iterations")
wg.Done() wg.Done()

View File

@@ -267,7 +267,7 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
return nil return nil
} }
func parseGlobalOption(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) { func parseGlobalOption(d *caddyfile.Dispenser, _ any) (any, error) {
app := &FrankenPHPApp{} app := &FrankenPHPApp{}
if err := app.UnmarshalCaddyfile(d); err != nil { if err := app.UnmarshalCaddyfile(d); err != nil {
return nil, err return nil, err

View File

@@ -42,7 +42,7 @@ func TestPHP(t *testing.T) {
} }
`, "caddyfile") `, "caddyfile")
for i := 0; i < 100; i++ { for i := range 100 {
wg.Add(1) wg.Add(1)
go func(i int) { go func(i int) {
@@ -105,7 +105,7 @@ func TestWorker(t *testing.T) {
} }
`, "caddyfile") `, "caddyfile")
for i := 0; i < 100; i++ { for i := range 100 {
wg.Add(1) wg.Add(1)
go func(i int) { go func(i int) {
@@ -157,7 +157,7 @@ func TestGlobalAndModuleWorker(t *testing.T) {
} }
`, "caddyfile") `, "caddyfile")
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { go func(i int) {
@@ -209,7 +209,7 @@ func TestNamedModuleWorkers(t *testing.T) {
} }
`, "caddyfile") `, "caddyfile")
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { go func(i int) {
@@ -456,7 +456,7 @@ func TestMetrics(t *testing.T) {
`, "caddyfile") `, "caddyfile")
// Make some requests // Make some requests
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { 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)) 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") workerName, _ := fastabs.FastAbs("../testdata/index.php")
// Make some requests // Make some requests
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { 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)) 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") `, "caddyfile")
// Make some requests // Make some requests
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { 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)) 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") workerName, _ := fastabs.FastAbs("../testdata/index.php")
// Make some requests // Make some requests
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { 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)) 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) { func testSingleIniConfiguration(tester *caddytest.Tester, key string, value string) {
// test twice to ensure the ini setting is not lost // test twice to ensure the ini setting is not lost
for i := 0; i < 2; i++ { for range 2 {
tester.AssertGetResponse( tester.AssertGetResponse(
"http://localhost:"+testPort+"/ini.php?key="+key, "http://localhost:"+testPort+"/ini.php?key="+key,
http.StatusOK, http.StatusOK,
@@ -940,7 +940,7 @@ func TestMaxWaitTime(t *testing.T) {
wg := sync.WaitGroup{} wg := sync.WaitGroup{}
success := atomic.Bool{} success := atomic.Bool{}
wg.Add(10) wg.Add(10)
for i := 0; i < 10; i++ { for range 10 {
go func() { go func() {
statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10", t) statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10", t)
if statusCode == http.StatusGatewayTimeout { if statusCode == http.StatusGatewayTimeout {
@@ -987,7 +987,7 @@ func TestMaxWaitTimeWorker(t *testing.T) {
wg := sync.WaitGroup{} wg := sync.WaitGroup{}
success := atomic.Bool{} success := atomic.Bool{}
wg.Add(10) wg.Add(10)
for i := 0; i < 10; i++ { for range 10 {
go func() { go func() {
statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10000&iteration=1", t) statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10000&iteration=1", t)
if statusCode == http.StatusGatewayTimeout { if statusCode == http.StatusGatewayTimeout {
@@ -1074,7 +1074,7 @@ func TestMultiWorkersMetrics(t *testing.T) {
`, "caddyfile") `, "caddyfile")
// Make some requests // Make some requests
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { 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)) 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") `, "caddyfile")
// Make some requests // Make some requests
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { 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)) 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 // Make some requests
for i := 0; i < 10; i++ { for i := range 10 {
wg.Add(1) wg.Add(1)
go func(i int) { go func(i int) {
tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/worker-restart.php?i=%d", i), http.StatusOK, fmt.Sprintf("Counter (%d)", i)) tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/worker-restart.php?i=%d", i), http.StatusOK, fmt.Sprintf("Counter (%d)", i))

View File

@@ -6,6 +6,7 @@ import (
"log/slog" "log/slog"
"net/http" "net/http"
"path/filepath" "path/filepath"
"slices"
"strconv" "strconv"
"strings" "strings"
@@ -450,12 +451,8 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
tryPolicy = "" tryPolicy = ""
} }
for _, tf := range tryFiles { if slices.Contains(tryFiles, dirIndex) {
if tf == dirIndex { dirRedir = true
dirRedir = true
break
}
} }
} }

View File

@@ -29,7 +29,7 @@ type frankenPHPContext struct {
responseWriter http.ResponseWriter responseWriter http.ResponseWriter
done chan interface{} done chan any
startedAt time.Time startedAt time.Time
} }
@@ -42,7 +42,7 @@ func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) {
// NewRequestWithContext creates a new FrankenPHP request context. // NewRequestWithContext creates a new FrankenPHP request context.
func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) { func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) {
fc := &frankenPHPContext{ fc := &frankenPHPContext{
done: make(chan interface{}), done: make(chan any),
startedAt: time.Now(), startedAt: time.Now(),
request: r, request: r,
} }

View File

@@ -522,7 +522,7 @@ func testFlush(t *testing.T, opts *testOptions) {
if j == 0 { if j == 0 {
assert.Equal(t, []byte("He"), buf) assert.Equal(t, []byte("He"), buf)
} else { } else {
assert.Equal(t, []byte(fmt.Sprintf("llo %d", i)), buf) assert.Equal(t, fmt.Appendf(nil, "llo %d", i), buf)
} }
j++ j++
@@ -663,7 +663,7 @@ func TestEnvIsNotResetInWorkerMode(t *testing.T) {
// reproduction of https://github.com/php/frankenphp/issues/1061 // reproduction of https://github.com/php/frankenphp/issues/1061
func TestModificationsToEnvPersistAcrossRequests(t *testing.T) { func TestModificationsToEnvPersistAcrossRequests(t *testing.T) {
runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { 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) 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") 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) req := httptest.NewRequest("GET", "http://example.com/index.php", nil)
w := httptest.NewRecorder() w := httptest.NewRecorder()
b.ResetTimer() for b.Loop() {
for i := 0; i < b.N; i++ {
handler(w, req) handler(w, req)
} }
} }
@@ -846,8 +845,7 @@ func BenchmarkEcho(b *testing.B) {
req := httptest.NewRequest("POST", "http://example.com/echo.php", r) req := httptest.NewRequest("POST", "http://example.com/echo.php", r)
w := httptest.NewRecorder() w := httptest.NewRecorder()
b.ResetTimer() for b.Loop() {
for i := 0; i < b.N; i++ {
r.Reset(body) r.Reset(body)
handler(w, req) handler(w, req)
} }
@@ -917,8 +915,7 @@ func BenchmarkServerSuperGlobal(b *testing.B) {
req := httptest.NewRequest("GET", "http://example.com/server-variable.php", nil) req := httptest.NewRequest("GET", "http://example.com/server-variable.php", nil)
w := httptest.NewRecorder() w := httptest.NewRecorder()
b.ResetTimer() for b.Loop() {
for i := 0; i < b.N; i++ {
handler(w, req) handler(w, req)
} }
} }

View File

@@ -306,8 +306,8 @@ func (cp *classParser) parseMethodSignature(className, signature string) (*phpCl
var params []phpParameter var params []phpParameter
if paramsStr != "" { if paramsStr != "" {
paramParts := strings.Split(paramsStr, ",") paramParts := strings.SplitSeq(paramsStr, ",")
for _, part := range paramParts { for part := range paramParts {
param, err := cp.parseMethodParameter(strings.TrimSpace(part)) param, err := cp.parseMethodParameter(strings.TrimSpace(part))
if err != nil { if err != nil {
return nil, fmt.Errorf("parsing parameter '%s': %w", part, err) return nil, fmt.Errorf("parsing parameter '%s': %w", part, err)

View File

@@ -378,8 +378,7 @@ func BenchmarkDocumentationGenerator_GenerateMarkdown(b *testing.B) {
generator: generator, generator: generator,
} }
b.ResetTimer() for b.Loop() {
for i := 0; i < b.N; i++ {
_, err := docGen.generateMarkdown() _, err := docGen.generateMarkdown()
assert.NoError(b, err) assert.NoError(b, err)
} }

View File

@@ -128,8 +128,8 @@ func (fp *FuncParser) parseSignature(signature string) (*phpFunction, error) {
var params []phpParameter var params []phpParameter
if paramsStr != "" { if paramsStr != "" {
paramParts := strings.Split(paramsStr, ",") paramParts := strings.SplitSeq(paramsStr, ",")
for _, part := range paramParts { for part := range paramParts {
param, err := fp.parseParameter(strings.TrimSpace(part)) param, err := fp.parseParameter(strings.TrimSpace(part))
if err != nil { if err != nil {
return nil, fmt.Errorf("parsing parameter '%s': %w", part, err) return nil, fmt.Errorf("parsing parameter '%s': %w", part, err)

View File

@@ -371,8 +371,7 @@ func internalTwo() {
analyzer := &SourceAnalyzer{} analyzer := &SourceAnalyzer{}
b.ResetTimer() for b.Loop() {
for i := 0; i < b.N; i++ {
_, _, err := analyzer.analyze(filename) _, _, err := analyzer.analyze(filename)
require.NoError(b, err) require.NoError(b, err)
} }
@@ -391,8 +390,7 @@ func test3() {
analyzer := &SourceAnalyzer{} analyzer := &SourceAnalyzer{}
b.ResetTimer() for b.Loop() {
for i := 0; i < b.N; i++ {
analyzer.extractInternalFunctions(content) analyzer.extractInternalFunctions(content)
} }
} }

View File

@@ -234,7 +234,7 @@ func BenchmarkSanitizePackageName(b *testing.B) {
for _, tc := range testCases { for _, tc := range testCases {
b.Run(tc, func(b *testing.B) { b.Run(tc, func(b *testing.B) {
for i := 0; i < b.N; i++ { for b.Loop() {
SanitizePackageName(tc) SanitizePackageName(tc)
} }
}) })

View File

@@ -6,6 +6,7 @@ import (
"go/parser" "go/parser"
"go/token" "go/token"
"regexp" "regexp"
"slices"
"strings" "strings"
) )
@@ -115,12 +116,7 @@ func (v *Validator) validateClassProperty(prop phpClassProperty) error {
} }
func (v *Validator) isValidPHPType(phpType phpType, validTypes []phpType) bool { func (v *Validator) isValidPHPType(phpType phpType, validTypes []phpType) bool {
for _, valid := range validTypes { return slices.Contains(validTypes, phpType)
if phpType == valid {
return true
}
}
return false
} }
// validateScalarTypes checks if PHP signature contains only supported scalar types // 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 { func (v *Validator) isScalarPHPType(phpType phpType, supportedTypes []phpType) bool {
for _, supported := range supportedTypes { return slices.Contains(supportedTypes, phpType)
if phpType == supported {
return true
}
}
return false
} }
// validateGoFunctionSignatureWithOptions validates with option for method vs function // validateGoFunctionSignatureWithOptions validates with option for method vs function

View File

@@ -150,7 +150,7 @@ func matchBracketPattern(pattern string, fileName string) bool {
// all bracket entries are checked individually, only one needs to match // all bracket entries are checked individually, only one needs to match
// *.{php,twig,yaml} -> *.php, *.twig, *.yaml // *.{php,twig,yaml} -> *.php, *.twig, *.yaml
for _, pattern := range strings.Split(betweenTheBrackets, ",") { for pattern := range strings.SplitSeq(betweenTheBrackets, ",") {
if matchPattern(beforeTheBrackets+pattern+afterTheBrackets, fileName) { if matchPattern(beforeTheBrackets+pattern+afterTheBrackets, fileName) {
return true return true
} }

View File

@@ -111,7 +111,7 @@ func TestTransitionThreadsWhileDoingRequests(t *testing.T) {
// try all possible permutations of transition, transition every ms // try all possible permutations of transition, transition every ms
transitions := allPossibleTransitions(worker1Path, worker2Path) transitions := allPossibleTransitions(worker1Path, worker2Path)
for i := 0; i < numThreads; i++ { for i := range numThreads {
go func(thread *phpThread, start int) { go func(thread *phpThread, start int) {
for { for {
for j := start; j < len(transitions); j++ { for j := start; j < len(transitions); j++ {
@@ -128,9 +128,9 @@ func TestTransitionThreadsWhileDoingRequests(t *testing.T) {
// randomly do requests to the 3 endpoints // randomly do requests to the 3 endpoints
wg.Add(numThreads) wg.Add(numThreads)
for i := 0; i < numThreads; i++ { for i := range numThreads {
go func(i int) { go func(i int) {
for j := 0; j < numRequestsPerThread; j++ { for range numRequestsPerThread {
switch rand.IntN(3) { switch rand.IntN(3) {
case 0: case 0:
assertRequestBody(t, "http://localhost/transition-worker-1.php", "Hello from worker 1") assertRequestBody(t, "http://localhost/transition-worker-1.php", "Hello from worker 1")

View File

@@ -227,7 +227,7 @@ func (rw *ResponseRecorder) Result() *http.Response {
if trailers, ok := rw.snapHeader["Trailer"]; ok { if trailers, ok := rw.snapHeader["Trailer"]; ok {
res.Trailer = make(http.Header, len(trailers)) res.Trailer = make(http.Header, len(trailers))
for _, k := range trailers { for _, k := range trailers {
for _, k := range strings.Split(k, ",") { for k := range strings.SplitSeq(k, ",") {
k = http.CanonicalHeaderKey(textproto.TrimString(k)) k = http.CanonicalHeaderKey(textproto.TrimString(k))
if !httpguts.ValidTrailerHeader(k) { if !httpguts.ValidTrailerHeader(k) {
// Ignore since forbidden by RFC 7230, section 4.1.2. // Ignore since forbidden by RFC 7230, section 4.1.2.

View File

@@ -39,9 +39,7 @@ func TestStateShouldHaveCorrectAmountOfSubscribers(t *testing.T) {
} }
func assertNumberOfSubscribers(t *testing.T, threadState *threadState, expected int) { func assertNumberOfSubscribers(t *testing.T, threadState *threadState, expected int) {
maxWaits := 10_000 // wait for 1 second max for range 10_000 { // wait for 1 second max
for i := 0; i < maxWaits; i++ {
time.Sleep(100 * time.Microsecond) time.Sleep(100 * time.Microsecond)
threadState.mu.RLock() threadState.mu.RLock()
if len(threadState.subscribers) == expected { if len(threadState.subscribers) == expected {

View File

@@ -46,7 +46,7 @@ func pollForWorkerReset(t *testing.T, handler func(http.ResponseWriter, *http.Re
assert.Equal(t, "requests:1", body) assert.Equal(t, "requests:1", body)
// now we spam file updates and check if the request counter resets // 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) updateTestFile("./testdata/files/test.txt", "updated", t)
time.Sleep(pollingTime * time.Millisecond) time.Sleep(pollingTime * time.Millisecond)
body, _ := testGet("http://example.com/worker-with-counter.php", handler, t) body, _ := testGet("http://example.com/worker-with-counter.php", handler, t)