no need to initialize nil structs during path traversal

4 files changed, 33 insertions, 38 deletions

diff --git a/parse.go b/parse.go
index e0de705..2126ee5 100644
--- a/parse.go
+++ b/parse.go
@@ -393,7 +393,7 @@ func (p *Parser) captureEnvVars(specs []*spec, wasPresent map[*spec]bool) error
 					err,
 				)
 			}
-			if err = setSlice(p.writable(spec.dest), values, !spec.separate); err != nil {
+			if err = setSlice(p.val(spec.dest), values, !spec.separate); err != nil {
 				return fmt.Errorf(
 					"error processing environment variable %s with multiple values: %v",
 					spec.env,
@@ -401,7 +401,7 @@ func (p *Parser) captureEnvVars(specs []*spec, wasPresent map[*spec]bool) error
 				)
 			}
 		} else {
-			if err := scalar.ParseValue(p.writable(spec.dest), value); err != nil {
+			if err := scalar.ParseValue(p.val(spec.dest), value); err != nil {
 				return fmt.Errorf("error processing environment variable %s: %v", spec.env, err)
 			}
 		}
@@ -457,7 +457,7 @@ func (p *Parser) process(args []string) error {
 			}
 
 			// instantiate the field to point to a new struct
-			v := p.writable(subcmd.dest)
+			v := p.val(subcmd.dest)
 			v.Set(reflect.New(v.Type().Elem())) // we already checked that all subcommands are struct pointers
 
 			// add the new options to the set of allowed options
@@ -512,7 +512,7 @@ func (p *Parser) process(args []string) error {
 			} else {
 				values = append(values, value)
 			}
-			err := setSlice(p.writable(spec.dest), values, !spec.separate)
+			err := setSlice(p.val(spec.dest), values, !spec.separate)
 			if err != nil {
 				return fmt.Errorf("error processing %s: %v", arg, err)
 			}
@@ -537,7 +537,7 @@ func (p *Parser) process(args []string) error {
 			i++
 		}
 
-		err := scalar.ParseValue(p.writable(spec.dest), value)
+		err := scalar.ParseValue(p.val(spec.dest), value)
 		if err != nil {
 			return fmt.Errorf("error processing %s: %v", arg, err)
 		}
@@ -553,13 +553,13 @@ func (p *Parser) process(args []string) error {
 		}
 		wasPresent[spec] = true
 		if spec.multiple {
-			err := setSlice(p.writable(spec.dest), positionals, true)
+			err := setSlice(p.val(spec.dest), positionals, true)
 			if err != nil {
 				return fmt.Errorf("error processing %s: %v", spec.long, err)
 			}
 			positionals = nil
 		} else {
-			err := scalar.ParseValue(p.writable(spec.dest), positionals[0])
+			err := scalar.ParseValue(p.val(spec.dest), positionals[0])
 			if err != nil {
 				return fmt.Errorf("error processing %s: %v", spec.long, err)
 			}
@@ -602,9 +602,9 @@ func isFlag(s string) bool {
 	return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != ""
 }
 
-// readable returns a reflect.Value corresponding to the current value for the
-// given
-func (p *Parser) readable(dest path) reflect.Value {
+// val returns a reflect.Value corresponding to the current value for the
+// given path
+func (p *Parser) val(dest path) reflect.Value {
 	v := p.roots[dest.root]
 	for _, field := range dest.fields {
 		if v.Kind() == reflect.Ptr {
@@ -626,35 +626,10 @@ func (p *Parser) readable(dest path) reflect.Value {
 	return v
 }
 
-// writable trav.patherses the destination struct to find the destination to
-// which the value of the given spec should be written. It fills in null
-// structs with pointers to the zero value for that struct.
-func (p *Parser) writable(dest path) reflect.Value {
-	v := p.roots[dest.root]
-	for _, field := range dest.fields {
-		if v.Kind() == reflect.Ptr {
-			if v.IsNil() {
-				v.Set(reflect.New(v.Type().Elem()))
-			}
-			v = v.Elem()
-		}
-
-		v = v.FieldByName(field)
-		if !v.IsValid() {
-			// it is appropriate to panic here because this can only happen due to
-			// an internal bug in this library (since we construct the path ourselves
-			// by reflecting on the same struct)
-			panic(fmt.Errorf("error resolving path %v: %v has no field named %v",
-				dest.fields, v.Type(), field))
-		}
-	}
-	return v
-}
-
 // parse a value as the appropriate type and store it in the struct
 func setSlice(dest reflect.Value, values []string, trunc bool) error {
 	if !dest.CanSet() {
-		return fmt.Errorf("field is not writable")
+		return fmt.Errorf("field is not val")
 	}
 
 	var ptr bool
diff --git a/parse_test.go b/parse_test.go
index c2544fd..c18dc16 100644
--- a/parse_test.go
+++ b/parse_test.go
@@ -877,6 +877,26 @@ func TestEmbedded(t *testing.T) {
 	assert.Equal(t, true, args.Z)
 }
 
+func TestEmbeddedPtr(t *testing.T) {
+	// embedded pointer fields are not supported so this should return an error
+	var args struct {
+		*A
+	}
+	err := parse("--x=hello", &args)
+	require.Error(t, err)
+}
+
+func TestEmbeddedPtrIgnored(t *testing.T) {
+	// embedded pointer fields are not supported but here we
+	var args struct {
+		*A `arg:"-"`
+		B
+	}
+	err := parse("--y=321", &args)
+	require.NoError(t, err)
+	assert.Equal(t, 321, args.Y)
+}
+
 func TestEmptyArgs(t *testing.T) {
 	origArgs := os.Args
 
diff --git a/subcommand.go b/subcommand.go
index b73e933..dff732c 100644
--- a/subcommand.go
+++ b/subcommand.go
@@ -10,7 +10,7 @@ func (p *Parser) Subcommand() interface{} {
 	if p.lastCmd == nil || p.lastCmd.parent == nil {
 		return nil
 	}
-	return p.readable(p.lastCmd.dest).Interface()
+	return p.val(p.lastCmd.dest).Interface()
 }
 
 // SubcommandNames returns the sequence of subcommands specified by the
diff --git a/usage.go b/usage.go
index 1f6b569..33b6184 100644
--- a/usage.go
+++ b/usage.go
@@ -179,7 +179,7 @@ func (p *Parser) printOption(w io.Writer, spec *spec) {
 	// If spec.dest is not the zero value then a default value has been added.
 	var v reflect.Value
 	if len(spec.dest.fields) > 0 {
-		v = p.readable(spec.dest)
+		v = p.val(spec.dest)
 	}
 
 	var defaultVal *string