internal/envflag: support int and string flags

As requested by Marcel, it would be useful for CUE_DEBUG to support
a logging level which would be useful for debugging the evaluator.

Similarly, we have talked about transitioning away from a one-off
evalv3 boolean experiment flag into an "eval" version string flag,
meaning that we could set eval=v2, eval=v3, and also reuse it
for future versions like eval=v4.

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

internal/envflag/flag.go

@@ -44,18 +44,16 @@ func Parse[T any](flags *T, env string) error {
 	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 {
 			for _, f := range strings.Split(tagStr, ",") {
 				key, rest, hasRest := strings.Cut(f, ":")
 				switch key {
 				case "default":
-					v, err := strconv.ParseBool(rest)
+					val, err := parseValue(name, field.Type.Kind(), rest)
 					if err != nil {
-						return fmt.Errorf("invalid default bool value for %s: %v", field.Name, err)
+						return err
 					}
-					defaultValue = v
-					fv.Field(i).SetBool(defaultValue)
+					fv.Field(i).Set(reflect.ValueOf(val))
 				case "deprecated":
 					if hasRest {
 						return fmt.Errorf("cannot have a value for deprecated tag")
@@ -83,41 +81,67 @@ func Parse[T any](flags *T, env string) error {
 			// even when the previous env var is empty.
 			continue
 		}
-		name, valueStr, ok := strings.Cut(elem, "=")
-		// "somename" is short for "somename=true" or "somename=1".
-		value := true
-		if ok {
-			v, err := strconv.ParseBool(valueStr)
+		name, valueStr, hasValue := strings.Cut(elem, "=")
+
+		index, knownFlag := indexByName[name]
+		if !knownFlag {
+			// Unknown option, proceed processing options as long as the format is valid.
+			errs = append(errs, fmt.Errorf("unknown flag %q", elem))
+			continue
+		}
+		field := fv.Field(index)
+		var val any
+		if hasValue {
+			var err error
+			val, err = parseValue(name, field.Kind(), valueStr)
 			if err != nil {
-				// Invalid format, return an error immediately.
-				return errInvalid{
-					fmt.Errorf("invalid bool value for %s: %v", name, err),
-				}
+				errs = append(errs, err)
+				continue
 			}
-			value = v
-		}
-		index, ok := indexByName[name]
-		if !ok {
-			// Unknown option, proceed processing options as long as the format
-			// is valid.
-			errs = append(errs, fmt.Errorf("unknown flag %q", elem))
+		} else if field.Kind() == reflect.Bool {
+			// For bools, "somename" is short for "somename=true" or "somename=1".
+			// This mimicks how Go flags work, e.g. -knob is short for -knob=true.
+			val = true
+		} else {
+			// For any other type, a value must be specified.
+			// This mimicks how Go flags work, e.g. -output=path does not allow -output.
+			errs = append(errs, fmt.Errorf("value needed for %s flag %q", field.Kind(), name))
 			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 {
+			// We allow setting deprecated flags to their default value so that
+			// bold explorers will not be penalised for their experimentation.
+			if field.Interface() != val {
 				errs = append(errs, fmt.Errorf("cannot change default value of deprecated flag %q", name))
 			}
 			continue
 		}
 
-		fv.Field(index).SetBool(value)
+		field.Set(reflect.ValueOf(val))
 	}
 	return errors.Join(errs...)
 }
 
+func parseValue(name string, kind reflect.Kind, str string) (any, error) {
+	var val any
+	var err error
+	switch kind {
+	case reflect.Bool:
+		val, err = strconv.ParseBool(str)
+	case reflect.Int:
+		val, err = strconv.Atoi(str)
+	case reflect.String:
+		val = str
+	default:
+		return nil, errInvalid{fmt.Errorf("unsupported kind %s", kind)}
+	}
+	if err != nil {
+		return nil, errInvalid{fmt.Errorf("invalid %s value for %s: %v", kind, name, err)}
+	}
+	return val, nil
+}
+
 // An ErrInvalid indicates a malformed input string.
 var ErrInvalid = errors.New("invalid value")
 

internal/envflag/flag_test.go

@@ -14,6 +14,11 @@ type testFlags struct {
 	DefaultTrue  bool `envflag:"default:true"`
 }
 
+type testTypes struct {
+	StringDefaultFoo string `envflag:"default:foo"`
+	IntDefault5      int    `envflag:"default:5"`
+}
+
 type deprecatedFlags struct {
 	Foo bool `envflag:"deprecated"`
 	Bar bool `envflag:"deprecated,default:true"`
@@ -123,9 +128,44 @@ var tests = []struct {
 		DefaultTrue: true,
 	}, "cannot parse TEST_VAR: unknown flag \"other1\"\nunknown flag \"other2\""),
 }, {
-	testName: "Invalid",
+	testName: "InvalidIntForBool",
 	envVal:   "foo=2,BarBaz=true",
 	test:     invalid(testFlags{DefaultTrue: true}),
+}, {
+	testName: "StringValue",
+	envVal:   "stringdefaultfoo=bar",
+	test: success(testTypes{
+		StringDefaultFoo: "bar",
+		IntDefault5:      5,
+	}),
+}, {
+	testName: "StringEmpty",
+	envVal:   "stringdefaultfoo=",
+	test: success(testTypes{
+		StringDefaultFoo: "",
+		IntDefault5:      5,
+	}),
+}, {
+	testName: "FailureStringAlone",
+	envVal:   "stringdefaultfoo",
+	test: failure(testTypes{
+		StringDefaultFoo: "foo",
+		IntDefault5:      5,
+	}, "cannot parse TEST_VAR: value needed for string flag \"stringdefaultfoo\""),
+}, {
+	testName: "IntValue",
+	envVal:   "intdefault5=123",
+	test: success(testTypes{
+		StringDefaultFoo: "foo",
+		IntDefault5:      123,
+	}),
+}, {
+	testName: "IntEmpty",
+	envVal:   "intdefault5=",
+	test: invalid(testTypes{
+		StringDefaultFoo: "foo",
+		IntDefault5:      5,
+	}),
 }, {
 	testName: "DeprecatedWithFalseDefault",
 	envVal:   "foo=1",