move construction logic out of parse.go into construct.go

v2/construct.go

@@ -0,0 +1,317 @@
+package arg
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"reflect"
+	"strings"
+)
+
+// Argument represents a command line argument
+type Argument struct {
+	dest        path
+	field       reflect.StructField // the struct field from which this option was created
+	long        string              // the --long form for this option, or empty if none
+	short       string              // the -s short form for this option, or empty if none
+	cardinality cardinality         // determines how many tokens will be present (possible values: zero, one, multiple)
+	required    bool                // if true, this option must be present on the command line
+	positional  bool                // if true, this option will be looked for in the positional flags
+	separate    bool                // if true, each slice and map entry will have its own --flag
+	help        string              // the help text for this option
+	env         string              // the name of the environment variable for this option, or empty for none
+	defaultVal  string              // default value for this option
+	placeholder string              // name of the data in help
+}
+
+// Command represents a named subcommand, or the top-level command
+type Command struct {
+	name        string
+	help        string
+	dest        path
+	args        []*Argument
+	subcommands []*Command
+	parent      *Command
+}
+
+// Parser represents a set of command line options with destination values
+type Parser struct {
+	cmd      *Command      // the top-level command
+	root     reflect.Value // destination struct to fill will values
+	version  string        // version from the argument struct
+	prologue string        // prologue for help text (from the argument struct)
+	epilogue string        // epilogue for help text (from the argument struct)
+
+	// the following fields are updated during processing of command line arguments
+	leaf            *Command           // the subcommand we processed last
+	accumulatedArgs []*Argument        // concatenation of the leaf subcommand's arguments plus all ancestors' arguments
+	seen            map[*Argument]bool // the arguments we encountered while processing command line arguments
+}
+
+// Versioned is the interface that the destination struct should implement to
+// make a version string appear at the top of the help message.
+type Versioned interface {
+	// Version returns the version string that will be printed on a line by itself
+	// at the top of the help message.
+	Version() string
+}
+
+// Described is the interface that the destination struct should implement to
+// make a description string appear at the top of the help message.
+type Described interface {
+	// Description returns the string that will be printed on a line by itself
+	// at the top of the help message.
+	Description() string
+}
+
+// Epilogued is the interface that the destination struct should implement to
+// add an epilogue string at the bottom of the help message.
+type Epilogued interface {
+	// Epilogue returns the string that will be printed on a line by itself
+	// at the end of the help message.
+	Epilogue() string
+}
+
+// the ParserOption interface matches options for the parser constructor
+type ParserOption interface {
+	parserOption()
+}
+
+type programNameParserOption struct {
+	s string
+}
+
+func (programNameParserOption) parserOption() {}
+
+// WithProgramName overrides the name of the program as displayed in help test
+func WithProgramName(name string) ParserOption {
+	return programNameParserOption{s: name}
+}
+
+// NewParser constructs a parser from a list of destination structs
+func NewParser(dest interface{}, options ...ParserOption) (*Parser, error) {
+	// check the destination type
+	t := reflect.TypeOf(dest)
+	if t.Kind() != reflect.Ptr {
+		panic(fmt.Sprintf("%s is not a pointer (did you forget an ampersand?)", t))
+	}
+
+	// pick a program name for help text and usage output
+	program := "program"
+	if len(os.Args) > 0 {
+		program = filepath.Base(os.Args[0])
+	}
+
+	// apply the options
+	for _, opt := range options {
+		switch opt := opt.(type) {
+		case programNameParserOption:
+			program = opt.s
+		}
+	}
+
+	// build the root command from the struct
+	cmd, err := cmdFromStruct(program, path{}, t)
+	if err != nil {
+		return nil, err
+	}
+
+	// construct the parser
+	p := Parser{
+		seen: make(map[*Argument]bool),
+		root: reflect.ValueOf(dest),
+		cmd:  cmd,
+	}
+
+	// check for version, prologue, and epilogue
+	if dest, ok := dest.(Versioned); ok {
+		p.version = dest.Version()
+	}
+	if dest, ok := dest.(Described); ok {
+		p.prologue = dest.Description()
+	}
+	if dest, ok := dest.(Epilogued); ok {
+		p.epilogue = dest.Epilogue()
+	}
+
+	return &p, nil
+}
+
+func cmdFromStruct(name string, dest path, t reflect.Type) (*Command, error) {
+	// commands can only be created from pointers to structs
+	if t.Kind() != reflect.Ptr {
+		return nil, fmt.Errorf("subcommands must be pointers to structs but %s is a %s",
+			dest, t.Kind())
+	}
+
+	t = t.Elem()
+	if t.Kind() != reflect.Struct {
+		return nil, fmt.Errorf("subcommands must be pointers to structs but %s is a pointer to %s",
+			dest, t.Kind())
+	}
+
+	cmd := Command{
+		name: name,
+		dest: dest,
+	}
+
+	var errs []string
+	walkFields(t, func(field reflect.StructField, t reflect.Type) bool {
+		// check for the ignore switch in the tag
+		tag := field.Tag.Get("arg")
+		if tag == "-" {
+			return false
+		}
+
+		// if this is an embedded struct then recurse into its fields, even if
+		// it is unexported, because exported fields on unexported embedded
+		// structs are still writable
+		if field.Anonymous && field.Type.Kind() == reflect.Struct {
+			return true
+		}
+
+		// ignore any other unexported field
+		if !isExported(field.Name) {
+			return false
+		}
+
+		// duplicate the entire path to avoid slice overwrites
+		subdest := dest.Child(field)
+		arg := Argument{
+			dest:  subdest,
+			field: field,
+			long:  strings.ToLower(field.Name),
+		}
+
+		help, exists := field.Tag.Lookup("help")
+		if exists {
+			arg.help = help
+		}
+
+		defaultVal, hasDefault := field.Tag.Lookup("default")
+		if hasDefault {
+			arg.defaultVal = defaultVal
+		}
+
+		// Look at the tag
+		var isSubcommand bool // tracks whether this field is a subcommand
+		for _, key := range strings.Split(tag, ",") {
+			if key == "" {
+				continue
+			}
+			key = strings.TrimLeft(key, " ")
+			var value string
+			if pos := strings.Index(key, ":"); pos != -1 {
+				value = key[pos+1:]
+				key = key[:pos]
+			}
+
+			switch {
+			case strings.HasPrefix(key, "---"):
+				errs = append(errs, fmt.Sprintf("%s.%s: too many hyphens", t.Name(), field.Name))
+			case strings.HasPrefix(key, "--"):
+				arg.long = key[2:]
+			case strings.HasPrefix(key, "-"):
+				if len(key) != 2 {
+					errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only",
+						t.Name(), field.Name))
+					return false
+				}
+				arg.short = key[1:]
+			case key == "required":
+				if hasDefault {
+					errs = append(errs, fmt.Sprintf("%s.%s: 'required' cannot be used when a default value is specified",
+						t.Name(), field.Name))
+					return false
+				}
+				arg.required = true
+			case key == "positional":
+				arg.positional = true
+			case key == "separate":
+				arg.separate = true
+			case key == "help": // deprecated
+				arg.help = value
+			case key == "env":
+				// Use override name if provided
+				if value != "" {
+					arg.env = value
+				} else {
+					arg.env = strings.ToUpper(field.Name)
+				}
+			case key == "subcommand":
+				// decide on a name for the subcommand
+				cmdname := value
+				if cmdname == "" {
+					cmdname = strings.ToLower(field.Name)
+				}
+
+				// parse the subcommand recursively
+				subcmd, err := cmdFromStruct(cmdname, subdest, field.Type)
+				if err != nil {
+					errs = append(errs, err.Error())
+					return false
+				}
+
+				subcmd.parent = &cmd
+				subcmd.help = field.Tag.Get("help")
+
+				cmd.subcommands = append(cmd.subcommands, subcmd)
+				isSubcommand = true
+			default:
+				errs = append(errs, fmt.Sprintf("unrecognized tag '%s' on field %s", key, tag))
+				return false
+			}
+		}
+
+		placeholder, hasPlaceholder := field.Tag.Lookup("placeholder")
+		if hasPlaceholder {
+			arg.placeholder = placeholder
+		} else if arg.long != "" {
+			arg.placeholder = strings.ToUpper(arg.long)
+		} else {
+			arg.placeholder = strings.ToUpper(arg.field.Name)
+		}
+
+		// Check whether this field is supported. It's good to do this here rather than
+		// wait until ParseValue because it means that a program with invalid argument
+		// fields will always fail regardless of whether the arguments it received
+		// exercised those fields.
+		if !isSubcommand {
+			cmd.args = append(cmd.args, &arg)
+
+			var err error
+			arg.cardinality, err = cardinalityOf(field.Type)
+			if err != nil {
+				errs = append(errs, fmt.Sprintf("%s.%s: %s fields are not supported",
+					t.Name(), field.Name, field.Type.String()))
+				return false
+			}
+			if arg.cardinality == multiple && hasDefault {
+				errs = append(errs, fmt.Sprintf("%s.%s: default values are not supported for slice or map fields",
+					t.Name(), field.Name))
+				return false
+			}
+		}
+
+		// if this was an embedded field then we already returned true up above
+		return false
+	})
+
+	if len(errs) > 0 {
+		return nil, errors.New(strings.Join(errs, "\n"))
+	}
+
+	// check that we don't have both positionals and subcommands
+	var hasPositional bool
+	for _, arg := range cmd.args {
+		if arg.positional {
+			hasPositional = true
+		}
+	}
+	if hasPositional && len(cmd.subcommands) > 0 {
+		return nil, fmt.Errorf("%s cannot have both subcommands and positional arguments", dest)
+	}
+
+	return &cmd, nil
+}

