nerdctl-stub: Allow positional arguments mixed with flags

scripts/windows/generate-nerdctl-stub.ps1

@@ -2,10 +2,14 @@
 # parsers.  This must be executed on Windows as we need a stable platform to be
 # able to find nerdctl.
 
+param(
+  [switch]$Verbose
+)
+
 $ENV:GOOS = "linux"
 
 Set-Location src/go/nerdctl-stub/generate
 go build .
-wsl.exe -d rancher-desktop --exec ./generate
+wsl.exe -d rancher-desktop --exec ./generate "-verbose=$Verbose"
 Remove-Item ./generate
 gofmt -w ../nerdctl_commands_generated.go

src/go/nerdctl-stub/generate/main_linux.go

@@ -32,6 +32,11 @@ type helpData struct {
 	// (`--version`) or the short option (`-v`), and the value is whether the
 	// option takes an argument.
 	Options map[string]bool
+	// If set, this command can have subcommands; this alters argument parsing.
+	canHaveSubcommands bool
+	// If set, this command can pass flags to foreign commands, as in `nerdctl run`.
+	// This alters argument parsing by letting us ignore unknown flags.
+	HasForeignFlags bool
 	// mergedOptions includes local options plus inherited options.
 	mergedOptions map[string]struct{}
 }
@@ -94,13 +99,27 @@ func main() {
 // writer is the file to write to for the result; it is expected that `go fmt`
 // will be run on it eventually.
 func buildSubcommand(args []string, parentData helpData, writer io.Writer) error {
-	logrus.WithField("args", args).Trace("building subcommand")
 	help, err := getHelp(args)
 	if err != nil {
 		return fmt.Errorf("error getting help for %v: %w", args, err)
 	}
 	subcommands := parseHelp(help, parentData)
 
+	fields := logrus.Fields{"args": args}
+	if subcommands.HasForeignFlags {
+		fields["type"] = "arguments"
+	} else if !subcommands.canHaveSubcommands {
+		fields["type"] = "positional"
+	}
+	logrus.WithFields(fields).Trace("building subcommand")
+
+	if !subcommands.canHaveSubcommands && len(subcommands.Commands) > 0 {
+		return fmt.Errorf("invalid command %v: has positional arguments, but also subcommands %+v", args, subcommands.Commands)
+	}
+	if subcommands.canHaveSubcommands && subcommands.HasForeignFlags {
+		return fmt.Errorf("invalid command %v: has subcommands and foreign flags", args)
+	}
+
 	err = emitCommand(args, subcommands, writer)
 	if err != nil {
 		return err
@@ -159,6 +178,19 @@ func parseHelp(help string, parentData helpData) helpData {
 				state = STATE_COMMANDS
 			} else if strings.HasSuffix(strings.ToUpper(line), "FLAGS:") {
 				state = STATE_OPTIONS
+			} else if strings.HasPrefix(strings.ToUpper(line), "USAGE:") {
+				// Usage is on the same line, so we have to process this now.
+				// Command is `nerdctl subcommand [flags]`; anything after `[flags]` is
+				// assumed to be positional arguments.
+				_, newArgs, _ := strings.Cut(strings.ToUpper(line), "[FLAGS]")
+				newArgs = strings.TrimSpace(newArgs)
+				// Unlike everything else, `nerdctl compose` has a usage string of
+				// `nerdctl compose [flags] COMMAND` so we need to ignore that.
+				if newArgs == "" || newArgs == "COMMAND" {
+					result.canHaveSubcommands = true
+				} else if strings.Contains(newArgs, "COMMAND") && strings.Contains(newArgs, "...") {
+					result.HasForeignFlags = true
+				}
 			} else {
 				state = STATE_OTHER
 			}
@@ -223,6 +255,9 @@ const commandTemplate = `
 				{{- printf "%q" $k -}}: {{ if $v -}} ignoredArgHandler {{- else -}} nil {{- end -}},
 			{{ end }}
 		},
+		{{- if .Data.HasForeignFlags }}
+			hasForeignFlags: true,
+		{{- end }}
 	},
 `
 

src/go/nerdctl-stub/parse_args.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"slices"
 	"strings"
 )
 
@@ -32,7 +33,7 @@ type argHandlersType struct {
 
 // commandHandlerType is the type of commandDefinition.handler, which is used
 // to handle positional arguments (and special subcommands).
-// The passed-in arguments include any flags given after positional arguments.
+// The passed-in arguments excludes any flags given after positional arguments.
 type commandHandlerType func(*commandDefinition, []string, argHandlersType) (*parsedArgs, error)
 
 type commandDefinition struct {
@@ -46,6 +47,10 @@ type commandDefinition struct {
 	// options for this (sub) command.  If the handler is null, the option does
 	// not take arguments.
 	options map[string]argHandler
+	// if set, this command can include foreign flags that should not be parsed.
+	// This should be set for things like `nerdctl run` where flags can be passed
+	// to the command to be run.
+	hasForeignFlags bool
 	// handler for any positional arguments and subcommands.  This should not
 	// include the name of the subcommand itself.  If this is not given, all
 	// subcommands are searched for, and positional arguments are ignored.
@@ -133,10 +138,28 @@ func (c *commandDefinition) parseOption(arg, next string) ([]string, bool, []cle
 // parse arguments for this command; this includes options (--long, -x) as well
 // as subcommands and positional arguments.
 func (c commandDefinition) parse(args []string) (*parsedArgs, error) {
+	// Parsing rules:
+	// - At each command level, short options (-x) at that level can be parsed.
+	// - At each command level, long options from the current or any previous level can be parsed.
+	// - If a command contains positional arguments, it may not contain any subcommands.
+	//   (We check this in `./generate` to make sure this stays true.)
+	// - Positional arguments can be intermixed with (both long and short) options.
+	// - If a command can have foreign flags (e.g. `nerdctl run`), we stop parsing
+	//   options on first positional argument.  This means we parse the flag in
+	//   `nerdctl run --env foo=bar image sh -c ...` but not the `--env` flag in
+	//   `nerdctl run image --env foo=bar sh -c ...`.
+	// - Having foreign flags is mutually exclusive with having subcommands; this
+	//   is also checked in `./generate`.
+	// - `--` stops parsing of options.
 	var result parsedArgs
+	var positionalArgs []string
 	for argIndex := 0; argIndex < len(args); argIndex++ {
 		arg := args[argIndex]
-		if strings.HasPrefix(arg, "-") {
+		if arg == "--" {
+			// No more options, only positional arguments.
+			positionalArgs = append(positionalArgs, args[argIndex+1:]...)
+			break
+		} else if strings.HasPrefix(arg, "-") {
 			next := ""
 			if argIndex+1 < len(args) {
 				next = args[argIndex+1]
@@ -157,18 +180,9 @@ func (c commandDefinition) parse(args []string) (*parsedArgs, error) {
 			if consumed {
 				argIndex++
 			}
-		} else {
-			// Handler positional arguments and subcommands.
-			if c.handler != nil {
-				childResult, err := c.handler(&c, args[argIndex:], argHandlers)
-				if err != nil {
-					return nil, err
-				}
-				result.args = append(result.args, childResult.args...)
-				result.cleanup = append(result.cleanup, childResult.cleanup...)
-				break
-			}
-			// No custom handler; look for subcommands.
+		} else if len(c.subcommands) > 0 {
+			// This command has subcommands; assume any non-flags are subcommands.
+			// Hand off argument parsing to the subcommand.
 			subcommandPath := c.commandPath
 			if subcommandPath != "" {
 				subcommandPath += " "
@@ -187,10 +201,38 @@ func (c commandDefinition) parse(args []string) (*parsedArgs, error) {
 				result.args = append(result.args, childResult.args...)
 				result.cleanup = append(result.cleanup, childResult.cleanup...)
 			} else {
-				// No subcommand; ignore positional arguments.
+				// Invalid subcommand; ignore positional arguments.
 				result.args = append(result.args, args[argIndex:]...)
 			}
 			break
+		} else {
+			if c.hasForeignFlags {
+				// If we have foreign flags, assume the rest of the arguments starting
+				// from the first positional argument is foreign.
+				positionalArgs = append(positionalArgs, args[argIndex:]...)
+				break
+			} else {
+				// This command doesn't have subcommands, nor foreign arguments.
+				// Everything is positional arguments; we still have to parse other
+				// arguments for flags, though.
+				positionalArgs = append(positionalArgs, arg)
+			}
+		}
+	}
+	// At this point, `result` is filled with options, and `positionalArgs`
+	// contains the unparsed positional arguments.
+	if c.handler != nil {
+		childResult, err := c.handler(&c, positionalArgs, argHandlers)
+		if err != nil {
+			return nil, err
+		}
+		result.args = append(result.args, childResult.args...)
+		result.cleanup = append(result.cleanup, childResult.cleanup...)
+	} else {
+		if len(positionalArgs) > 0 && !slices.Contains(result.args, "--") {
+			result.args = slices.Concat(result.args, []string{"--"}, positionalArgs)
+		} else {
+			result.args = append(result.args, positionalArgs...)
 		}
 	}
 	return &result, nil

src/go/nerdctl-stub/parse_args_test.go

@@ -202,7 +202,7 @@ func TestParse(t *testing.T) {
 		result, err := c.parse([]string{"hello", "world"})
 		assert.NoError(t, err)
 		if assert.NotNil(t, result) {
-			assert.Equal(t, []string{"hello", "world"}, result.args)
+			assert.Equal(t, []string{"--", "hello", "world"}, result.args)
 		}
 	})
 	t.Run("subcommand handler", func(t *testing.T) {
@@ -211,6 +211,9 @@ func TestParse(t *testing.T) {
 		localCommands := make(map[string]commandDefinition)
 		localCommands[""] = commandDefinition{
 			commands: &localCommands,
+			subcommands: map[string]struct{}{
+				"subcommand": {},
+			},
 		}
 		localCommands["subcommand"] = commandDefinition{
 			commands: &localCommands,
@@ -224,4 +227,67 @@ func TestParse(t *testing.T) {
 		assert.NoError(t, err)
 		assert.True(t, run)
 	})
+	t.Run("subcommand with mixed arguments", func(t *testing.T) {
+		t.Parallel()
+		var seenArgs []string
+		localCommands := make(map[string]commandDefinition)
+		localCommands[""] = commandDefinition{
+			commands: &localCommands,
+			subcommands: map[string]struct{}{
+				"subcommand": {},
+			},
+			options: map[string]argHandler{
+				"--foo": ignoredArgHandler,
+			},
+		}
+		localCommands["subcommand"] = commandDefinition{
+			commandPath: "subcommand",
+			commands:    &localCommands,
+			options: map[string]argHandler{
+				"--bar": ignoredArgHandler,
+			},
+			handler: func(cd *commandDefinition, s []string, argHandlers argHandlersType) (*parsedArgs, error) {
+				seenArgs = s
+				return &parsedArgs{}, nil
+			},
+		}
+		result, err := localCommands[""].parse([]string{"subcommand", "--foo", "FOO", "qq", "--bar", "BAR", "zz"})
+		if assert.NoError(t, err) {
+			assert.Equal(t, []string{"qq", "zz"}, seenArgs)
+			// Because we have a custom handler, they don't show up in result.args
+			assert.Equal(t, []string{"subcommand", "--foo", "FOO", "--bar", "BAR"}, result.args)
+		}
+	})
+	t.Run("subcommand with foreign flags", func(t *testing.T) {
+		t.Parallel()
+		var seenArgs []string
+		localCommands := make(map[string]commandDefinition)
+		localCommands[""] = commandDefinition{
+			commands: &localCommands,
+			subcommands: map[string]struct{}{
+				"subcommand": {},
+			},
+			options: map[string]argHandler{
+				"--foo": ignoredArgHandler,
+			},
+		}
+		localCommands["subcommand"] = commandDefinition{
+			commandPath: "subcommand",
+			commands:    &localCommands,
+			options: map[string]argHandler{
+				"--bar": ignoredArgHandler,
+			},
+			hasForeignFlags: true,
+			handler: func(cd *commandDefinition, s []string, argHandlers argHandlersType) (*parsedArgs, error) {
+				seenArgs = s
+				return &parsedArgs{}, nil
+			},
+		}
+		result, err := localCommands[""].parse([]string{"subcommand", "--foo", "FOO", "qq", "--bar", "BAR", "zz"})
+		if assert.NoError(t, err) {
+			assert.Equal(t, []string{"qq", "--bar", "BAR", "zz"}, seenArgs)
+			// Because we have a custom handler, they don't show up in result.args
+			assert.Equal(t, []string{"subcommand", "--foo", "FOO"}, result.args)
+		}
+	})
 }