Reviewing files that changed from the base of the PR and between 8ee0ca7 and eb29c53.

• .golangci.yml is excluded by !**/*.yml

• adapter_test.go (3 hunks)
• app.go (2 hunks)
• app_test.go (47 hunks)
• bind_test.go (3 hunks)
• binder/mapping.go (2 hunks)
• color.go (1 hunks)
• ctx_interface_gen.go (1 hunks)
• ctx_test.go (49 hunks)
• extractors/extractors_test.go (1 hunks)
• hooks.go (1 hunks)
• hooks_test.go (3 hunks)
• listen.go (9 hunks)
• listen_test.go (4 hunks)
• middleware/adaptor/adaptor_test.go (9 hunks)
• middleware/basicauth/basicauth.go (1 hunks)
• middleware/basicauth/basicauth_test.go (20 hunks)
• middleware/cache/cache_test.go (56 hunks)
• middleware/compress/compress_test.go (30 hunks)
• middleware/cors/cors.go (4 hunks)
• middleware/cors/cors_test.go (2 hunks)
• middleware/csrf/config.go (1 hunks)
• middleware/csrf/csrf.go (8 hunks)
• middleware/csrf/csrf_test.go (11 hunks)
• middleware/earlydata/earlydata_test.go (6 hunks)
• middleware/encryptcookie/encryptcookie_test.go (4 hunks)
• middleware/envvar/envvar_test.go (7 hunks)
• middleware/etag/etag_test.go (9 hunks)
• middleware/expvar/expvar_test.go (6 hunks)
• middleware/favicon/favicon_test.go (7 hunks)
• middleware/healthcheck/healthcheck_test.go (3 hunks)
• middleware/helmet/helmet_test.go (8 hunks)
• middleware/idempotency/idempotency_test.go (11 hunks)
• middleware/keyauth/keyauth_test.go (30 hunks)
• middleware/limiter/limiter.go (2 hunks)
• middleware/limiter/limiter_fixed.go (1 hunks)
• middleware/limiter/limiter_sliding.go (1 hunks)
• middleware/limiter/limiter_test.go (32 hunks)
• middleware/logger/config.go (1 hunks)
• middleware/logger/default_logger.go (3 hunks)
• middleware/logger/logger.go (2 hunks)
• middleware/logger/logger_test.go (31 hunks)
• middleware/logger/tags.go (1 hunks)
• middleware/logger/utils.go (2 hunks)
• middleware/pprof/pprof_test.go (11 hunks)
• middleware/proxy/proxy.go (2 hunks)
• middleware/proxy/proxy_test.go (34 hunks)
• middleware/recover/recover_test.go (4 hunks)
• middleware/redirect/redirect_test.go (9 hunks)
• middleware/requestid/requestid_test.go (4 hunks)
• middleware/rewrite/rewrite_test.go (5 hunks)
• middleware/session/middleware.go (3 hunks)
• middleware/skip/skip_test.go (4 hunks)
• middleware/static/static_test.go (48 hunks)
• middleware/timeout/timeout_test.go (8 hunks)
• mount_test.go (17 hunks)
• prefork.go (1 hunks)
• prefork_test.go (4 hunks)
• redirect.go (1 hunks)
• redirect_test.go (4 hunks)
• res.go (2 hunks)
• res_interface_gen.go (1 hunks)
• router.go (2 hunks)
• router_test.go (41 hunks)
• services.go (1 hunks)
• services_test.go (1 hunks)

• GitHub Check: assign-v3
• GitHub Check: unit (1.25.x, windows-latest)
• GitHub Check: lint
• GitHub Check: Compare
• GitHub Check: repeated

Reviewing files that changed from the base of the PR and between eb29c53 and 578b96c.

• app.go (3 hunks)
• app_test.go (47 hunks)
• client/client_test.go (2 hunks)
• client/helper_test.go (1 hunks)
• ctx.go (1 hunks)
• ctx_interface_gen.go (2 hunks)
• ctx_test.go (52 hunks)
• extractors/extractors.go (1 hunks)
• helpers.go (5 hunks)
• internal/storage/memory/memory.go (3 hunks)
• internal/tlstest/tls.go (3 hunks)
• middleware/adaptor/adaptor_test.go (10 hunks)
• middleware/cache/heap.go (1 hunks)
• middleware/cors/utils.go (2 hunks)
• middleware/csrf/helpers.go (1 hunks)
• middleware/idempotency/idempotency_test.go (12 hunks)
• middleware/logger/tags.go (2 hunks)
• middleware/proxy/proxy_test.go (35 hunks)
• middleware/static/static.go (1 hunks)
• mount_test.go (18 hunks)
• path.go (1 hunks)
• req.go (7 hunks)
• req_interface_gen.go (1 hunks)
• res.go (5 hunks)
• router.go (4 hunks)

• middleware/logger/tags.go
• app_test.go
• middleware/proxy/proxy_test.go

• GitHub Check: Compare
• GitHub Check: repeated
• GitHub Check: unit (1.25.x, windows-latest)
• GitHub Check: unit (1.25.x, macos-latest)
• GitHub Check: unit (1.25.x, macos-13)
• GitHub Check: lint

middleware/static/static.go (1)

205-205: LGTM! Idiomatic empty string check.

The change to use != "" instead of len(cacheControlValue) > 0 is more idiomatic in Go and improves readability while maintaining identical behavior.

middleware/cors/utils.go (2)

21-21: LGTM! Idiomatic empty-string check.

The change from len(input) > 0 to input != "" is more idiomatic in Go and aligns with the modernization objectives of this PR.

---

34-34: LGTM! Named return values improve clarity.

The addition of named return values (valid bool, normalized string) makes the function signature self-documenting and improves code readability. The implementation correctly uses explicit returns throughout.

middleware/cache/heap.go (3)

84-87: LGTM! Named return parameters improve documentation.

The addition of named return parameters makes the function signature self-documenting, clarifying that the method returns the cache key and entry size.

---

90-92: LGTM! Consistent with the other removal methods.

The named return parameters maintain consistency with removeInternal and clearly document the method's return values.

---

95-97: LGTM! Completes the consistent signature pattern.

The named return parameters are consistent with the other removal methods, making the API more uniform and self-documenting.

client/client_test.go (1)

29-32: LGTM! Correct use of named return values.

The function signature now uses named return values, and the assignment on line 32 correctly uses = instead of := to avoid shadowing the named return parameter app.

client/helper_test.go (1)

73-88: LGTM! Named returns improve clarity.

The refactor to named return values is correct and makes the function signature more self-documenting. The implementation properly assigns to the named returns (app = fiber.New() instead of app :=), and the closures correctly capture the necessary variables. The explicit return statement maintains clarity even though naked return could be used.

mount_test.go (1)

25-31: Use of http.NoBody in tests: LGTM

Switching nil bodies to http.NoBody matches gocritic httpNoBody guidance and keeps semantics.

Also applies to: 47-51, 77-90, 100-137, 144-151

internal/storage/memory/memory.go (1)

52-55: Idiom improvement for empty key checks

key == "" is clearer than len(key) == 0. Behavior preserved.

Also applies to: 80-83, 112-115

middleware/csrf/helpers.go (1)

21-48: Named return values: OK

Switching to named returns improves clarity without changing behavior. Origin validation/normalization logic remains sound.

middleware/idempotency/idempotency_test.go (1)

83-97: http.NoBody usage: LGTM

The move to http.NoBody across requests is correct and aligns with gocritic httpNoBody.

Also applies to: 254-264, 272-279, 287-294, 302-309, 328-343, 352-363, 372-378, 400-405, 415-420

path.go (1)

215-226: Loop condition change is approved; Go version compatibility confirmed

for pattern != "" is semantically correct and clear. The module's Go version (1.25.0) fully supports all modern constructs used in the codebase, including integer-range loops like for i := range segLen at path.go:262. No compatibility issues will arise.

req_interface_gen.go (1)

33-39: Verification complete: interface signature is compatible with implementation

The implementation in req.go:84 matches the interface signatures in both req_interface_gen.go:33 and ctx_interface_gen.go:187. Named return values don't affect Go's type identity, and the single implementation found compiles correctly with the interface signature. No breaking changes.

ctx_interface_gen.go (2)

187-187: LGTM! Named return values improve clarity.

The addition of named return values (body []byte, decodesRealized uint8, err error) enhances readability without changing behavior.

---

376-376: LGTM! Pointer parameter reduces memory overhead.

Changing the route parameter from Route to *Route addresses the gocritic hugeParam issue by avoiding a struct copy. The Route struct contains multiple fields and slices, making pointer passing more efficient.

ctx.go (1)

408-408: LGTM! More idiomatic empty string check.

The change from len(token) == 0 to token == "" is a style improvement with no behavioral change. Both are functionally equivalent, but the latter is more idiomatic in Go.

internal/tlstest/tls.go (1)

20-20: LGTM! Named return values improve clarity.

The refactor to use named return values (serverTLSConf and clientTLSConf) is implemented correctly:

• Signature declares the named returns
• Assignments changed from := to = to properly assign to the named variables
• Behavior remains unchanged

This makes the function's return contract more explicit.

Also applies to: 119-119, 128-128

middleware/adaptor/adaptor_test.go (1)

225-225: LGTM! Correctly addresses gocritic httpNoBody check.

Replacing nil with http.NoBody for requests without a body is the correct fix for the gocritic httpNoBody linter check. http.NoBody is more explicit and semantically correct, indicating an intentionally empty body rather than an uninitialized value.

All changes are in test code with no behavioral impact on test outcomes.

Also applies to: 253-253, 345-345, 354-354, 399-399, 423-423, 434-434, 675-675, 796-796

helpers.go (3)

168-168: LGTM! More idiomatic empty string checks.

The changes from len(value) == 0 and len(path) == 0 to value == "" and path == "" are style improvements with no behavioral changes. The latter form is more idiomatic in Go.

Also applies to: 175-175

---

616-626: LGTM! Named return values improve clarity.

The refactor to use named return values for normalizeEtag is implemented correctly:

• Line 616: Signature declares named returns (value string, weak bool, ok bool)
• Line 617: Assignment to weak uses = correctly
• Return statements properly return the named values

This makes the function's contract more explicit.

---

689-722: LGTM! Named return values implemented correctly.

The refactor to use named return values for parseAddr is correct:

• Line 689: Signature declares (host string, port string)
• Lines 699, 709: Assignments use = to assign to named return variables
• All return paths correctly return or assign the named variables

This improves code clarity without changing behavior.

res.go (3)

143-143: LGTM! More idiomatic empty string check.

The change from len(h) == 0 to h == "" is a style improvement with no behavioral change.

---

574-577: Nil check is necessary but consider edge cases.

The nil check prevents panics when getLocationFromRoute is called with a nil pointer, which is good defensive programming. However, note that GetRouteURL (line 609) calls app.GetRoute(routeName) which returns a zero-value Route{} (not nil) when the route is not found, then passes &route to this function.

For that call path:

• A zero-value Route will have an empty routeParser.segs slice
• The loop at line 581 will iterate zero times (safe)
• The function will return an empty location string (line 601)

The nil check protects against direct calls with nil, which is appropriate for a public method.

---

797-799: LGTM! Idiomatic empty string checks.

The changes to use file == "" and cacheControlValue != "" instead of length checks are more idiomatic with no behavioral changes.

Also applies to: 849-849

extractors/extractors.go (1)

130-130: LGTM! More idiomatic empty string check.

The change from len(rest) == 0 to rest == "" is a style improvement with no behavioral change. The latter is more idiomatic in Go.

app.go (3)

629-629: LGTM! Pointer parameter avoids struct copy.

Passing &app.config.ColorScheme as a pointer to defaultColors addresses the gocritic hugeParam issue by avoiding a copy of the ColorScheme struct. This aligns with the broader refactor to use pointer parameters for large structs.

---

650-650: LGTM! More idiomatic empty string check.

The change from len(s) == 0 to s == "" is a style improvement with no behavioral change.

---

740-740: LGTM! Pointer parameter matches updated hook signature.

Passing app.latestRoute (which is *Route) to executeOnNameHooks is correct, as the hook signature was updated to accept *Route instead of Route as part of the hugeParam fixes.

router.go (3)

96-96: LGTM: Idiomatic string empty check.

The change from len(detectionPath) > 0 to detectionPath != "" is a more idiomatic Go pattern for checking non-empty strings while preserving the same logic.

---

520-520: LGTM: More idiomatic nil check.

Changing from nil == h to h == nil follows Go conventions by avoiding Yoda-style conditionals.

---

616-616: Code changes verified—no issues found.

The executeOnRouteHooks signature has been correctly updated to accept *Route (line 140 in hooks.go). Both calls at lines 616 and 679 in router.go correctly pass route pointers directly without dereferencing, which aligns with the PR's goal of avoiding large struct copies.

req.go (3)

87-87: LGTM: Named return values improve clarity.

Adding named return values provides better documentation without changing behavior. All return statements remain explicit, which is good practice.

---

146-146: LGTM: Idiomatic string emptiness checks.

The changes from len(str) > 0 / len(str) == 0 to direct string comparison (str != "" / str == "") are idiomatic Go patterns that improve readability while preserving the same logic.

Also applies to: 317-317, 353-353, 586-586

---

410-410: LGTM: Label removal is correct.

Removing the Local label from continue statements is appropriate since these are within the innermost loops. The plain continue statements correctly skip to the next iteration.

Also applies to: 462-462

Reviewing files that changed from the base of the PR and between f3f20d3 and d02760d.

• client/client_test.go (2 hunks)
• client/helper_test.go (1 hunks)
• ctx_interface_gen.go (2 hunks)
• helpers.go (5 hunks)
• internal/tlstest/tls.go (3 hunks)
• middleware/cache/heap.go (1 hunks)
• middleware/cors/utils.go (2 hunks)
• middleware/csrf/helpers.go (1 hunks)
• middleware/idempotency/idempotency_test.go (12 hunks)
• middleware/proxy/proxy_test.go (35 hunks)
• req.go (8 hunks)
• req_interface_gen.go (1 hunks)

• middleware/cors/utils.go

• GitHub Check: lint
• GitHub Check: repeated
• GitHub Check: unit (1.25.x, macos-latest)
• GitHub Check: unit (1.25.x, macos-13)
• GitHub Check: unit (1.25.x, windows-latest)
• GitHub Check: Compare

middleware/idempotency/idempotency_test.go (2)

84-84: LGTM: http.NoBody migration is correct.

Replacing nil with http.NoBody across all test request creation is the proper way to represent an empty request body and aligns with the gocritic httpNoBody linting check. Since none of these tests require actual request bodies, the change is semantically equivalent while being more explicit.

Also applies to: 250-250, 254-254, 268-268, 283-283, 298-298, 324-324, 331-331, 348-348, 368-368, 382-382, 396-396, 411-411

---

226-236: Helper function changes maintain clarity.

The addition of named returns and the variable rename from body to payload avoids shadowing the return parameter while keeping the function's behavior unchanged. The nolint comment appropriately explains the design choice.

middleware/cache/heap.go (1)

84-96: LGTM: Named returns added to satisfy gocritic.

The addition of named return values to these three methods addresses gocritic's unnamedResult check as described in the PR objectives. The approach is consistent across all three methods, and the nolint directives appropriately document the conflict with the nonamedreturns check. Since the function bodies still use explicit returns rather than relying on implicit named returns, the behavior remains unchanged.

client/client_test.go (2)

29-36: LGTM! Named returns properly implemented for gocritic compliance.

The function signature correctly uses named returns with an appropriate nolint comment explaining the requirement. The initialization of app via assignment (rather than declaration) is the correct approach when using named return values.

---

40-50: Good practice: explicit parameter instead of closure capture.

While not strictly required for the named returns refactor, passing app as an explicit parameter to the goroutine (rather than capturing it from the closure) makes the data flow more explicit and is generally better practice for concurrent code.

client/helper_test.go (1)

73-88: LGTM! Clean refactor to named returns.

The function has been cleanly refactored to use named returns for gocritic compliance. The explicit variable definitions for dial and start with their function types make the code very readable, and the explicit return statement is good practice even with named returns.

internal/tlstest/tls.go (3)

20-20: LGTM: Named returns improve clarity for TLS config pair.

The named return parameters clearly distinguish the server and client TLS configurations, which is valuable given the function returns two configs of the same type. The nolint directive appropriately allows this to satisfy gocritic's unnamedResult preference while suppressing conflicting linters.

---

119-122: LGTM: Correct assignment syntax for named return.

The change from := to = is required because serverTLSConf is now a named return parameter. The assignment correctly sets the server TLS configuration that will be returned.

---

128-131: LGTM: Correct assignment syntax for named return.

The change from := to = is required because clientTLSConf is now a named return parameter. The assignment correctly sets the client TLS configuration that will be returned.

middleware/proxy/proxy_test.go (5)

7-7: LGTM! Import added for http.NoBody.

The net/http import is required for the http.NoBody constant used throughout the test file.

---

33-47: LGTM! Named returns implemented correctly.

The helper function now uses named returns with appropriate nolint documentation. The implementation correctly assigns to the named return variables and maintains existing behavior.

---

49-52: LGTM! Wrapper function with named returns.

The IPv4 wrapper correctly delegates to createProxyTestServer with the appropriate network constant.

---

54-57: LGTM! Wrapper function with named returns.

The IPv6 wrapper correctly delegates to createProxyTestServer with the appropriate network constant.

---

126-938: LGTM! Systematic replacement of nil with http.NoBody.

All test request constructions now correctly use http.NoBody instead of nil for empty request bodies. This addresses the gocritic httpNoBody check and aligns with Go best practices for test requests. The changes are consistent across all 33+ test cases and maintain existing behavior.

req.go (1)

84-127: Named returns keep decode accounting clear.

The switch to named results and the per-iteration decodeErr makes the decoding loop easier to follow without altering the existing fallbacks. Nice cleanup.

Reviewing files that changed from the base of the PR and between ce7a74f and b445ffe.

• docs/middleware/limiter.md (2 hunks)
• docs/middleware/logger.md (3 hunks)
• docs/middleware/proxy.md (2 hunks)
• docs/whats_new.md (4 hunks)

• GitHub Check: Compare
• GitHub Check: unit (1.25.x, macos-latest)
• GitHub Check: unit (1.25.x, windows-latest)
• GitHub Check: repeated
• GitHub Check: lint
• GitHub Check: update_release_draft

docs/middleware/proxy.md (1)

13-13: Documentation properly reflects Balancer variadic signature change.

The signature update to variadic config is clearly documented and all examples show appropriate usage patterns.

docs/middleware/logger.md (1)

103-117: Documentation correctly reflects LoggerFunc pointer-to-Config signature change.

The config parameter is now properly documented as a pointer type, and examples for predefined formats are clear and consistent.

docs/middleware/limiter.md (1)

23-26: Handler interface properly documented with pointer-to-Config signature.

The new interface signature is clearly presented and the LimiterMiddleware field explanation properly communicates the pointer-based config pattern.

docs/whats_new.md (3)

1285-1286: LoggerFunc signature change properly documented with migration guidance.

The documentation clearly explains the shift to pointer-based config in custom logger implementations and provides clear examples of the updated signature.

---

1448-1449: Limiter Handler interface change well-explained for custom implementations.

The documentation accurately conveys that custom algorithms must now implement the updated interface with pointer-to-Config parameter, supporting migration from v2 to v3.

---

1466-1467: Proxy.Balancer variadic config change consistently documented.

Both the feature overview (lines 1466-1467) and migration guide (line 2487) properly document the move to optional variadic configuration, providing clear guidance for users upgrading from v2.

Also applies to: 2487-2487