pkg/structs: decide how MinFields and MaxFields should behave with required fields

[mvdan]

From https://review.gerrithub.io/c/cue-lang/cue/+/1194515/comments/4fc29f3a_d942865e:

A question remains whether we should count required fields. With this change they are not counted. (The IsOptional method is inappropriately named, a remnant from when we did not have required fields.) Technically, required fields are not fields. So that means this change is correct. On the other hand, if we have, say, a MaxFields of 1, and we have two required fields, we know there is no completed configuration that can ever satisfy this constraint.

I'm inclined to say that we should count required fields for both MinFields and MaxFields, but this can be left for another CL.

[rogpeppe]

I tend to agree that required fields should be considered "present" for the purposes of MinFields and MaxFields. It could be argued that making these operations work on incomplete structs is similar to the fact that

x: <5
x: >7

results in an error even when the concrete value of x is unknown: we know that there will be a concrete value at some point, and that it does not satisfy the constraints.

That said, I'm not sure that this makes any practical difference other than a slightly different error reported (the required field will still result in an error). For example, the following code does not result in an error on vet or export, because the _x is not considered when exporting and this error is considered an "incomplete" error rather than a permanent error.

_x: struct.MinFields(1) & {}

[rogpeppe]

Oh yes, I was thinking that one possible objection might be that it would not be good if struct.Len(s) did not correspond to len([for x in s {x}]), but that's actually OK because one can't iterate over an incomplete struct so both of those will be an error if there are any required fields.

[mvdan]

@mpvl and I discussed this briefly and we both still lean in favor of counting required fields for MinFields and MaxFields for the sake of giving the user a more useful error straight away. Take, for example, this testscript:

exec cue export input.cue

-- input.cue --
package p

import "struct"

#Foo: struct.MaxFields(1) & {
	bar!: string
	baz!: string
}

foo: #Foo & {
}

It fails, complaining about the required fields not being present:

> exec cue export input.cue
[stderr]
foo.bar: field is required but not present:
    ./input.cue:6:2
    ./input.cue:10:6
foo.baz: field is required but not present:
    ./input.cue:7:2
    ./input.cue:10:6

OK, so as a user, I go and fill those in:

exec cue export input.cue

-- input.cue --
package p

import "struct"

#Foo: struct.MaxFields(1) & {
	bar!: string
	baz!: string
}

foo: #Foo & {
	bar: "bar value"
	baz: "baz value"
}

Only to then be greeted by another error:

> exec cue export input.cue
[stderr]
foo: invalid value {bar:string & "bar value",baz:string & "baz value"} (does not satisfy struct.MaxFields(1)): len(fields) > MaxFields(1) (2 > 1):
    ./input.cue:5:7
    ./input.cue:5:24
    ./input.cue:10:6

It would have saved the user time if they were given the underlying MinFields error directly, which cannot be resolved by adding more fields. Telling the user that they need to add a field to resolve an error, and having them spend the time to do that, is non ideal given that we know there will then be another error they run into.