cue-lang
cmd/cue: get go should allow go structs which implement special interfaces to be force-imported
What version of CUE are you using (cue version)?
❯ paru -Q cue cue 0.5.0-1
Does this issue reproduce with the latest stable release?
Yes.
What did you do?
I want to import https://pkg.go.dev/github.com/VictoriaMetrics/operator/[email protected]/victoriametrics/v1beta1 for use as a schema definition with Kubernetes manifests, but I can't because of this rule:
Rules of Converting Go types to CUE
Go structs are converted to cue structs adhering to the following conventions:
...
- a type that implements MarshalJSON, UnmarshalJSON, MarshalYAML, or
UnmarshalYAML is translated to top (_) to indicate it may be any
value. For some Go core types for which the implementation of these
methods is known, like time.Time, the type may be more specific.
What did you expect to see?
I would like to generate the CUE anyway.
What did you see instead?
They implement UnmarshalJSON in a few places, which generates top values.
// VMClusterSpec defines the desired state of VMCluster
// +k8s:openapi-gen=true
#VMClusterSpec: _
I would also like a way to disable this completely. There are some instances where types implement custom json marshalers, but have the same API.
https://pkg.go.dev/github.com/cilium/[email protected]/pkg/policy/api#EndpointSelector
I would also like a way to disable this completely. There are some instances where types implement custom json marshalers, but have the same API.
Interesting - why does that happen, do you think?
Your patch in https://github.com/cue-lang/cue/pull/2642 seems like an okay fix to me. I had written that TODO as a reminder to circle back to this at some point - my understanding is that the heuristic should require both marshal and unmarshal methods, like your patch now does, but I want to check with Marcel first to see if there's any good reason for not doing that.
I'll get back to you on this. Apologies if it takes a week or more, since Marcel is currently in a bit of a focus mode to finish his v0.7 performance work, and I'd rather not distract him with entirely different issues.
@mvdan Sounds good, thank you for your help.
Custom JSON marshalling is sometimes used for validation or sanitising - it looks like the example I gave above wants to marshal the the embedded struct directly if it can, or does some simple transformation otherwise. Regardless, there is no clear reason to use top here.
https://github.com/cilium/cilium/blob/a67489466c715854c4c472c939d32d35df3fe748/pkg/policy/api/selector.go#L88
That sort of makes sense, thanks.
One potential counter-argument to no longer treating "only marshaler" or "only unmarshaler" types as top is that perhaps the code really only needs to decode, and the shape of the encoded value has nothing to do with the Go type. For example, a Go type representing a user configuration provided as input, so it's only ever JSON unmarshaled, and never marshaled. And then with an UnmarshalJSON to transform the shape - for example if the user's input JSON is an array of key-values and not an object.
I think such a case is perhaps a bit strange, but cue get go treating it as top seems right. Or at least it seems like the safe thing to do, since not treating it as top means we are choosing the wrong shape.
I briefly spoke to @mpvl and he confirmed my suspicion from the previous comment: that the heuristic defaults to treating a type as top if it has either the marshal or unmarshal method since it might only be needed for encoding in one way. I don't think we should change the heuristic there, because falling back to top is safer than falling back to assuming the shape is exactly the same.
That said, this should be the default for the heuristic, and we should support overriding that behavior somehow. That reminds me a bit of the discussion in https://github.com/cue-lang/cue/issues/2678, where the heuristic tries to guess if a field should be required or optional, but the user should be able to override the heuristic if they know better.
And also, this means that my TODO should be deleted either way :) I wrote it with the assumption that maybe we should change the heuristic's behavior, but per the above, we probably should not.
I also have a case where I face this limitation in translating golang model to cue defs: Because some of my types define UnmarshalJSON & UnmarshalYAML to run some additional checks (no custom transformation here), I get a top-value generated instead of the full def I expect.
Is there any plan to provide a way to work around this default "cautious" behavior?
istio (popular service mesh) implementation all objects are impl MarsalJSON 🌵 realy nice have --ignore-marshaler to forced cue get go generate definitions
That said, this should be the default for the heuristic, and we should support overriding that behavior somehow. That reminds me a bit of the discussion in #2678, where the heuristic tries to guess if a field should be required or optional, but the user should be able to override the heuristic if they know better.
@mvdan and I briefly discussed this last week (and it somewhat relates to other conversations I had with @rogpeppe).
The ability to override the default behaviour seems fine to me, with a flag or otherwise.
The behaviour of cue get go with respect to optional, required fields, and indeed defaults seems somewhat less straightforward.
In some situations people might want the Go type for the "shape" it provides, and hence want optional fields in the generated definitions.
Others might want to leverage the behaviour of Go values with respect to the zero value of types. This seems a related but orthogonal aspect (which could be unified in).
And building on that last point, another group might actually want default values that behave like the zero values in Go: yet another aspect.
That said I think the option to vary the behaviour with respect to JSON marshalling interfaces is orthogonal and probably fine.
