Repository: alecthomas/kong (go)

Issue:
Optional arguments should not be allowed after repeated args
This makes no sense: `cmd <arg> ... [<last>]`. `<last>` will never be provided.

Pull request description:
Cumulative argument needs to be last
Closes #276

No other arguments can be after a cumulative argument.
This excludes required positional arguments such as `cmd <arg> ... <last>`.

I believe the best practice shall be - cumulative arguments are always last.
Let me know if you think required arguments shall be supported after cumulative. I'll be happy to update the PR.

Patch:
diff --git a/README.md b/README.md
index 5b6667a..099ac90 100644
--- a/README.md
+++ b/README.md
@@ -656,13 +656,14 @@ Example resolvers can be found in [resolver.go](https://github.com/alecthomas/ko
 Command-line arguments are mapped to Go values via the Mapper interface:
 
 ```go
-// A Mapper knows how to map command-line input to Go.
+// A Mapper represents how a field is mapped from command-line values to Go.
+//
+// Mappers can be associated with concrete fields via pointer, reflect.Type, reflect.Kind, or via a "type" tag.
+//
+// Additionally, if a type implements the MapperValue interface, it will be used.
 type Mapper interface {
-  // Decode scan into target.
-  //
-  // "ctx" contains context about the value being decoded that may be useful
-  // to some mappers.
-  Decode(ctx *MapperContext, scan *Scanner, target reflect.Value) error
+	// Decode ctx.Value with ctx.Scanner into target.
+	Decode(ctx *DecodeContext, target reflect.Value) error
 }
 ```
 
diff --git a/build.go b/build.go
index 32c83e1..2ab9f21 100644
--- a/build.go
+++ b/build.go
@@ -171,17 +171,33 @@ MAIN:
 		}
 	}
 
-	// Scan through argument positionals to ensure optional is never before a required.
+	if err := validatePositionalArguments(node); err != nil {
+		return nil, err
+	}
+
+	return node, nil
+}
+
+func validatePositionalArguments(node *Node) error {
 	var last *Value
 	for i, curr := range node.Positional {
-		if last != nil && !last.Required && curr.Required {
-			return nil, fmt.Errorf("%s: required %q can not come after optional %q", node.FullPath(), curr.Name, last.Name)
+		if last != nil {
+			// Scan through argument positionals to ensure optional is never before a required.
+			if !last.Required && curr.Required {
+				return fmt.Errorf("%s: required %q cannot come after optional %q", node.FullPath(), curr.Name, last.Name)
+			}
+
+			// Cumulative argument needs to be last.
+			if last.IsCumulative() {
+				return fmt.Errorf("%s: argument %q cannot come after cumulative %q", node.FullPath(), curr.Name, last.Name)
+			}
 		}
+
 		last = curr
 		curr.Position = i
 	}
 
-	return node, nil
+	return nil
 }
 
 func buildChild(k *Kong, node *Node, typ NodeType, v reflect.Value, ft reflect.StructField, fv reflect.Value, tag *Tag, name string, seenFlags map[string]bool) error {


Test patch:
diff --git a/kong_test.go b/kong_test.go
index 18ac9f1..cb7f204 100644
--- a/kong_test.go
+++ b/kong_test.go
@@ -1718,3 +1718,21 @@ func TestChildNameCanBeDuplicated(t *testing.T) {
 	}
 	mustNew(t, &cli)
 }
+
+func TestCumulativeArgumentLast(t *testing.T) {
+	var cli struct {
+		Arg1 string   `arg:""`
+		Arg2 []string `arg:""`
+	}
+	_, err := kong.New(&cli)
+	assert.NoError(t, err)
+}
+
+func TestCumulativeArgumentNotLast(t *testing.T) {
+	var cli struct {
+		Arg2 []string `arg:""`
+		Arg1 string   `arg:""`
+	}
+	_, err := kong.New(&cli)
+	assert.Error(t, err)
+}