binder/form.go (1)

34-35: Fail-fast on formatBindData error clarifies control flow; verify no partial writes and cross-binder intent

Returning immediately on formatBindData errors improves readability and avoids doing unnecessary work. Two quick follow-ups:

• Please confirm formatBindData is transactional with respect to the destination map (i.e., it doesn’t partially mutate data before returning an error). If it can, we may end up with partially-bound inputs before parse runs.
• Header/respHeader binders reportedly ignore formatBindData errors, while form/cookie now short-circuit. If this difference is intentional, consider documenting it in each binder to avoid surprises for users relying on consistent binder semantics.

binder/cookie_test.go (1)

89-100: Strengthen the negative-path test: run in parallel and assert no partial binding

The test correctly exercises the parse error path. Two small improvements:

• Make the test parallel to match the style used elsewhere and speed up CI.
• Assert that no partial binding occurred (Age remains zero-value).

 func Test_CookieBinder_Bind_ParseError(t *testing.T) {
-	b := &CookieBinding{}
+	t.Parallel()
+	b := &CookieBinding{}
 	type User struct {
 		Age int `cookie:"age"`
 	}
 	var user User
 	req := fasthttp.AcquireRequest()
 	req.Header.SetCookie("age", "invalid")
 	t.Cleanup(func() { fasthttp.ReleaseRequest(req) })
 	err := b.Bind(req, &user)
 	require.Error(t, err)
+	require.Zero(t, user.Age)
 }

binder/header_test.go (1)

89-100: Make the test parallel and remove case-ambiguity in header name (nitpick)

• Add t.Parallel for consistency and speed.
• Optional: set the header using the same casing as the struct tag ("Age") to reduce ambiguity when reading the test (the binder likely treats headers case-insensitively anyway).

 func Test_HeaderBinder_Bind_ParseError(t *testing.T) {
-	b := &HeaderBinding{}
+	t.Parallel()
+	b := &HeaderBinding{}
 	type User struct {
 		Age int `header:"Age"`
 	}
 	var user User
 	req := fasthttp.AcquireRequest()
-	req.Header.Set("age", "invalid")
+	// Use same casing as the tag for clarity (binder is case-insensitive)
+	req.Header.Set("Age", "invalid")
 	t.Cleanup(func() { fasthttp.ReleaseRequest(req) })
 	err := b.Bind(req, &user)
 	require.Error(t, err)
 }

binder/cookie.go (1)

25-26: Early return on formatBindData error is consistent with fail-fast policy; confirm intended divergence from header binder

This change aligns cookie binding with a fail-fast approach (vs. header binder which reportedly ignores formatBindData errors). If this divergence is intentional (e.g., to surface malformed cookie shapes earlier), please document it to set expectations for users switching between binders.

Also, similar to form binder, please confirm formatBindData doesn’t partially mutate data when returning an error.

binder/resp_header_test.go (1)

79-90: Parallelize and assert no partial binding for the negative path

Good coverage of the parse error. Recommend:

• Add t.Parallel for consistency.
• Assert that the destination field remains the zero-value when binding fails.

 func Test_RespHeaderBinder_Bind_ParseError(t *testing.T) {
-	b := &RespHeaderBinding{}
+	t.Parallel()
+	b := &RespHeaderBinding{}
 	type User struct {
 		Age int `respHeader:"age"`
 	}
 	var user User
 	resp := fasthttp.AcquireResponse()
 	resp.Header.Set("age", "invalid")
 	t.Cleanup(func() { fasthttp.ReleaseResponse(resp) })
 	err := b.Bind(resp, &user)
 	require.Error(t, err)
+	require.Zero(t, user.Age)
 }

binder/resp_header.go (1)

