internal/envflag: support deprecated flags

We wish to remove support for disabling the modules experiment
but we do not wish to break users who have set `CUE_EXPERIMENT=modules=1`
to use the experiment.

So change `envflag` to support marking a field as deprecated,
meaning that any attempt to change it from its default value
will result in an error.

Also in passing remove some unnecessary type parameters.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: Ifd552d9212cda1516343d338a6dcc30f1febe150
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200577
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: rogpeppe

internal/envflag/flag.go

@@ -24,6 +24,10 @@ func Init[T any](flags *T, envVar string) error {
 // The struct field tag may contain a default value other than the zero value,
 // such as `envflag:"default:true"` to set a boolean field to true by default.
 //
+// The tag may be marked as deprecated with `envflag:"deprecated"`
+// which will cause Parse to return an error if the user attempts to set
+// its value to anything but the default value.
+//
 // The string may contain a comma-separated list of name=value pairs values
 // representing the boolean fields in the struct type T. If the value is omitted
 // entirely, the value is assumed to be name=true.
@@ -34,25 +38,35 @@ func Init[T any](flags *T, envVar string) error {
 func Parse[T any](flags *T, env string) error {
 	// Collect the field indices and set the default values.
 	indexByName := make(map[string]int)
+	deprecated := make(map[string]bool)
 	fv := reflect.ValueOf(flags).Elem()
 	ft := fv.Type()
 	for i := 0; i < ft.NumField(); i++ {
 		field := ft.Field(i)
+		name := strings.ToLower(field.Name)
 		defaultValue := false
 		if tagStr, ok := field.Tag.Lookup("envflag"); ok {
-			defaultStr, ok := strings.CutPrefix(tagStr, "default:")
-			// TODO: consider panicking for these error types.
-			if !ok {
-				return fmt.Errorf("expected tag like `envflag:\"default:true\"`: %s", tagStr)
-			}
-			v, err := strconv.ParseBool(defaultStr)
-			if err != nil {
-				return fmt.Errorf("invalid default bool value for %s: %v", field.Name, err)
+			for _, f := range strings.Split(tagStr, ",") {
+				key, rest, hasRest := strings.Cut(f, ":")
+				switch key {
+				case "default":
+					v, err := strconv.ParseBool(rest)
+					if err != nil {
+						return fmt.Errorf("invalid default bool value for %s: %v", field.Name, err)
+					}
+					defaultValue = v
+					fv.Field(i).SetBool(defaultValue)
+				case "deprecated":
+					if hasRest {
+						return fmt.Errorf("cannot have a value for deprecated tag")
+					}
+					deprecated[name] = true
+				default:
+					return fmt.Errorf("unknown envflag tag %q", f)
+				}
 			}
-			defaultValue = v
-			fv.Field(i).SetBool(defaultValue)
 		}
-		indexByName[strings.ToLower(field.Name)] = i
+		indexByName[name] = i
 	}
 
 	if env == "" {
@@ -80,6 +94,16 @@ func Parse[T any](flags *T, env string) error {
 			errs = append(errs, fmt.Errorf("unknown %s", elem))
 			continue
 		}
+		if deprecated[name] {
+			// We allow setting deprecated flags to their default value so
+			// that bold explorers will not be penalised for their
+			// experimentation.
+			if fv.Field(index).Bool() != value {
+				errs = append(errs, fmt.Errorf("cannot change default value of deprecated flag %q", name))
+			}
+			continue
+		}
+
 		fv.Field(index).SetBool(value)
 	}
 	return errors.Join(errs...)

internal/envflag/flag_test.go

@@ -14,6 +14,11 @@ type testFlags struct {
 	DefaultTrue  bool `envflag:"default:true"`
 }
 
+type deprecatedFlags struct {
+	Foo bool `envflag:"deprecated"`
+	Bar bool `envflag:"deprecated,default:true"`
+}
+
 func success[T comparable](want T) func(t *testing.T) {
 	return func(t *testing.T) {
 		var x T
@@ -54,7 +59,7 @@ var tests = []struct {
 }, {
 	testName: "Unknown",
 	envVal:   "ratchet",
-	test: failure[testFlags](testFlags{DefaultTrue: true},
+	test: failure(testFlags{DefaultTrue: true},
 		"cannot parse TEST_VAR: unknown ratchet"),
 }, {
 	testName: "Set",
@@ -73,7 +78,7 @@ var tests = []struct {
 }, {
 	testName: "SetWithUnknown",
 	envVal:   "foo,other",
-	test: failure[testFlags](testFlags{
+	test: failure(testFlags{
 		Foo:         true,
 		DefaultTrue: true,
 	}, "cannot parse TEST_VAR: unknown other"),
@@ -108,6 +113,30 @@ var tests = []struct {
 	testName: "Invalid",
 	envVal:   "foo=2,BarBaz=true",
 	test:     invalid(testFlags{DefaultTrue: true}),
+}, {
+	testName: "DeprecatedWithFalseDefault",
+	envVal:   "foo=1",
+	test: failure(deprecatedFlags{
+		Bar: true,
+	}, `cannot parse TEST_VAR: cannot change default value of deprecated flag "foo"`),
+}, {
+	testName: "DeprecatedNoopWhenSameAndFalseDefault",
+	envVal:   "foo=false",
+	test: success(deprecatedFlags{
+		Bar: true,
+	}),
+}, {
+	testName: "DeprecatedWithTrueDefault",
+	envVal:   "bar=0",
+	test: failure(deprecatedFlags{
+		Bar: true,
+	}, `cannot parse TEST_VAR: cannot change default value of deprecated flag "bar"`),
+}, {
+	testName: "DeprecatedNoopWhenSameAndTrueDefault",
+	envVal:   "bar=1",
+	test: success(deprecatedFlags{
+		Bar: true,
+	}),
 }}
 
 func TestInit(t *testing.T) {