[Question] Is ignoring mismatched slice element types intended behavior?

[lll-lll-lll-lll]

Is there an existing feature request for this?

• I have searched the existing feature requests

Is your feature request related to a problem? Please describe.

Question

when a struct field is a slice with two different element types, the current implementation seems to ignore element of the wrong type. Is this intentional behavior? Is there no error handling for this scenario?

Implementation

https://github.com/gorilla/schema/blob/main/decoder.go#L130-L137

for _, val := range vals {
	//this check is to handle if the wrong value is provided
	if convertedVal := builtinConverters[f.typ.Elem().Kind()](val); convertedVal.IsValid() {
		defaultSlice = reflect.Append(defaultSlice, convertedVal)
	}
}

example

var decoder = schema.NewDecoder()

type D struct {
	S string `schema:"s,default:test1"`
	A []int  `schema:"a,default:test|1"`
}

func main() {
	data := map[string][]string{}
	d := D{}
	if err := decoder.Decode(&d, data); err != nil {
		log.Fatal("Error while decoding:", err)
	}
	fmt.Println(d)
}

// output
// {test1 [1]}

Describe the solution that you would like.

for _, val := range vals {
	//this check is to handle if the wrong value is provided
	if convertedVal := builtinConverters[f.typ.Elem().Kind()](val); !convertedVal.IsValid() {
            // error handling
	}
        defaultSlice = reflect.Append(defaultSlice, convertedVal)
}

Describe alternatives you have considered.

No response

Anything else?

No response

[zak905]

Hi @lll-lll-lll-lll, thanks. It seems like this is intentional, but I honestly do not recall the reasons behind the choice because it has been a while since I submitted the PR. Maybe it was because default was considered as something optional and therefore should not break the decoding, no matter what. There is also a test case for this behavior: https://github.com/gorilla/schema/blob/main/decoder_test.go#L2217

I like the idea of failing when a default value that do not match the type is provided. This would enforce correctness and notify when some unexpected value is used. It's certainly a plus.

@AlexVulaj, @jaitaiwan what do you think ?

[AlexVulaj]

I'm in agreement with @zak905 here - I think it's better to handle cases with an unexpected value. Please feel free to submit a PR for this change and tag me in it, thanks!

[lll-lll-lll-lll]

Thank you, I will try to make a PR!