#3924 - generate UnmarshalText conversion for struct:field:type on HTTP path and query params

[sbryzak]

Fix for #3924: generate UnmarshalText conversion for struct:field:type on HTTP path and query params.

When struct:field:type meta is used on a string-based attribute mapped to an HTTP path or query parameter, the generated encode_decode.go assigned the raw string value directly to the typed field, causing a compile error.

This fix detects when struct:field:type overrides a string attribute and the custom type implements encoding.TextUnmarshaler, and generates a call to UnmarshalText instead of the current direct string assignment. This should work for both path params (read via mux.Vars) and query params (read via url.Values.Get).

A new isStringMetaType helper function has been added to service_Data.go, which sets IsTextUnmarshaler on AttributeData when the condition is met. The request_elements.go.tpl and query_type_conversion.go.tpl templates use this flag to bypass the string short-circuit path and emit the correct conversion code. The mustValidate logic is also updated so that err is declared in the generated var block when a TextUnmarshaler conversion is present.

Format validation is also skipped for attributes with a struct:field:type meta that implement encoding.TextUnmarshaler, as the UnmarshalText call already validates the format implicitly during deserialization.

[raphael]

Thanks for jumping on this, this looks like the right direction and the generated required path/query cases are much better.

I think there are two edge cases worth tightening before merging:

• For optional query params with struct:field:type, the generated variable is a pointer (*uuid.UUID etc.), but the template calls p.UnmarshalText(...) directly. That means an optional value can end up calling the method on a nil pointer. I think this needs to unmarshal into a local value and then assign &v when the param is present.

• The Format(...) validation skip in codegen/validation.go looks broader than the HTTP path/query case this PR is fixing. Since it keys only off struct:field:type, it can remove format validation in places where no UnmarshalText decode happened, like body/result/service validation. I’d keep the validation behavior unchanged there and only avoid duplicate validation in the specific generated HTTP param path if needed.

It’d also be good to add a golden case that matches the original issue more closely: Format(FormatUUID) plus struct:field:type, and ideally an optional query param case so the pointer path is covered too.

[sbryzak]

Thanks for the feedback! I've updated the PR as follows:

Nil pointer for optional query params The template now unmarshals into a local stack value and only assigns &v to the pointer variable on success, so a missing or invalid optional param can no longer cause a nil pointer dereference.

Format validation skip - I did take your suggestion to revert the changes to codegen/validation.go and narrow the skip to the HTTP param codegen path only, but testing against my real project revealed that goa.ValidateFormat always expects a string argument. Passing a uuid.UUID or any other struct:field:type override causes a compile error regardless of context (body fields, result types, etc) So the format validation skip in codegen/validate.go keyed off struct:field:type is actually necessary everywhere, not just in the HTTP param path. Because of this I've had to keep the change in place.

Golden test cases I've added two new cases - Format(FormatUUID) combined with struct:field:type on a required param param (matching the original issue closely) and an optional query param to exercise the pointer variable path.

[sbryzak]

I encountered one additional edge case while testing in a real project - types where UUIDField() (or any struct:field:type plus Format combination) was the only validation on a nested body type would cause a compile error in the generated code.

The parent type's validate function would emit a ValidateX(nested) call, but no ValidateX function was generated for the nested type, because hasValidations() was returning true based on the structural presence of a Format validation, without accounting for the fact that format checks on struct:field:type attributes are skipped at code gen time.

Fixed by adding the same struct:field:type + format-only guard to hasValidations() in codegen/validation.go, keeping it consistent with the skip already applied in validationCode().

[raphael]

Nice, thanks for chasing those extra cases down. The optional query path looks much better now, and the added golden cases cover the shape I was worried about.

One small thought on the latest hasValidations() change: I agree with the behavior, but I wonder if we can make it compose a bit more naturally with the existing validation generator. Right now hasValidations() is starting to mirror the structure of expr.ValidationExpr and has to know that Format is skipped for struct:field:type. That feels like it could drift the next time validation codegen grows another special case.

Could we instead have hasValidations() ask the same local validation code path whether anything would actually be emitted? In other words, factor the non-recursive part so both places use validationCode(...) as the source of truth for “does this attribute produce validation code?”, with dummy target/context names. I definitely wouldn’t call recurseValidationCode() from there, since that brings back the recursion problem the current comment calls out, but reusing the local renderer would avoid hand-maintaining a parallel predicate.

Something along those lines would make the nested ValidateX guard follow future validation-codegen skips automatically, instead of encoding just this one Format case in hasValidations().

[sbryzak]

That's a great point re. hasValidations() and makes way more sense. I've refactored it to delegate to validationCode() directly.

[raphael]

This looks good to me now, the hasValidations() shape is much nicer with validationCode() as the source of truth.

One last small thing: since the regression you found was specifically the nested body ValidateX mismatch, could we add a tiny focused test for that path too? That would make this much harder to regress later.

Also, super minor: the extra generatedRequiredValidation() check in hasValidations() looks like it may be redundant now, since validationCode() already calls it at the end. Not a big deal either way, but the comment there reads a little stale because of that.

[sbryzak]

Done - two small changes made:

Redundant Check - I've removed the generatedRequiredValidation() fallback from hasValidations(), and updated the comment. As @raphael correctly pointed out the call is redundant as validationCode() already calls it.

Regression test - Added a focused DSL case with a nested body type whose only validations are Format(FormatUUID) and struct:field:type. The golden output confirms that the parent validate function does not emit a ValidateNestedUUIDOnly call, which is the exact shape that was previously generating an unresolved reference compile error.

[raphael]

Perfect, thank you!