fs/rc: add ParseOptions and CheckParamsUsed unified options helpers

fs/rc/context.go

@@ -2,40 +2,16 @@ package rc
 
 import (
 	"context"
+	"fmt"
 	"reflect"
-	"sync"
+	"sort"
+	"strings"
 
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/config/configstruct"
 	"github.com/rclone/rclone/fs/filter"
 )
 
-var (
-	configOptionsOnce sync.Once
-	configOptionsMap  map[string]bool
-
-	filterOptionsOnce sync.Once
-	filterOptionsMap  map[string]bool
-)
-
-func initConfigOptions() {
-	configOptionsOnce.Do(func() {
-		configOptionsMap = make(map[string]bool, len(fs.ConfigOptionsInfo))
-		for _, opt := range fs.ConfigOptionsInfo {
-			configOptionsMap[opt.Name] = true
-		}
-	})
-}
-
-func initFilterOptions() {
-	filterOptionsOnce.Do(func() {
-		filterOptionsMap = make(map[string]bool, len(filter.OptionsInfo))
-		for _, opt := range filter.OptionsInfo {
-			filterOptionsMap[opt.Name] = true
-		}
-	})
-}
-
 // isMap returns true if v's underlying type is a map
 func isMap(v any) bool {
 	if v == nil {
@@ -48,118 +24,88 @@ func isMap(v any) bool {
 	return t.Kind() == reflect.Map
 }
 
-// hasConfigOption checks if any config options are present in the params
+// hasOption checks if any options are present in the params
-func hasConfigOption(in Params) bool {
+func hasOption(in Params, key string, opt any) bool {
-	if _, ok := in["_config"]; ok {
+	if _, ok := in[key]; ok {
 		return true
 	}
-	initConfigOptions()
+	items, err := configstruct.Items(opt)
-	for k, v := range in {
+	if err != nil {
-		if configOptionsMap[k] {
+		return false
-			if isMap(v) {
+	}
-				continue
+	for _, item := range items {
-			}
+		if v, ok := in[item.Name]; ok && !isMap(v) {
 			return true
 		}
 	}
 	return false
 }
 
-// hasFilterOption checks if any filter options are present in the params
+// ParseOptions sets opt from the flat parameters in whose names match opt's
-func hasFilterOption(in Params) bool {
+// fields, then overlays the nested block under key if present. Every key it
-	if _, ok := in["_filter"]; ok {
+// consumes is deleted from in. Values in the nested block take precedence over
-		return true
+// flat ones. opt must be a pointer to a struct of configstruct-settable fields.
+func ParseOptions(in Params, key string, opt any) error {
+	items, err := configstruct.Items(opt)
+	if err != nil {
+		return err
 	}
-	initFilterOptions()
+	flat := make(map[string]any)
-	for k, v := range in {
+	for _, item := range items {
-		if filterOptionsMap[k] {
+		if v, ok := in[item.Name]; ok && !isMap(v) {
-			if isMap(v) {
+			flat[item.Name] = v
-				continue
-			}
-			return true
 		}
 	}
-	return false
+	if len(flat) > 0 {
+		if err := configstruct.SetAny(flat, opt); err != nil {
+			return err
+		}
+		for k := range flat {
+			delete(in, k)
+		}
+	}
+	if err := in.GetStructMissingOK(key, opt); err != nil {
+		return err
+	}
+	delete(in, key)
+	return nil
+}
+
+// CheckParamsUsed returns an error if any parameters remain in the map.
+// It formats them as a list of sorted keys for determinism.
+func CheckParamsUsed(in Params) error {
+	if len(in) == 0 {
+		return nil
+	}
+	keys := make([]string, 0, len(in))
+	for k := range in {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return fmt.Errorf("unknown parameters: %s", strings.Join(keys, ", "))
 }
 
 // AddConfig parses any config options from the parameters and returns a new context with the configuration.
 func AddConfig(ctx context.Context, in Params) (context.Context, error) {
-	if !hasConfigOption(in) {
+	if !hasOption(in, "_config", &fs.ConfigInfo{}) {
 		return ctx, nil
 	}
 	ctx, ci := fs.AddConfig(ctx)
-
+	if err := ParseOptions(in, "_config", ci); err != nil {
-	// Extract the genuine flat config options
+		return ctx, err
-	initConfigOptions()
-	flatConfig := make(map[string]any)
-	for k, v := range in {
-		if configOptionsMap[k] {
-			if isMap(v) {
-				continue
-			}
-			flatConfig[k] = v
-		}
-	}
-
-	if len(flatConfig) > 0 {
-		err := configstruct.SetAny(flatConfig, ci)
-		if err != nil {
-			return ctx, err
-		}
-		// Remove the consumed flat options from the input params
-		for k := range flatConfig {
-			delete(in, k)
-		}
-	}
-
-	if _, ok := in["_config"]; ok {
-		err := in.GetStruct("_config", ci)
-		if err != nil {
-			return ctx, err
-		}
-		delete(in, "_config") // remove the parameter
 	}
 	return ctx, nil
 }
 
 // AddFilter parses any filter options from the parameters and returns a new context with the filter.
 func AddFilter(ctx context.Context, in Params) (context.Context, error) {
-	if !hasFilterOption(in) {
+	if !hasOption(in, "_filter", &filter.Opt) {
 		return ctx, nil
 	}
 	// Copy of the current filter options
 	opt := filter.GetConfig(ctx).Opt
-
+	if err := ParseOptions(in, "_filter", &opt); err != nil {
-	// Extract the genuine flat filter options
+		return ctx, err
-	initFilterOptions()
-	flatFilter := make(map[string]any)
-	for k, v := range in {
-		if filterOptionsMap[k] {
-			if isMap(v) {
-				continue
-			}
-			flatFilter[k] = v
-		}
-	}
-
-	if len(flatFilter) > 0 {
-		err := configstruct.SetAny(flatFilter, &opt)
-		if err != nil {
-			return ctx, err
-		}
-		// Remove the consumed flat options from the input params
-		for k := range flatFilter {
-			delete(in, k)
-		}
-	}
-
-	if _, ok := in["_filter"]; ok {
-		// Update the options from the parameter
-		err := in.GetStruct("_filter", &opt)
-		if err != nil {
-			return ctx, err
-		}
-		delete(in, "_filter") // remove the parameter
 	}
 	fi, err := filter.NewFilter(&opt)
 	if err != nil {

fs/rc/context_test.go

@@ -0,0 +1,124 @@
+package rc
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type dummyOptions struct {
+	StringOpt string   `config:"string_opt"`
+	IntOpt    int      `config:"int_opt"`
+	BoolOpt   bool     `config:"bool_opt"`
+	SliceOpt  []string `config:"slice_opt"`
+}
+
+func TestParseOptions(t *testing.T) {
+	t.Run("FlatOnly", func(t *testing.T) {
+		in := Params{
+			"string_opt": "hello",
+			"int_opt":    42,
+			"bool_opt":   true,
+			"slice_opt":  []any{"a", "b"},
+		}
+		var opt dummyOptions
+		err := ParseOptions(in, "dummy", &opt)
+		require.NoError(t, err)
+		assert.Equal(t, "hello", opt.StringOpt)
+		assert.Equal(t, 42, opt.IntOpt)
+		assert.True(t, opt.BoolOpt)
+		assert.Equal(t, []string{"a", "b"}, opt.SliceOpt)
+		assert.Empty(t, in)
+	})
+
+	t.Run("NestedOnly", func(t *testing.T) {
+		in := Params{
+			"dummy": Params{
+				"StringOpt": "nested_hello",
+				"IntOpt":    100,
+				"BoolOpt":   false,
+				"SliceOpt":  []string{"x", "y"},
+			},
+		}
+		var opt dummyOptions
+		err := ParseOptions(in, "dummy", &opt)
+		require.NoError(t, err)
+		assert.Equal(t, "nested_hello", opt.StringOpt)
+		assert.Equal(t, 100, opt.IntOpt)
+		assert.False(t, opt.BoolOpt)
+		assert.Equal(t, []string{"x", "y"}, opt.SliceOpt)
+		assert.Empty(t, in)
+	})
+
+	t.Run("BothPrecedence", func(t *testing.T) {
+		in := Params{
+			"string_opt": "flat_value",
+			"int_opt":    42,
+			"dummy": Params{
+				"StringOpt": "nested_value",
+			},
+		}
+		var opt dummyOptions
+		err := ParseOptions(in, "dummy", &opt)
+		require.NoError(t, err)
+		assert.Equal(t, "nested_value", opt.StringOpt) // Nested takes precedence
+		assert.Equal(t, 42, opt.IntOpt)                // Flat is still parsed
+		assert.Empty(t, in)
+	})
+
+	t.Run("MapSkip", func(t *testing.T) {
+		in := Params{
+			"string_opt": Params{"nested_key": "some_value"}, // string_opt matches but value is a map, should be skipped
+			"int_opt":    42,
+		}
+		var opt dummyOptions
+		err := ParseOptions(in, "dummy", &opt)
+		require.NoError(t, err)
+		assert.Equal(t, "", opt.StringOpt) // skipped
+		assert.Equal(t, 42, opt.IntOpt)
+		assert.Len(t, in, 1)
+		assert.Contains(t, in, "string_opt")
+	})
+
+	t.Run("NullErrors", func(t *testing.T) {
+		in := Params{
+			"string_opt": nil, // string_opt is nil, configstruct should return error
+		}
+		var opt dummyOptions
+		err := ParseOptions(in, "dummy", &opt)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "interpreting <nil> as string failed")
+	})
+
+	t.Run("UnknownLeftBehind", func(t *testing.T) {
+		in := Params{
+			"string_opt": "value",
+			"unknown":    "leftover",
+		}
+		var opt dummyOptions
+		err := ParseOptions(in, "dummy", &opt)
+		require.NoError(t, err)
+		assert.Equal(t, "value", opt.StringOpt)
+		assert.Len(t, in, 1)
+		assert.Equal(t, "leftover", in["unknown"])
+	})
+}
+
+func TestCheckParamsUsed(t *testing.T) {
+	t.Run("Empty", func(t *testing.T) {
+		in := Params{}
+		err := CheckParamsUsed(in)
+		assert.NoError(t, err)
+	})
+
+	t.Run("Leftovers", func(t *testing.T) {
+		in := Params{
+			"z": "last",
+			"a": "first",
+		}
+		err := CheckParamsUsed(in)
+		assert.Error(t, err)
+		assert.Equal(t, "unknown parameters: a, z", err.Error())
+	})
+}