v2/construct_test.go

@@ -0,0 +1,25 @@
+package arg
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestInvalidTag(t *testing.T) {
+	var args struct {
+		Foo string `arg:"this_is_not_valid"`
+	}
+	_, err := NewParser(&args)
+	assert.Error(t, err)
+}
+
+func TestUnexportedFieldsSkipped(t *testing.T) {
+	var args struct {
+		unexported struct{}
+	}
+
+	_, err := NewParser(&args)
+	require.NoError(t, err)
+}

v2/parse.go

@@ -5,65 +5,12 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"path/filepath"
 	"reflect"
 	"strings"
 
 	scalar "github.com/alexflint/go-scalar"
 )
 
-// path represents a sequence of steps to find the output location for an
-// argument or subcommand in the final destination struct
-type path struct {
-	fields []reflect.StructField // sequence of struct fields to traverse
-}
-
-// String gets a string representation of the given path
-func (p path) String() string {
-	s := "args"
-	for _, f := range p.fields {
-		s += "." + f.Name
-	}
-	return s
-}
-
-// Child gets a new path representing a child of this path.
-func (p path) Child(f reflect.StructField) path {
-	// copy the entire slice of fields to avoid possible slice overwrite
-	subfields := make([]reflect.StructField, len(p.fields)+1)
-	copy(subfields, p.fields)
-	subfields[len(subfields)-1] = f
-	return path{
-		fields: subfields,
-	}
-}
-
-// Arg represents a command line argument
-type Argument struct {
-	dest        path
-	field       reflect.StructField // the struct field from which this option was created
-	long        string              // the --long form for this option, or empty if none
-	short       string              // the -s short form for this option, or empty if none
-	cardinality cardinality         // determines how many tokens will be present (possible values: zero, one, multiple)
-	required    bool                // if true, this option must be present on the command line
-	positional  bool                // if true, this option will be looked for in the positional flags
-	separate    bool                // if true, each slice and map entry will have its own --flag
-	help        string              // the help text for this option
-	env         string              // the name of the environment variable for this option, or empty for none
-	defaultVal  string              // default value for this option
-	placeholder string              // name of the data in help
-}
-
-// Command represents a named subcommand, or the top-level command
-type Command struct {
-	name        string
-	help        string
-	dest        path
-	args        []*Argument
-	subcommands []*Command
-	parent      *Command
-}
-
 // ErrHelp indicates that -h or --help were provided
 var ErrHelp = errors.New("help requested by user")
 
