From 6614bbf94d71a9e77fadc0ef279a1051bbfc5c7c Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 8 Apr 2026 12:39:36 -0500 Subject: [PATCH 1/5] Implement env and required struct tag support The runtime CLI has been using env:"VAR_NAME" and required:"true" struct tags for a while now, but mflags never actually honored them. They were silently ignored. With strict tag validation coming in MIR-985, upgrading mflags in the runtime repo would surface these as errors, so we need to teach mflags what they mean first. env tags populate a flag's default from an environment variable, giving us layered precedence: default < env < CLI arg. required tags validate after parsing that the flag or positional was given a value (via CLI or env). Both tags work on flags and positionals. Help output now shows (env: VAR_NAME) annotations, and the JSON help doc includes envVar and required fields. A new ErrRequired sentinel lets callers distinguish required-validation failures from other parse errors. --- help_doc.go | 50 ++++++---- mflags.go | 121 ++++++++++++++++++++-- mflags_test.go | 265 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 406 insertions(+), 30 deletions(-) diff --git a/help_doc.go b/help_doc.go index 7b587be..f8cf6de 100644 --- a/help_doc.go +++ b/help_doc.go @@ -36,21 +36,25 @@ type ExampleDoc struct { // FlagDoc describes a single flag in the help document. type FlagDoc struct { - Name string `json:"name"` - Short string `json:"short"` - Type string `json:"type"` - Default string `json:"default,omitempty"` - Usage string `json:"usage"` - Group string `json:"group,omitempty"` - IsBool bool `json:"isBool,omitempty"` - Choices []string `json:"choices,omitempty"` + Name string `json:"name"` + Short string `json:"short"` + Type string `json:"type"` + Default string `json:"default,omitempty"` + Usage string `json:"usage"` + Group string `json:"group,omitempty"` + IsBool bool `json:"isBool,omitempty"` + Choices []string `json:"choices,omitempty"` + EnvVar string `json:"envVar,omitempty"` + Required bool `json:"required,omitempty"` } // PositionalDoc describes a positional argument in the help document. type PositionalDoc struct { - Name string `json:"name"` - Usage string `json:"usage"` - Type string `json:"type"` + Name string `json:"name"` + Usage string `json:"usage"` + Type string `json:"type"` + EnvVar string `json:"envVar,omitempty"` + Required bool `json:"required,omitempty"` } // FlagSetDoc describes a standalone FlagSet's help information. @@ -74,13 +78,15 @@ func (f *FlagSet) HelpDoc() *FlagSetDoc { f.VisitAll(func(flag *Flag) { fd := FlagDoc{ - Name: flag.Name, - Type: flag.Value.Type(), - Default: flag.DefValue, - Usage: flag.Usage, - Group: flag.Group, - IsBool: flag.Value.IsBool(), - Choices: []string{}, + Name: flag.Name, + Type: flag.Value.Type(), + Default: flag.DefValue, + Usage: flag.Usage, + Group: flag.Group, + IsBool: flag.Value.IsBool(), + Choices: []string{}, + EnvVar: flag.EnvVar, + Required: flag.Required, } if flag.Short != 0 { fd.Short = string(flag.Short) @@ -97,9 +103,11 @@ func (f *FlagSet) HelpDoc() *FlagSetDoc { for _, pf := range f.GetPositionalFields() { doc.PositionalArgs = append(doc.PositionalArgs, PositionalDoc{ - Name: pf.Name, - Usage: pf.Usage, - Type: pf.Type.String(), + Name: pf.Name, + Usage: pf.Usage, + Type: pf.Type.String(), + EnvVar: pf.EnvVar, + Required: pf.Required, }) } diff --git a/mflags.go b/mflags.go index d5a9c2f..81b0911 100644 --- a/mflags.go +++ b/mflags.go @@ -3,6 +3,7 @@ package mflags import ( "errors" "fmt" + "os" "reflect" "strconv" "strings" @@ -16,14 +17,18 @@ var ( ErrInvalidChoice = errors.New("invalid choice") ErrHelp = errors.New("help requested") ErrShowHelp = errors.New("show help") // Return from Command.Run to trigger help display + ErrRequired = errors.New("required flag not provided") ) // PositionalField represents a positional argument field type PositionalField struct { - Name string // Field name (e.g., "Command", "Target") - Usage string // Usage description for help output - Value reflect.Value // The reflect.Value of the field - Type reflect.Type // The type of the field + Name string // Field name (e.g., "Command", "Target") + Usage string // Usage description for help output + Value reflect.Value // The reflect.Value of the field + Type reflect.Type // The type of the field + EnvVar string // environment variable name (from env:"..." struct tag) + Required bool // whether this positional must be provided + HasValue bool // true if value was set by env var or CLI arg } type FlagSet struct { @@ -41,6 +46,9 @@ type FlagSet struct { disableAutoHelp bool // If true, don't automatically handle -h/--help in Parse currentGroup string // ambient group name set by FromStruct options or Group() calls groupOrder []string // ordered list of distinct group names (insertion order) + setFlags map[string]bool // flags explicitly set during Parse + requiredFlags []string // flag names marked required:"true" + requiredPos []int // positional indices marked required:"true" } type Flag struct { @@ -50,6 +58,9 @@ type Flag struct { Value Value DefValue string Group string // group name for help rendering; empty = default "Options:" + EnvVar string // environment variable name (from env:"..." struct tag) + Required bool // whether this flag must be provided + HasValue bool // true if value was set by env var or CLI arg } type Value interface { @@ -865,6 +876,7 @@ func NewFlagSet(name string) *FlagSet { flags: make(map[string]*Flag), shortMap: make(map[rune]*Flag), posFields: make(map[int]*PositionalField), + setFlags: make(map[string]bool), } } @@ -1424,6 +1436,7 @@ func (f *FlagSet) Parse(arguments []string) error { if err := setFieldValue(field.Value, f.args[pos]); err != nil { return fmt.Errorf("invalid value for position %d: %v", pos, err) } + field.HasValue = true } } @@ -1449,6 +1462,38 @@ func (f *FlagSet) Parse(arguments []string) error { *f.unknownField = f.unknownFlags } + if err := f.validateRequired(); err != nil { + return err + } + + return nil +} + +// validateRequired checks that all flags and positionals marked required:"true" +// have been provided a value (via CLI arg or env var). +func (f *FlagSet) validateRequired() error { + var missing []string + + for _, name := range f.requiredFlags { + if flag, ok := f.flags[name]; ok { + if !flag.HasValue { + missing = append(missing, "--"+name) + } + } + } + + for _, pos := range f.requiredPos { + if field, ok := f.posFields[pos]; ok { + if !field.HasValue { + missing = append(missing, field.Name) + } + } + } + + if len(missing) > 0 { + return fmt.Errorf("%w: %s", ErrRequired, strings.Join(missing, ", ")) + } + return nil } @@ -1492,6 +1537,9 @@ func (f *FlagSet) parseLongFlag(name string, args []string, index *int) (bool, e return false, fmt.Errorf("%w: --%s: %v", ErrInvalidValue, name, err) } + flag.HasValue = true + f.setFlags[flag.Name] = true + return true, nil } @@ -1515,6 +1563,8 @@ func (f *FlagSet) parseShortFlags(shortFlags string, args []string, index *int) if err := flag.Value.Set("true"); err != nil { return fmt.Errorf("%w: -%c: %v", ErrInvalidValue, r, err) } + flag.HasValue = true + f.setFlags[flag.Name] = true } else { // Check if there are more characters after this flag if i < len(runes)-1 { @@ -1529,6 +1579,8 @@ func (f *FlagSet) parseShortFlags(shortFlags string, args []string, index *int) if err := flag.Value.Set(value); err != nil { return fmt.Errorf("%w: -%c: %v", ErrInvalidValue, r, err) } + flag.HasValue = true + f.setFlags[flag.Name] = true break } else if *index+1 < len(args) { value := args[*index+1] @@ -1536,6 +1588,8 @@ func (f *FlagSet) parseShortFlags(shortFlags string, args []string, index *int) if err := flag.Value.Set(value); err != nil { return fmt.Errorf("%w: -%c: %v", ErrInvalidValue, r, err) } + flag.HasValue = true + f.setFlags[flag.Name] = true } else { return fmt.Errorf("%w: -%c", ErrMissingValue, r) } @@ -1858,11 +1912,30 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { if posUsage == "" { posUsage = field.Tag.Get("description") } + posEnvVar := field.Tag.Get("env") + posRequired := field.Tag.Get("required") == "true" + posHasValue := false + + // Environment variable sets the positional default + if posEnvVar != "" { + if envVal, ok := os.LookupEnv(posEnvVar); ok { + setFieldValue(fieldValue, envVal) + posHasValue = true + } + } + f.posFields[pos] = &PositionalField{ - Name: field.Name, - Usage: posUsage, - Value: fieldValue, - Type: field.Type, + Name: field.Name, + Usage: posUsage, + Value: fieldValue, + Type: field.Type, + EnvVar: posEnvVar, + Required: posRequired, + HasValue: posHasValue, + } + + if posRequired { + f.requiredPos = append(f.requiredPos, pos) } } continue // Don't process position field as a flag @@ -1902,6 +1975,18 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { } defaultValue := field.Tag.Get("default") + envVar := field.Tag.Get("env") + required := field.Tag.Get("required") == "true" + + // Environment variable overrides default + hasEnvValue := false + if envVar != "" { + if envVal, ok := os.LookupEnv(envVar); ok { + defaultValue = envVal + hasEnvValue = true + } + } + usage := field.Tag.Get("usage") if usage == "" { usage = field.Tag.Get("description") @@ -1910,6 +1995,10 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { } } + if required { + f.requiredFlags = append(f.requiredFlags, longName) + } + // Register the flag based on field type switch field.Type.Kind() { case reflect.Bool: @@ -2076,6 +2165,13 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { f.Var(&uint64PtrValue{p: p}, longName, short, usage) } } + + // Set env/required metadata on the registered flag + if flag, ok := f.flags[longName]; ok { + flag.EnvVar = envVar + flag.Required = required + flag.HasValue = hasEnvValue + } } return nil @@ -2103,6 +2199,9 @@ func formatFlagLine(flag *Flag) string { if flag.DefValue != "" && flag.DefValue != "false" && flag.DefValue != "0" { line += fmt.Sprintf(" (default: %s)", flag.DefValue) } + if flag.EnvVar != "" { + line += fmt.Sprintf(" (env: %s)", flag.EnvVar) + } return line } return flagStr @@ -2202,7 +2301,11 @@ func (f *FlagSet) ShowHelp() { if field, ok := f.posFields[i]; ok { argStr := fmt.Sprintf(" <%s>", strings.ToLower(field.Name)) if field.Usage != "" { - fmt.Printf("%-30s %s\n", argStr, field.Usage) + line := fmt.Sprintf("%-30s %s", argStr, field.Usage) + if field.EnvVar != "" { + line += fmt.Sprintf(" (env: %s)", field.EnvVar) + } + fmt.Println(line) } else { fmt.Println(argStr) } diff --git a/mflags_test.go b/mflags_test.go index de58a33..4154f49 100644 --- a/mflags_test.go +++ b/mflags_test.go @@ -3645,3 +3645,268 @@ func TestShowHelpWithGroups(t *testing.T) { assert.Contains(t, output, "-v, --verbose") assert.Contains(t, output, "-o, --output") } + +// --- env tag tests --- + +func TestFromStructEnvFlag(t *testing.T) { + type Config struct { + Name string `long:"name" env:"TEST_MFLAGS_NAME"` + } + + t.Run("picks up env var", func(t *testing.T) { + t.Setenv("TEST_MFLAGS_NAME", "from-env") + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.Equal(t, "from-env", config.Name) + }) + + t.Run("env overrides default", func(t *testing.T) { + type ConfigWithDefault struct { + Name string `long:"name" default:"hardcoded" env:"TEST_MFLAGS_NAME"` + } + t.Setenv("TEST_MFLAGS_NAME", "from-env") + config := &ConfigWithDefault{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.Equal(t, "from-env", config.Name) + }) + + t.Run("CLI arg overrides env", func(t *testing.T) { + t.Setenv("TEST_MFLAGS_NAME", "from-env") + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{"--name", "from-cli"}) + assert.NoError(t, err) + assert.Equal(t, "from-cli", config.Name) + }) + + t.Run("unset env uses default", func(t *testing.T) { + type ConfigWithDefault struct { + Name string `long:"name" default:"hardcoded" env:"TEST_MFLAGS_NAME_UNSET"` + } + config := &ConfigWithDefault{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.Equal(t, "hardcoded", config.Name) + }) + + t.Run("env with int flag", func(t *testing.T) { + type ConfigInt struct { + Count int `long:"count" env:"TEST_MFLAGS_COUNT"` + } + t.Setenv("TEST_MFLAGS_COUNT", "42") + config := &ConfigInt{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.Equal(t, 42, config.Count) + }) + + t.Run("env with bool flag", func(t *testing.T) { + type ConfigBool struct { + Verbose bool `long:"verbose" env:"TEST_MFLAGS_VERBOSE"` + } + t.Setenv("TEST_MFLAGS_VERBOSE", "true") + config := &ConfigBool{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.True(t, config.Verbose) + }) +} + +func TestFromStructEnvPositional(t *testing.T) { + type Config struct { + Target string `position:"0" env:"TEST_MFLAGS_TARGET"` + } + + t.Run("picks up env var", func(t *testing.T) { + t.Setenv("TEST_MFLAGS_TARGET", "from-env") + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.Equal(t, "from-env", config.Target) + }) + + t.Run("CLI arg overrides env", func(t *testing.T) { + t.Setenv("TEST_MFLAGS_TARGET", "from-env") + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{"from-cli"}) + assert.NoError(t, err) + assert.Equal(t, "from-cli", config.Target) + }) +} + +func TestEnvHelpText(t *testing.T) { + type Config struct { + Name string `long:"name" env:"MY_APP_NAME" usage:"Application name"` + } + + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + fs.WriteFlagHelp() + + w.Close() + os.Stdout = old + + var buf bytes.Buffer + io.Copy(&buf, r) + output := buf.String() + + assert.Contains(t, output, "(env: MY_APP_NAME)") +} + +// --- required tag tests --- + +func TestFromStructRequiredFlag(t *testing.T) { + type Config struct { + Name string `long:"name" required:"true"` + } + + t.Run("errors when not provided", func(t *testing.T) { + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.Error(t, err) + assert.ErrorIs(t, err, ErrRequired) + assert.Contains(t, err.Error(), "--name") + }) + + t.Run("succeeds when provided via CLI", func(t *testing.T) { + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{"--name", "hello"}) + assert.NoError(t, err) + assert.Equal(t, "hello", config.Name) + }) + + t.Run("succeeds when provided via env", func(t *testing.T) { + type ConfigEnv struct { + Name string `long:"name" required:"true" env:"TEST_MFLAGS_REQ_NAME"` + } + t.Setenv("TEST_MFLAGS_REQ_NAME", "from-env") + config := &ConfigEnv{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.Equal(t, "from-env", config.Name) + }) + + t.Run("multiple missing reports all", func(t *testing.T) { + type ConfigMulti struct { + Name string `long:"name" required:"true"` + Host string `long:"host" required:"true"` + } + config := &ConfigMulti{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.Error(t, err) + assert.ErrorIs(t, err, ErrRequired) + assert.Contains(t, err.Error(), "--name") + assert.Contains(t, err.Error(), "--host") + }) +} + +func TestFromStructRequiredPositional(t *testing.T) { + type Config struct { + Command string `position:"0" required:"true"` + } + + t.Run("errors when not provided", func(t *testing.T) { + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.Error(t, err) + assert.ErrorIs(t, err, ErrRequired) + assert.Contains(t, err.Error(), "Command") + }) + + t.Run("succeeds when provided", func(t *testing.T) { + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{"deploy"}) + assert.NoError(t, err) + assert.Equal(t, "deploy", config.Command) + }) + + t.Run("succeeds when provided via env", func(t *testing.T) { + type ConfigEnv struct { + Command string `position:"0" required:"true" env:"TEST_MFLAGS_CMD"` + } + t.Setenv("TEST_MFLAGS_CMD", "from-env") + config := &ConfigEnv{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.Equal(t, "from-env", config.Command) + }) +} + +func TestRequiredBoolFlag(t *testing.T) { + type Config struct { + Accept bool `long:"accept" required:"true"` + } + + t.Run("errors when not provided", func(t *testing.T) { + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.Error(t, err) + assert.ErrorIs(t, err, ErrRequired) + }) + + t.Run("succeeds when provided", func(t *testing.T) { + config := &Config{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{"--accept"}) + assert.NoError(t, err) + assert.True(t, config.Accept) + }) +} From 616f9ef9931c9f7dab77503de163bb4e0d3e0f74 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 8 Apr 2026 12:44:04 -0500 Subject: [PATCH 2/5] Handle setFieldValue error for positional env vars The env var path for positionals was ignoring the error from setFieldValue, which would silently swallow parse failures (e.g. "abc" for an int positional). Now we propagate the error. --- mflags.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mflags.go b/mflags.go index 81b0911..6b40f5d 100644 --- a/mflags.go +++ b/mflags.go @@ -1919,7 +1919,9 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { // Environment variable sets the positional default if posEnvVar != "" { if envVal, ok := os.LookupEnv(posEnvVar); ok { - setFieldValue(fieldValue, envVal) + if err := setFieldValue(fieldValue, envVal); err != nil { + return fmt.Errorf("invalid value for env var %s: %w", posEnvVar, err) + } posHasValue = true } } From 5d5b28f07b691cc00b45daae5ba7a3da8c4b87b4 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 8 Apr 2026 16:00:40 -0500 Subject: [PATCH 3/5] Apply env values through Flag.Value.Set for validation and type coverage The previous approach routed env values through the defaultValue string, which meant parse errors were silently swallowed and pointer/slice types never received the value. Now env values are applied via flag.Value.Set() after registration, which validates the value and works uniformly across all types. Also documents env and required tags in FromStruct's doc comment. --- mflags.go | 24 +++++++++++++----------- mflags_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/mflags.go b/mflags.go index 6b40f5d..96c1afa 100644 --- a/mflags.go +++ b/mflags.go @@ -1817,6 +1817,8 @@ func InGroup(name string) FromStructOption { // - `long:"name"` - long flag name (defaults to lowercase field name) // - `short:"x"` - short flag name (single character) // - `default:"value"` - default value for the flag +// - `env:"VAR_NAME"` - populate default from an environment variable (overrides default, overridden by CLI) +// - `required:"true"` - return a parse error if the flag/positional wasn't provided // - `usage:"description"` - usage description // - `description:"description"` - alternate usage description // - `choice:"value"` - constrain string field to specific values (can be repeated for multiple choices) @@ -1980,15 +1982,6 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { envVar := field.Tag.Get("env") required := field.Tag.Get("required") == "true" - // Environment variable overrides default - hasEnvValue := false - if envVar != "" { - if envVal, ok := os.LookupEnv(envVar); ok { - defaultValue = envVal - hasEnvValue = true - } - } - usage := field.Tag.Get("usage") if usage == "" { usage = field.Tag.Get("description") @@ -2168,11 +2161,20 @@ func (f *FlagSet) FromStruct(v any, opts ...FromStructOption) error { } } - // Set env/required metadata on the registered flag + // Set env/required metadata on the registered flag, and apply + // env var value through the flag's Value.Set path so it gets + // validated and works for all types including pointers and slices. if flag, ok := f.flags[longName]; ok { flag.EnvVar = envVar flag.Required = required - flag.HasValue = hasEnvValue + if envVar != "" { + if envVal, ok := os.LookupEnv(envVar); ok { + if err := flag.Value.Set(envVal); err != nil { + return fmt.Errorf("invalid value for env var %s: %w", envVar, err) + } + flag.HasValue = true + } + } } } diff --git a/mflags_test.go b/mflags_test.go index 4154f49..2c8c676 100644 --- a/mflags_test.go +++ b/mflags_test.go @@ -3716,6 +3716,33 @@ func TestFromStructEnvFlag(t *testing.T) { assert.Equal(t, 42, config.Count) }) + t.Run("env with invalid int errors", func(t *testing.T) { + type ConfigInt struct { + Count int `long:"count" env:"TEST_MFLAGS_COUNT"` + } + t.Setenv("TEST_MFLAGS_COUNT", "abc") + config := &ConfigInt{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.Error(t, err) + assert.Contains(t, err.Error(), "TEST_MFLAGS_COUNT") + }) + + t.Run("env with pointer type", func(t *testing.T) { + type ConfigPtr struct { + Name *string `long:"name" env:"TEST_MFLAGS_PTR_NAME"` + } + t.Setenv("TEST_MFLAGS_PTR_NAME", "from-env") + config := &ConfigPtr{} + fs := NewFlagSet("test") + err := fs.FromStruct(config) + assert.NoError(t, err) + err = fs.Parse([]string{}) + assert.NoError(t, err) + assert.NotNil(t, config.Name) + assert.Equal(t, "from-env", *config.Name) + }) + t.Run("env with bool flag", func(t *testing.T) { type ConfigBool struct { Verbose bool `long:"verbose" env:"TEST_MFLAGS_VERBOSE"` From cad95e7b34521d0dfd3f287737cdc8dd6e97f0c2 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 8 Apr 2026 16:03:44 -0500 Subject: [PATCH 4/5] Register env and required as known struct tags After rebasing on MIR-985's strict tag validation, env and required need to be added to knownTags so FromStruct doesn't reject them. Updates the test that was asserting they were unknown. --- mflags.go | 2 ++ mflags_test.go | 16 +++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/mflags.go b/mflags.go index 96c1afa..577dd52 100644 --- a/mflags.go +++ b/mflags.go @@ -1674,6 +1674,8 @@ var knownTags = map[string]bool{ "long": true, "short": true, "default": true, + "env": true, + "required": true, "usage": true, "description": true, "choice": true, diff --git a/mflags_test.go b/mflags_test.go index 2c8c676..992ba39 100644 --- a/mflags_test.go +++ b/mflags_test.go @@ -872,15 +872,25 @@ func TestFromStructRejectsUnknownTags(t *testing.T) { }) t.Run("multiple unknown tags across fields", func(t *testing.T) { + type Opts struct { + Name string `long:"name" bogus:"yes"` + Port int `long:"port" nope:"true"` + } + fs := NewFlagSet("test") + err := fs.FromStruct(&Opts{}) + assert.Error(t, err) + assert.Contains(t, err.Error(), `"bogus"`) + assert.Contains(t, err.Error(), `"nope"`) + }) + + t.Run("env and required are recognized tags", func(t *testing.T) { type Opts struct { Name string `long:"name" env:"MY_NAME"` Port int `long:"port" required:"true"` } fs := NewFlagSet("test") err := fs.FromStruct(&Opts{}) - assert.Error(t, err) - assert.Contains(t, err.Error(), `"env"`) - assert.Contains(t, err.Error(), `"required"`) + assert.NoError(t, err) }) t.Run("non-mflags tag like json", func(t *testing.T) { From c790ea42d12fc9993ec018224c09b68d71275de8 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 8 Apr 2026 16:06:33 -0500 Subject: [PATCH 5/5] Remove unused setFlags map Was populated during Parse but never read. The HasValue bool on Flag handles required validation directly, making this redundant. --- mflags.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/mflags.go b/mflags.go index 577dd52..bb24b58 100644 --- a/mflags.go +++ b/mflags.go @@ -46,7 +46,6 @@ type FlagSet struct { disableAutoHelp bool // If true, don't automatically handle -h/--help in Parse currentGroup string // ambient group name set by FromStruct options or Group() calls groupOrder []string // ordered list of distinct group names (insertion order) - setFlags map[string]bool // flags explicitly set during Parse requiredFlags []string // flag names marked required:"true" requiredPos []int // positional indices marked required:"true" } @@ -876,7 +875,6 @@ func NewFlagSet(name string) *FlagSet { flags: make(map[string]*Flag), shortMap: make(map[rune]*Flag), posFields: make(map[int]*PositionalField), - setFlags: make(map[string]bool), } } @@ -1538,7 +1536,7 @@ func (f *FlagSet) parseLongFlag(name string, args []string, index *int) (bool, e } flag.HasValue = true - f.setFlags[flag.Name] = true + return true, nil } @@ -1564,7 +1562,7 @@ func (f *FlagSet) parseShortFlags(shortFlags string, args []string, index *int) return fmt.Errorf("%w: -%c: %v", ErrInvalidValue, r, err) } flag.HasValue = true - f.setFlags[flag.Name] = true + } else { // Check if there are more characters after this flag if i < len(runes)-1 { @@ -1580,7 +1578,7 @@ func (f *FlagSet) parseShortFlags(shortFlags string, args []string, index *int) return fmt.Errorf("%w: -%c: %v", ErrInvalidValue, r, err) } flag.HasValue = true - f.setFlags[flag.Name] = true + break } else if *index+1 < len(args) { value := args[*index+1] @@ -1589,7 +1587,7 @@ func (f *FlagSet) parseShortFlags(shortFlags string, args []string, index *int) return fmt.Errorf("%w: -%c: %v", ErrInvalidValue, r, err) } flag.HasValue = true - f.setFlags[flag.Name] = true + } else { return fmt.Errorf("%w: -%c", ErrMissingValue, r) }