Skip to content

Conversation

@1911860538
Copy link
Contributor

@1911860538 1911860538 commented Sep 26, 2025

This ensures that empty slices/arrays properly use default values when available,
and prevents index out of bounds errors when no defaults exist.
Fixes #4377

@codecov
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.92%. Comparing base (3dc1cd6) to head (71dee21).
⚠️ Report is 191 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4380      +/-   ##
==========================================
- Coverage   99.21%   98.92%   -0.29%     
==========================================
  Files          42       44       +2     
  Lines        3182     3446     +264     
==========================================
+ Hits         3157     3409     +252     
- Misses         17       26       +9     
- Partials        8       11       +3     
Flag Coverage Δ
?
--ldflags="-checklinkname=0" -tags sonic 98.91% <100.00%> (?)
-tags go_json 98.86% <100.00%> (?)
-tags nomsgpack 98.91% <100.00%> (?)
go-1.18 ?
go-1.19 ?
go-1.20 ?
go-1.21 ?
go-1.23 98.92% <100.00%> (?)
go-1.24 98.92% <100.00%> (?)
go-1.25 98.92% <100.00%> (?)
macos-latest 98.92% <100.00%> (-0.29%) ⬇️
ubuntu-latest 98.92% <100.00%> (-0.29%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@appleboy appleboy requested a review from Copilot September 27, 2025 03:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes empty slice/array handling in form binding to properly use default values when available and prevents index out of bounds errors when no defaults exist. It addresses issue #4377.

Key changes:

  • Modified the condition from checking form field presence to checking if the field has empty values
  • Added proper handling for cases where no default value exists for empty slices/arrays
  • Ensured default values are applied when empty arrays/slices are encountered

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
binding/form_mapping.go Updated slice and array handling logic to check for empty values instead of field presence
binding/form_mapping_test.go Added comprehensive test coverage for empty slice/array scenarios with and without defaults

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +233 to +236
if len(vs) == 0 {
if !opt.isDefaultExists {
return false, nil
}
Copy link

Copilot AI Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition len(vs) == 0 will be true both when the field is not present in the form (ok == false) and when the field is present but has an empty slice. Consider adding a comment to clarify this intentional behavior change from the previous !ok check.

Copilot uses AI. Check for mistakes.
Comment on lines +256 to +259
if len(vs) == 0 {
if !opt.isDefaultExists {
return false, nil
}
Copy link

Copilot AI Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition len(vs) == 0 will be true both when the field is not present in the form (ok == false) and when the field is present but has an empty slice. Consider adding a comment to clarify this intentional behavior change from the previous !ok check.

Copilot uses AI. Check for mistakes.
@appleboy appleboy added the type/bug Found something you weren't expecting? Report it here! label Sep 27, 2025
@appleboy appleboy added this to the v1.12 milestone Sep 27, 2025
@appleboy appleboy merged commit c3d1092 into gin-gonic:master Oct 11, 2025
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Found something you weren't expecting? Report it here!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v1.11.0 regression: panic in binding/form_mapping.go setByForm()

2 participants