@@ -103,308 +50,6 @@ func Parse(dest interface{}, options ...ParserOption) error {
 	return p.Parse(os.Args, os.Environ())
 }
 
-// Parser represents a set of command line options with destination values
-type Parser struct {
-	cmd      *Command      // the top-level command
-	root     reflect.Value // destination struct to fill will values
-	version  string        // version from the argument struct
-	prologue string        // prologue for help text (from the argument struct)
-	epilogue string        // epilogue for help text (from the argument struct)
-
-	// the following fields are updated during processing of command line arguments
-	leaf            *Command           // the subcommand we processed last
-	accumulatedArgs []*Argument        // concatenation of the leaf subcommand's arguments plus all ancestors' arguments
-	seen            map[*Argument]bool // the arguments we encountered while processing command line arguments
-}
-
-// Versioned is the interface that the destination struct should implement to
-// make a version string appear at the top of the help message.
-type Versioned interface {
-	// Version returns the version string that will be printed on a line by itself
-	// at the top of the help message.
-	Version() string
-}
-
-// Described is the interface that the destination struct should implement to
-// make a description string appear at the top of the help message.
-type Described interface {
-	// Description returns the string that will be printed on a line by itself
-	// at the top of the help message.
-	Description() string
-}
-
-// Epilogued is the interface that the destination struct should implement to
-// add an epilogue string at the bottom of the help message.
-type Epilogued interface {
-	// Epilogue returns the string that will be printed on a line by itself
-	// at the end of the help message.
-	Epilogue() string
-}
-
-// walkFields calls a function for each field of a struct, recursively expanding struct fields.
-func walkFields(t reflect.Type, visit func(field reflect.StructField, owner reflect.Type) bool) {
-	walkFieldsImpl(t, visit, nil)
-}
-
-func walkFieldsImpl(t reflect.Type, visit func(field reflect.StructField, owner reflect.Type) bool, path []int) {
-	for i := 0; i < t.NumField(); i++ {
-		field := t.Field(i)
-		field.Index = make([]int, len(path)+1)
-		copy(field.Index, append(path, i))
-		expand := visit(field, t)
-		if expand && field.Type.Kind() == reflect.Struct {
-			var subpath []int
-			if field.Anonymous {
-				subpath = append(path, i)
-			}
-			walkFieldsImpl(field.Type, visit, subpath)
-		}
-	}
-}
-
-// the ParserOption interface matches options for the parser constructor
-type ParserOption interface {
-	parserOption()
-}
-
-type programNameParserOption struct {
-	s string
-}
-
-func (programNameParserOption) parserOption() {}
-
-// WithProgramName overrides the name of the program as displayed in help test
-func WithProgramName(name string) ParserOption {
-	return programNameParserOption{s: name}
-}
-
-// NewParser constructs a parser from a list of destination structs
-func NewParser(dest interface{}, options ...ParserOption) (*Parser, error) {
-	// check the destination type
-	t := reflect.TypeOf(dest)
-	if t.Kind() != reflect.Ptr {
-		panic(fmt.Sprintf("%s is not a pointer (did you forget an ampersand?)", t))
-	}
-
-	// pick a program name for help text and usage output
-	program := "program"
-	if len(os.Args) > 0 {
-		program = filepath.Base(os.Args[0])
-	}
-
-	// apply the options
-	for _, opt := range options {
-		switch opt := opt.(type) {
-		case programNameParserOption:
-			program = opt.s
-		}
-	}
-
-	// build the root command from the struct
-	cmd, err := cmdFromStruct(program, path{}, t)
-	if err != nil {
-		return nil, err
-	}
-
-	// construct the parser
-	p := Parser{
-		seen: make(map[*Argument]bool),
-		root: reflect.ValueOf(dest),
-		cmd:  cmd,
-	}
-
-	// check for version, prologue, and epilogue
-	if dest, ok := dest.(Versioned); ok {
-		p.version = dest.Version()
-	}
-	if dest, ok := dest.(Described); ok {
-		p.prologue = dest.Description()
-	}
-	if dest, ok := dest.(Epilogued); ok {
-		p.epilogue = dest.Epilogue()
-	}
-
-	return &p, nil
-}
-
-func cmdFromStruct(name string, dest path, t reflect.Type) (*Command, error) {
-	// commands can only be created from pointers to structs
-	if t.Kind() != reflect.Ptr {
-		return nil, fmt.Errorf("subcommands must be pointers to structs but %s is a %s",
-			dest, t.Kind())
-	}
-
-	t = t.Elem()
-	if t.Kind() != reflect.Struct {
-		return nil, fmt.Errorf("subcommands must be pointers to structs but %s is a pointer to %s",
-			dest, t.Kind())
-	}
-
-	cmd := Command{
-		name: name,
-		dest: dest,
-	}
-
-	var errs []string
-	walkFields(t, func(field reflect.StructField, t reflect.Type) bool {
-		// check for the ignore switch in the tag
-		tag := field.Tag.Get("arg")
-		if tag == "-" {
-			return false
-		}
-
-		// if this is an embedded struct then recurse into its fields, even if
-		// it is unexported, because exported fields on unexported embedded
-		// structs are still writable
-		if field.Anonymous && field.Type.Kind() == reflect.Struct {
-			return true
-		}
-
-		// ignore any other unexported field
-		if !isExported(field.Name) {
-			return false
-		}
-
-		// duplicate the entire path to avoid slice overwrites
-		subdest := dest.Child(field)
-		arg := Argument{
-			dest:  subdest,
-			field: field,
-			long:  strings.ToLower(field.Name),
-		}
-
-		help, exists := field.Tag.Lookup("help")
-		if exists {
-			arg.help = help
-		}
-
-		defaultVal, hasDefault := field.Tag.Lookup("default")
-		if hasDefault {
-			arg.defaultVal = defaultVal
-		}
-
-		// Look at the tag
-		var isSubcommand bool // tracks whether this field is a subcommand
-		for _, key := range strings.Split(tag, ",") {
-			if key == "" {
-				continue
-			}
-			key = strings.TrimLeft(key, " ")
-			var value string
-			if pos := strings.Index(key, ":"); pos != -1 {
-				value = key[pos+1:]
-				key = key[:pos]
-			}
-
-			switch {
-			case strings.HasPrefix(key, "---"):
-				errs = append(errs, fmt.Sprintf("%s.%s: too many hyphens", t.Name(), field.Name))
-			case strings.HasPrefix(key, "--"):
-				arg.long = key[2:]
-			case strings.HasPrefix(key, "-"):
-				if len(key) != 2 {
-					errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only",
-						t.Name(), field.Name))
-					return false
-				}
-				arg.short = key[1:]
-			case key == "required":
-				if hasDefault {
-					errs = append(errs, fmt.Sprintf("%s.%s: 'required' cannot be used when a default value is specified",
-						t.Name(), field.Name))
-					return false
-				}
-				arg.required = true
-			case key == "positional":
-				arg.positional = true
-			case key == "separate":
-				arg.separate = true
-			case key == "help": // deprecated
-				arg.help = value
-			case key == "env":
-				// Use override name if provided
-				if value != "" {
-					arg.env = value
-				} else {
-					arg.env = strings.ToUpper(field.Name)
-				}
-			case key == "subcommand":
-				// decide on a name for the subcommand
-				cmdname := value
-				if cmdname == "" {
-					cmdname = strings.ToLower(field.Name)
-				}
-
-				// parse the subcommand recursively
-				subcmd, err := cmdFromStruct(cmdname, subdest, field.Type)
-				if err != nil {
-					errs = append(errs, err.Error())
-					return false
-				}
-
-				subcmd.parent = &cmd
-				subcmd.help = field.Tag.Get("help")
-
-				cmd.subcommands = append(cmd.subcommands, subcmd)
-				isSubcommand = true
-			default:
-				errs = append(errs, fmt.Sprintf("unrecognized tag '%s' on field %s", key, tag))
-				return false
-			}
-		}
-
-		placeholder, hasPlaceholder := field.Tag.Lookup("placeholder")
-		if hasPlaceholder {
-			arg.placeholder = placeholder
-		} else if arg.long != "" {
-			arg.placeholder = strings.ToUpper(arg.long)
-		} else {
-			arg.placeholder = strings.ToUpper(arg.field.Name)
-		}
-
-		// Check whether this field is supported. It's good to do this here rather than
-		// wait until ParseValue because it means that a program with invalid argument
-		// fields will always fail regardless of whether the arguments it received
-		// exercised those fields.
-		if !isSubcommand {
-			cmd.args = append(cmd.args, &arg)
-
-			var err error
-			arg.cardinality, err = cardinalityOf(field.Type)
-			if err != nil {
-				errs = append(errs, fmt.Sprintf("%s.%s: %s fields are not supported",
-					t.Name(), field.Name, field.Type.String()))
-				return false
-			}
-			if arg.cardinality == multiple && hasDefault {
-				errs = append(errs, fmt.Sprintf("%s.%s: default values are not supported for slice or map fields",
-					t.Name(), field.Name))
-				return false
-			}
-		}
-
-		// if this was an embedded field then we already returned true up above
-		return false
-	})
-
-	if len(errs) > 0 {
-		return nil, errors.New(strings.Join(errs, "\n"))
-	}
-
-	// check that we don't have both positionals and subcommands
-	var hasPositional bool
-	for _, arg := range cmd.args {
-		if arg.positional {
-			hasPositional = true
-		}
-	}
-	if hasPositional && len(cmd.subcommands) > 0 {
-		return nil, fmt.Errorf("%s cannot have both subcommands and positional arguments", dest)
-	}
-
-	return &cmd, nil
-}
-
 // Parse processes the given command line option, storing the results in the field
 // of the structs from which NewParser was constructed
 func (p *Parser) Parse(args, env []string) error {
@@ -763,11 +408,6 @@ func (p *Parser) Validate() error {
 	return nil
 }
 
-// isFlag returns true if a token is a flag such as "-v" or "--user" but not "-" or "--"
-func isFlag(s string) bool {
-	return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != ""
-}
-
 // val returns a reflect.Value corresponding to the current value for the
 // given path
 func (p *Parser) val(dest path) reflect.Value {
@@ -785,6 +425,58 @@ func (p *Parser) val(dest path) reflect.Value {
 	return v
 }
 
+// path represents a sequence of steps to find the output location for an
+// argument or subcommand in the final destination struct
+type path struct {
+	fields []reflect.StructField // sequence of struct fields to traverse
+}
+
+// String gets a string representation of the given path
+func (p path) String() string {
+	s := "args"
+	for _, f := range p.fields {
+		s += "." + f.Name
+	}
+	return s
+}
+
+// Child gets a new path representing a child of this path.
+func (p path) Child(f reflect.StructField) path {
+	// copy the entire slice of fields to avoid possible slice overwrite
+	subfields := make([]reflect.StructField, len(p.fields)+1)
+	copy(subfields, p.fields)
+	subfields[len(subfields)-1] = f
+	return path{
+		fields: subfields,
+	}
+}
+
+// walkFields calls a function for each field of a struct, recursively expanding struct fields.
+func walkFields(t reflect.Type, visit func(field reflect.StructField, owner reflect.Type) bool) {
+	walkFieldsImpl(t, visit, nil)
+}
+
+func walkFieldsImpl(t reflect.Type, visit func(field reflect.StructField, owner reflect.Type) bool, path []int) {
+	for i := 0; i < t.NumField(); i++ {
+		field := t.Field(i)
+		field.Index = make([]int, len(path)+1)
+		copy(field.Index, append(path, i))
+		expand := visit(field, t)
+		if expand && field.Type.Kind() == reflect.Struct {
+			var subpath []int
+			if field.Anonymous {
+				subpath = append(path, i)
+			}
+			walkFieldsImpl(field.Type, visit, subpath)
+		}
+	}
+}
+
+// isFlag returns true if a token is a flag such as "-v" or "--user" but not "-" or "--"
+func isFlag(s string) bool {
+	return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != ""
+}
+
 // findOption finds an option from its name, or returns nil if no arg is found
 func findOption(args []*Argument, name string) *Argument {
 	for _, arg := range args {

v2/parse_test.go

@@ -629,14 +629,6 @@ func TestParse(t *testing.T) {
 	assert.Equal(t, "bar", args.Foo)
 }
 
-func TestParseError(t *testing.T) {
-	var args struct {
-		Foo string `arg:"this_is_not_valid"`
-	}
-	_, err := NewParser(&args)
-	assert.Error(t, err)
-}
-
 func TestMustParse(t *testing.T) {
 	var args struct {
 		Foo string
@@ -795,13 +787,13 @@ func TestEnvironmentVariableIgnored(t *testing.T) {
 }
 
 func TestDefaultValuesIgnored(t *testing.T) {
-	var args struct {
+	// check that default values are not automatically applied
-		Foo string `default:"bad"`
-	}
-
-	// just checking that default values are not automatically applied
 	// in ProcessCommandLine or ProcessEnvironment
 
+	var args struct {
+		Foo string `default:"hello"`
+	}
+
 	p, err := NewParser(&args)
 	require.NoError(t, err)
 
@@ -1390,15 +1382,6 @@ func TestDefaultValuesNotAllowedWithSlice(t *testing.T) {
 	assert.EqualError(t, err, ".A: default values are not supported for slice or map fields")
 }
 
-func TestUnexportedFieldsSkipped(t *testing.T) {
-	var args struct {
-		unexported struct{}
-	}
-
-	_, err := NewParser(&args)
-	require.NoError(t, err)
-}
-
 func TestMustParseInvalidParser(t *testing.T) {
 	originalExit := osExit
 	originalStdout := stdout