Skip to content

feat(binding): add support for encoding.UnmarshalText in uri/query binding #4203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 49 additions & 22 deletions binding/form_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package binding

import (
"encoding"
"errors"
"fmt"
"mime/multipart"
Expand Down Expand Up @@ -136,6 +137,7 @@ func mapping(value reflect.Value, field reflect.StructField, setter setter, tag
type setOptions struct {
isDefaultExists bool
defaultValue string
parser string
}

func tryToSetValue(value reflect.Value, field reflect.StructField, setter setter, tag string) (bool, error) {
Expand Down Expand Up @@ -167,6 +169,8 @@ func tryToSetValue(value reflect.Value, field reflect.StructField, setter setter
setOpt.defaultValue = strings.ReplaceAll(v, ";", ",")
}
}
} else if k, v = head(opt, "="); k == "parser" {
setOpt.parser = v
}
}

Expand All @@ -190,6 +194,20 @@ func trySetCustom(val string, value reflect.Value) (isSet bool, err error) {
return false, nil
}

// trySetUsingParser tries to set a custom type value based on the presence of the "parser" tag on the field.
// If the parser tag does not exist or does not match any of the supported parsers, gin will skip over this.
func trySetUsingParser(val string, value reflect.Value, parser string) (isSet bool, err error) {
switch parser {
case "encoding.TextUnmarshaler":
v, ok := value.Addr().Interface().(encoding.TextUnmarshaler)
if !ok {
return false, nil
}
return true, v.UnmarshalText([]byte(val))
}
return false, nil
}

func trySplit(vs []string, field reflect.StructField) (newVs []string, err error) {
cfTag := field.Tag.Get("collection_format")
if cfTag == "" || cfTag == "multi" {
Expand All @@ -207,7 +225,7 @@ func trySplit(vs []string, field reflect.StructField) (newVs []string, err error
case "pipes":
sep = "|"
default:
return vs, fmt.Errorf("%s is not supported in the collection_format. (csv, ssv, pipes)", cfTag)
return vs, fmt.Errorf("%s is not supported in the collection_format. (multi, csv, ssv, tsv, pipes)", cfTag)
}

totalLength := 0
Expand All @@ -230,7 +248,7 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][

switch value.Kind() {
case reflect.Slice:
if !ok {
if !ok || len(vs) == 0 || (len(vs) > 0 && vs[0] == "") {
vs = []string{opt.defaultValue}

// pre-process the default value for multi if present
Expand All @@ -240,17 +258,19 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][
}
}

if ok, err = trySetCustom(vs[0], value); ok {
if ok, err = trySetUsingParser(vs[0], value, opt.parser); ok {
return ok, err
} else if ok, err = trySetCustom(vs[0], value); ok {
return ok, err
Comment on lines +261 to 264
Copy link
Preview

Copilot AI May 20, 2025

Choose a reason for hiding this comment

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

[nitpick] The pattern of invoking trySetUsingParser followed by trySetCustom is repeated in multiple branches. Extracting this into a small helper would reduce duplication and improve readability.

Suggested change
if ok, err = trySetUsingParser(vs[0], value, opt.parser); ok {
return ok, err
} else if ok, err = trySetCustom(vs[0], value); ok {
return ok, err
if ok, err = trySetValue(vs[0], value, opt); ok {
return ok, err

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only repeated once. Extracting it to a function will make this code even more difficult to understand than it already is

Comment on lines +261 to 264
Copy link
Preview

Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

[nitpick] The pattern of calling trySetUsingParser and then trySetCustom is repeated in multiple branches. Consider unifying this sequence into a helper to reduce duplication and improve clarity.

Suggested change
if ok, err = trySetUsingParser(vs[0], value, opt.parser); ok {
return ok, err
} else if ok, err = trySetCustom(vs[0], value); ok {
return ok, err
if ok, err = trySetValue(vs[0], value, opt.parser); ok {
return ok, err

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

@takanuva15 takanuva15 May 28, 2025

Choose a reason for hiding this comment

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

It's only repeated once. Extracting it to a function will make this code even more difficult to understand than it already is.

Other comments;

  • test coverage: form_mapping.go is at 100% test coverage, which is the only file I edited
  • doc json mistake: fixed it and repushed

}

if vs, err = trySplit(vs, field); err != nil {
return false, err
}

return true, setSlice(vs, value, field)
return true, setSlice(vs, value, field, opt)
case reflect.Array:
if !ok {
if !ok || len(vs) == 0 || (len(vs) > 0 && vs[0] == "") {
vs = []string{opt.defaultValue}

// pre-process the default value for multi if present
Expand All @@ -260,7 +280,9 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][
}
}

if ok, err = trySetCustom(vs[0], value); ok {
if ok, err = trySetUsingParser(vs[0], value, opt.parser); ok {
return ok, err
} else if ok, err = trySetCustom(vs[0], value); ok {
return ok, err
}

Expand All @@ -272,27 +294,32 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][
return false, fmt.Errorf("%q is not valid value for %s", vs, value.Type().String())
}

return true, setArray(vs, value, field)
return true, setArray(vs, value, field, opt)
default:
var val string
if !ok {
if !ok || len(vs) == 0 || (len(vs) > 0 && vs[0] == "") {
val = opt.defaultValue
}

if len(vs) > 0 {
} else if len(vs) > 0 {
val = vs[0]
if val == "" {
val = opt.defaultValue
}
}
if ok, err := trySetCustom(val, value); ok {

if ok, err = trySetUsingParser(val, value, opt.parser); ok {
return ok, err
} else if ok, err = trySetCustom(val, value); ok {
return ok, err
}
return true, setWithProperType(val, value, field)
return true, setWithProperType(val, value, field, opt)
}
}

func setWithProperType(val string, value reflect.Value, field reflect.StructField) error {
func setWithProperType(val string, value reflect.Value, field reflect.StructField, opt setOptions) error {
// this if-check is required for parsing nested types like []MyId, where MyId is [12]byte
if ok, err := trySetUsingParser(val, value, opt.parser); ok {
return err
} else if ok, err = trySetCustom(val, value); ok {
return err
}

switch value.Kind() {
case reflect.Int:
return setIntField(val, 0, value)
Expand Down Expand Up @@ -340,7 +367,7 @@ func setWithProperType(val string, value reflect.Value, field reflect.StructFiel
if !value.Elem().IsValid() {
value.Set(reflect.New(value.Type().Elem()))
}
return setWithProperType(val, value.Elem(), field)
return setWithProperType(val, value.Elem(), field, opt)
default:
return errUnknownType
}
Expand Down Expand Up @@ -447,19 +474,19 @@ func setTimeField(val string, structField reflect.StructField, value reflect.Val
return nil
}

func setArray(vals []string, value reflect.Value, field reflect.StructField) error {
func setArray(vals []string, value reflect.Value, field reflect.StructField, opt setOptions) error {
for i, s := range vals {
err := setWithProperType(s, value.Index(i), field)
err := setWithProperType(s, value.Index(i), field, opt)
if err != nil {
return err
}
}
return nil
}

func setSlice(vals []string, value reflect.Value, field reflect.StructField) error {
func setSlice(vals []string, value reflect.Value, field reflect.StructField, opt setOptions) error {
slice := reflect.MakeSlice(value.Type(), len(vals), len(vals))
err := setArray(vals, slice, field)
err := setArray(vals, slice, field, opt)
if err != nil {
return err
}
Expand Down
Loading