ISSUE-210 - Add Required property to indicate a config registration c…

[BowlOfSoup]

Add Required property to indicate a config registration can't be omitted or nil (+ tests)

References

Issue(s): #210

Description

Changes in this PR make it possible to, when registering a config value, to indicate that the value can not be nil and also validates omitting the complete config line. Both will trigger a validation message.

Possible drawbacks/suggestions

Imo we should provide a 'constructor' like config.NewEntry() in which we can set default values to prevent this change to the Entry struct to be a breaking change for people not using keyed initialization on structs. This was discussed and marked as not breaking.

Proposal for constructor NewEntry():

package config

type EntryOpts struct {
    AuthorizedValues []any
    IsSlice bool
    Required bool
}

func NewEntry(value any, type reflect.Kind, opts EntryOpts) *Entry {
    return &Entry{
        Value: "default value",
	Type: reflect.String,
	IsSlice: opts.IsSlice,
	AuthorizedValues: opts.AutorizedValues,
	Required: opts.Required
    }
}

// use it like this
config.Register("customCategory.customEntry", config.NewEntry(
    Value: "default value",
    Type: reflect.String,
    EntryOpts{
        AuthorizedValues: []any{},
        IsSlice: false,
        Required: true,
    }
)

[System-Glitch]

This doesn't look like a breaking change to me if we drop the "empty" validation. It would only be breaking for codebases not always using keyed struct fields, which is very rare and usually not recommended anyway. The behavior for entries without Required: true stays the same as before.

I would prefer not adding NewEntry().

[System-Glitch]

Very minor adjustments to make, otherwise LGTM! 👍🏻

[System-Glitch]

Suggested change

	if e.Required && (!v.IsValid() || e.Value == nil) {
	if e.Required && (!v.IsValid() || v.IsNil()) {

[BowlOfSoup]

IsNil reports whether its argument v is nil. The argument must be a chan, func, interface, map, pointer, or slice value; if it is not, IsNil panics.

I've tested this when you first suggested it, but it will not work if the value is not one of the above. This happens e.g. when an inputted value is actually nil.

So !v.IsValid() || e.Value == nil would cover the functionality we want.

If it's not a value (IsValid) it will return true. If it is, it will check for explicit nil.

[System-Glitch]

You're right. Here is what I suggest to cover the scenario I showed in the playground link earlier.

Suggested change

	if e.Required && (!v.IsValid() || e.Value == nil) {
	if e.Required && (!v.IsValid() || e.Value == nil || (v.Kind() == reflect.Pointer && v.IsNil())) {

Updated playground link

[coveralls]

Pull Request Test Coverage Report for Build 9608948098

Details

• 11 of 11 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 97.335%

---

Totals
Change from base Build 9594754702:	0.001%
Covered Lines:	6100
Relevant Lines:	6267

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 9610067852

Details

• 11 of 11 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 97.335%

---

Totals
Change from base Build 9594754702:	0.001%
Covered Lines:	6100
Relevant Lines:	6267

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 9610325654

Details

• 11 of 11 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 97.335%

---

Totals
Change from base Build 9594754702:	0.001%
Covered Lines:	6100
Relevant Lines:	6267

---

💛 - Coveralls

[System-Glitch]

Thank you very much! 🎉