Struct experiment

[deankarn]

PR

This PR does the following:

• Reverts Feature: nested struct validation #1122
• Re-implements struct level validations in a different way which also support ors etc.. which previous implementation did not.
• Adds special case to ignore required validation on non-pointer structs to preserve pre-struct level tag validation support.
  • Added new WithRequiredStructEnabled option to opt-in to this new behaviour, that will become the default in the next major version.

[coveralls]

coverage: 73.843% (-0.03%) from 73.872% when pulling e912ff3 on struct-experiment into 3094d59 on master.

[MysteriousPotato]

Would you consider adding an opt-in option to enable the required tag for structs?

This would prevent users that are already using the required tag on non-pointer structs (such as myself) from having to refactor their codebase as well as allowing users to use it without having to wait for the next major version.

Something like:

type Validate struct {
	//...
	requiredStructEnabled bool // new field
}

type ValidateOpt func(*Validate)

// Option to enable required tag on nested structs
func EnableRequiredStruct(v *Validate) {
	v.requiredStructEnabled = true
}

func New(opts ...ValidateOpt) *Validate {
	//...
	v := &Validate{
		//...
	}

	for _, opt := range opts {
		opt(v)
	}
	//...
}

func (v *validate) traverseField(ctx context.Context, parent reflect.Value, current reflect.Value, ns []byte, structNs []byte, cf *cField, ct *cTag) {
	//...
	if ct != nil && ct.tag == requiredTag && !v.v.requiredStructEnabled {
		ct = ct.next
	}
	//...
}

[deankarn]

Yep @MysteriousPotato totally an option :)

was meaning to ask what u think of this alt approach that will support or tag etc on structs.

will be a few days before I can finish adding the tests and docs back and then add that option.

[MysteriousPotato]

@deankarn I think this is clearly an improvement over my implementation.

Besides adding or support (which I totally overlooked) it also, among other things, makes the flow of traverseField easier to follow IMO.

One thing though. Looking a bit more into it, I think this may have changed the behavior of structonly for non-struct types where validateStruct would get called.

Not that it's a problem in and of itself since it doesn't make any sense to do that but considering the recent events, maybe adding some kind of check before calling validateStruct or returning an insightful error would be a good idea just in case?

Unrelated to this MR:
While perusing the code, I noticed the structonly tag fails for nil struct pointers. Is this behavior intended?

[deankarn]

@MysteriousPotato

One thing though. Looking a bit more into it, I think this may have changed the behavior of structonly for non-struct types where validateStruct would get called.

Nice catch agreed, handled in this commit to preserve old behaviour.
771b7bf

[phoenix2x]

What is the way to set requiredStructEnabled to 'false'? If this option will be supported by the lib, I assume there should be a way to disable it as well:)

[deankarn]

But it is off by default @phoenix2x

If you mean it will be the default and a way to turn off in v11+, there will not be an option for that or way to opt-out and people will have to fix their bad code using required incorrectly.

[phoenix2x]

That make sense, thank you for the explanation:)