25-26: Align error-handling with HeaderBinding (formatBindData can't fail here).

Given bracket translation is disabled (last arg = false) and v is a string, formatBindData cannot return an error for response headers. Returning early on a theoretically impossible error makes this binder’s behavior diverge from HeaderBinding, which ignores the error with a clear comment. Consider mirroring HeaderBinding for consistency.

Apply this diff to align behavior:

-		if err := formatBindData(b.Name(), out, data, k, v, b.EnableSplitting, false); err != nil {
-			return err
-		}
+		_ = formatBindData(b.Name(), out, data, k, v, b.EnableSplitting, false) //nolint:errcheck // always returns nil for string values with bracket translation disabled

Optional (perf/readability): switch to the zero-allocation visitor and the narrow helper since headers are strings only:

// Optional: replace the loop with VisitAll + assignBindData
resp.Header.VisitAll(func(key, val []byte) {
	assignBindData(b.Name(), out, data, utils.UnsafeString(key), utils.UnsafeString(val), b.EnableSplitting)
})

binder/header.go (1)

24-24: LGTM; add precondition note or use assignBindData to avoid nolint.

Ignoring formatBindData errors here is correct because: value type is string and bracket translation is disabled. To make this future-proof, consider either:

• Adding a short precondition comment (value is string; bracket=false ⇒ guaranteed nil), or
• Using assignBindData, which matches the exact needs and removes the nolint.

Example using assignBindData (optional):

for key, val := range req.Header.All() {
	assignBindData(b.Name(), out, data, utils.UnsafeString(key), utils.UnsafeString(val), b.EnableSplitting)
}

Optional (perf): prefer VisitAll to avoid map allocations, mirroring Fiber’s zero-allocation goals.

binder/form_test.go (3)

57-70: Strengthen the assertion to validate the error content.

The test currently checks only that an error occurs. Verifying the message guards against regressions in error surfacing.

 err := b.Bind(req, &user)
-require.Error(t, err)
+require.Error(t, err)
+require.ErrorContains(t, err, "error converting value for \"age\"")

---

208-222: Prefer ErrorContains over string Contains on err.Error().

require.ErrorContains reads cleaner and avoids manual .Error() calls.

- require.Error(t, err)
- require.Contains(t, err.Error(), "unmatched brackets")
+ require.Error(t, err)
+ require.ErrorContains(t, err, "unmatched brackets")

---

223-240: Ditto: use ErrorContains for consistency and clarity.

Same rationale as above.

- require.Error(t, err)
- require.Contains(t, err.Error(), "unmatched brackets")
+ require.Error(t, err)
+ require.ErrorContains(t, err, "unmatched brackets")

binder/mapping_test.go (3)

379-398: Test is a bit weak for map[string]int; assert observable behavior.

The map[string]int branch doesn’t validate outputs. If parseToMap is expected to succeed, asserting the resulting contents avoids a vacuous pass. If values aren’t convertible, clarify the intended fallback (omit keys vs. zero values).

Proposed strengthening:

 m3 := make(map[string]int)
 err = parseToMap(m3, data)
 require.NoError(t, err)
+// If non-convertible values are skipped:
+_, ok := m3["key1"]
+require.False(t, ok)
+// Always present but zeroed if empty input:
+v, ok := m3["empty"]
+require.True(t, ok)
+require.Equal(t, 0, v)

If the intended behavior differs (e.g., attempt conversion and error), please adjust the assertions accordingly to reflect the contract you expect from parseToMap.

---

400-407: Return decoders to the pool to avoid perturbing later tests.

Not strictly necessary here, but putting the decoder back is cheap and keeps the pool state tidy for subsequent tests.

 for _, tag := range tags {
   decAny := decoderPoolMap[tag].Get()
   dec, ok := decAny.(*schema.Decoder)
   require.True(t, ok)
   require.NotNil(t, dec)
+  decoderPoolMap[tag].Put(dec)
 }

---

438-454: Unexported-field scenario is covered; verify intention on nestedKinds.

Asserting nestedKinds[reflect.Int] may be brittle if internal representation changes. If the goal is to ensure unexported fields do not break traversal, consider asserting on observed, stable facets (e.g., presence of exported fields under the correct names) rather than internal-kind bookkeeping.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.

Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.

Learnt from: efectn
PR: gofiber/fiber#3162
File: hooks_test.go:228-228
Timestamp: 2024-12-13T08:14:22.851Z
Learning: In Go test files, prefer using the `require` methods from the `testify` package for assertions instead of manual comparisons and calls to `t.Fatal` or `t.Fatalf`.

Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The project uses the testify/assert package for assertions in unit tests.