mcfg: make Parse and Populate return a Context, to allow for sub-commands

mcfg/cli.go

@@ -75,7 +75,7 @@ const (
 )
 
 // Parse implements the method for the Source interface
-func (cli *SourceCLI) Parse(ctx context.Context, params []Param) ([]ParamValue, error) {
+func (cli *SourceCLI) Parse(ctx context.Context, params []Param) (context.Context, []ParamValue, error) {
 	args := cli.Args
 	if cli.Args == nil {
 		args = os.Args[1:]
@@ -83,7 +83,7 @@ func (cli *SourceCLI) Parse(ctx context.Context, params []Param) ([]ParamValue,
 
 	pM, err := cli.cliParams(params)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	pvs := make([]ParamValue, 0, len(args))
 	var (
@@ -119,10 +119,10 @@ func (cli *SourceCLI) Parse(ctx context.Context, params []Param) ([]ParamValue,
 			}
 			if !pOk {
 				if ok := populateCLITail(ctx, args[i:]); ok {
-					return pvs, nil
+					return ctx, pvs, nil
 				}
 				ctx := mctx.Annotate(context.Background(), "param", arg)
-				return nil, merr.New("unexpected config parameter", ctx)
+				return nil, nil, merr.New("unexpected config parameter", ctx)
 			}
 		}
 
@@ -153,10 +153,10 @@ func (cli *SourceCLI) Parse(ctx context.Context, params []Param) ([]ParamValue,
 	}
 	if pOk && !pvStrValOk {
 		ctx := mctx.Annotate(p.Context, "param", key)
-		return nil, merr.New("param expected a value", ctx)
+		return nil, nil, merr.New("param expected a value", ctx)
 	}
 
-	return pvs, nil
+	return ctx, pvs, nil
 }
 
 func (cli *SourceCLI) cliParams(params []Param) (map[string]Param, error) {

mcfg/cli_test.go

@@ -143,7 +143,7 @@ func TestWithCLITail(t *T) {
 
 	for _, tc := range cases {
 		ctx, tail := WithCLITail(ctx)
-		err := Populate(ctx, &SourceCLI{Args: tc.args})
+		_, err := Populate(ctx, &SourceCLI{Args: tc.args})
 		massert.Require(t, massert.Comment(massert.All(
 			massert.Nil(err),
 			massert.Equal(tc.expTail, *tail),
@@ -157,7 +157,7 @@ func ExampleWithCLITail() {
 	ctx, tail := WithCLITail(ctx)
 	ctx, bar := WithString(ctx, "bar", "defaultVal", "Description of bar.")
 
-	err := Populate(ctx, &SourceCLI{
+	_, err := Populate(ctx, &SourceCLI{
 		Args: []string{"--foo=100", "BADARG", "--bar", "BAR"},
 	})
 

mcfg/env.go

@@ -43,7 +43,7 @@ func (env *SourceEnv) expectedName(path []string, name string) string {
 }
 
 // Parse implements the method for the Source interface
-func (env *SourceEnv) Parse(ctx context.Context, params []Param) ([]ParamValue, error) {
+func (env *SourceEnv) Parse(ctx context.Context, params []Param) (context.Context, []ParamValue, error) {
 	kvs := env.Env
 	if kvs == nil {
 		kvs = os.Environ()
@@ -60,7 +60,7 @@ func (env *SourceEnv) Parse(ctx context.Context, params []Param) ([]ParamValue,
 		split := strings.SplitN(kv, "=", 2)
 		if len(split) != 2 {
 			ctx := mctx.Annotate(context.Background(), "kv", kv)
-			return nil, merr.New("malformed environment key/value pair", ctx)
+			return nil, nil, merr.New("malformed environment key/value pair", ctx)
 		}
 		k, v := split[0], split[1]
 		if p, ok := pM[k]; ok {
@@ -72,5 +72,5 @@ func (env *SourceEnv) Parse(ctx context.Context, params []Param) ([]ParamValue,
 		}
 	}
 
-	return pvs, nil
+	return ctx, pvs, nil
 }

mcfg/mcfg.go

@@ -77,9 +77,14 @@ func paramHash(path []string, name string) string {
 // multiple times with the same Context, each time will only affect the values
 // of the Params which were provided by the respective Source.
 //
+// Populating Params can affect the Context itself, for example in the case of
+// sub-commands. For this reason Populate will return a Context instance which
+// may have been affected by the Params (or, if not, will be the same one which
+// was passed in).
+//
 // Source may be nil to indicate that no configuration is provided. Only default
 // values will be used, and if any parameters are required this will error.
-func Populate(ctx context.Context, src Source) error {
+func Populate(ctx context.Context, src Source) (context.Context, error) {
 	params := collectParams(ctx)
 	if src == nil {
 		src = ParamValues(nil)
@@ -97,9 +102,9 @@ func Populate(ctx context.Context, src Source) error {
 		pM[hash] = p
 	}
 
-	pvs, err := src.Parse(ctx, params)
+	ctx, pvs, err := src.Parse(ctx, params)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// dedupe the ParamValues based on their hashes, with the last ParamValue
@@ -120,7 +125,7 @@ func Populate(ctx context.Context, src Source) error {
 		} else if _, ok := pvM[hash]; !ok {
 			ctx := mctx.Annotate(p.Context,
 				"param", paramFullName(mctx.Path(p.Context), p.Name))
-			return merr.New("required parameter is not set", ctx)
+			return nil, merr.New("required parameter is not set", ctx)
 		}
 	}
 
@@ -129,9 +134,9 @@ func Populate(ctx context.Context, src Source) error {
 		// at this point, all ParamValues in pvM have a corresponding pM Param
 		p := pM[hash]
 		if err := json.Unmarshal(pv.Value, p.Into); err != nil {
-			return err
+			return nil, err
 		}
 	}
 
-	return nil
+	return ctx, nil
 }

mcfg/mcfg_test.go

@@ -17,7 +17,7 @@ func TestPopulate(t *T) {
 		ctxChild, c := WithInt(ctxChild, "c", 0, "")
 		ctx = mctx.WithChild(ctx, ctxChild)
 
-		err := Populate(ctx, &SourceCLI{
+		_, err := Populate(ctx, &SourceCLI{
 			Args: []string{"--a=1", "--foo-b=2"},
 		})
 		assert.NoError(t, err)
@@ -34,12 +34,12 @@ func TestPopulate(t *T) {
 		ctxChild, c := WithRequiredInt(ctxChild, "c", "")
 		ctx = mctx.WithChild(ctx, ctxChild)
 
-		err := Populate(ctx, &SourceCLI{
+		_, err := Populate(ctx, &SourceCLI{
 			Args: []string{"--a=1", "--foo-b=2"},
 		})
 		assert.Error(t, err)
 
-		err = Populate(ctx, &SourceCLI{
+		_, err = Populate(ctx, &SourceCLI{
 			Args: []string{"--a=1", "--foo-b=2", "--foo-c=3"},
 		})
 		assert.NoError(t, err)

mcfg/source.go

@@ -17,6 +17,11 @@ type ParamValue struct {
 // sorted set of possible Params to parse, and the Context from with the Params
 // were extracted.
 //
+// It's possible for Parsing to affect the Context itself, for example in the
+// case of sub-commands. For this reason Parse can return a Context, which will
+// get used for subsequent Parse commands inside, and then returned from,
+// Populate.
+//
 // Source should not return ParamValues which were not explicitly set to a value
 // by the configuration source.
 //
@@ -25,7 +30,7 @@ type ParamValue struct {
 // ParamValues which do not correspond to any of the passed in Params. These
 // will be ignored in Populate.
 type Source interface {
-	Parse(context.Context, []Param) ([]ParamValue, error)
+	Parse(context.Context, []Param) (context.Context, []ParamValue, error)
 }
 
 // ParamValues is simply a slice of ParamValue elements, which implements Parse
@@ -33,8 +38,8 @@ type Source interface {
 type ParamValues []ParamValue
 
 // Parse implements the method for the Source interface.
-func (pvs ParamValues) Parse(context.Context, []Param) ([]ParamValue, error) {
+func (pvs ParamValues) Parse(ctx context.Context, _ []Param) (context.Context, []ParamValue, error) {
-	return pvs, nil
+	return ctx, pvs, nil
 }
 
 // Sources combines together multiple Source instances into one. It will call
@@ -43,14 +48,15 @@ func (pvs ParamValues) Parse(context.Context, []Param) ([]ParamValue, error) {
 type Sources []Source
 
 // Parse implements the method for the Source interface.
-func (ss Sources) Parse(ctx context.Context, params []Param) ([]ParamValue, error) {
+func (ss Sources) Parse(ctx context.Context, params []Param) (context.Context, []ParamValue, error) {
 	var pvs []ParamValue
 	for _, s := range ss {
-		innerPVs, err := s.Parse(ctx, params)
+		var innerPVs []ParamValue
-		if err != nil {
+		var err error
-			return nil, err
+		if ctx, innerPVs, err = s.Parse(ctx, params); err != nil {
+			return nil, nil, err
 		}
 		pvs = append(pvs, innerPVs...)
 	}
-	return pvs, nil
+	return ctx, pvs, nil
 }

mcfg/source_test.go

@@ -143,7 +143,7 @@ func (scs srcCommonState) applyCtxAndPV(p srcCommonParams) srcCommonState {
 // ParamValues
 func (scs srcCommonState) assert(s Source) error {
 	root := scs.mkRoot()
-	gotPVs, err := s.Parse(root, collectParams(root))
+	_, gotPVs, err := s.Parse(root, collectParams(root))
 	if err != nil {
 		return err
 	}
@@ -159,7 +159,7 @@ func TestSources(t *T) {
 	ctx, b := WithRequiredInt(ctx, "b", "")
 	ctx, c := WithRequiredInt(ctx, "c", "")
 
-	err := Populate(ctx, Sources{
+	_, err := Populate(ctx, Sources{
 		&SourceCLI{Args: []string{"--a=1", "--b=666"}},
 		&SourceEnv{Env: []string{"B=2", "C=3"}},
 	})
@@ -179,7 +179,7 @@ func TestSourceParamValues(t *T) {
 	foo, c := WithBool(foo, "c", "")
 	ctx = mctx.WithChild(ctx, foo)
 
-	err := Populate(ctx, ParamValues{
+	_, err := Populate(ctx, ParamValues{
 		{Name: "a", Value: json.RawMessage(`4`)},
 		{Path: []string{"foo"}, Name: "b", Value: json.RawMessage(`"bbb"`)},
 		{Path: []string{"foo"}, Name: "c", Value: json.RawMessage("true")},