diff --git a/mage/args_test.go b/mage/args_test.go index b8b421c7..f2df022f 100644 --- a/mage/args_test.go +++ b/mage/args_test.go @@ -33,6 +33,303 @@ not coughing } } +func TestVariadicArgs(t *testing.T) { + for _, tc := range []struct { + name string + args []string + want string + }{ + { + name: "empty", + args: []string{"variadic"}, + want: "variadic:[]\n", + }, + { + name: "all remaining tokens", + args: []string{"variadic", "first", "-flag", "variadic", "--"}, + want: "variadic:[\"first\" \"-flag\" \"variadic\" \"--\"]\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + Args: tc.args, + } + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + if got := stdout.String(); got != tc.want { + t.Fatalf("expected output %q, got %q", tc.want, got) + } + }) + } +} + +// TestOptionalVariadicArgs verifies implicit and explicit pass-through boundaries +// across empty, fixed-prefix, and typed optional-argument target signatures. +func TestOptionalVariadicArgs(t *testing.T) { + for _, tc := range []struct { + name string + args []string + want string + }{ + { + name: "implicit tail preserves later option-like tokens", + args: []string{ + "optionalandvariadic", "-prefix=chosen", + "first", "-unknown=value", "--", "fixed", + }, + want: "optional:chosen:[\"first\" \"-unknown=value\" \"--\" \"fixed\"]\n", + }, + { + name: "empty options and tail", + args: []string{"optionalandvariadic"}, + want: "optional::[]\n", + }, + { + name: "required prefix and typed options", + args: []string{ + "optionaltypes", "required", + "-text=value", "-count=2", "-ratio=1.5", "-enabled", "-timeout=25ms", + "first", "-count=99", + }, + want: "optionaltypes:required:value:2:1.5:true:25ms:[\"first\" \"-count=99\"]\n", + }, + { + name: "double dash starts an option-like tail", + args: []string{ + "optionalandvariadic", "-prefix=chosen", "--", + "-unknown=value", "--", "fixed", + }, + want: "optional:chosen:[\"-unknown=value\" \"--\" \"fixed\"]\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + code := Invoke(Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + Args: tc.args, + }) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + if got := stdout.String(); got != tc.want { + t.Fatalf("expected output %q, got %q", tc.want, got) + } + }) + } +} + +// TestOptionalVariadicArgsRejectInvalidOptionsBeforeTail verifies that option +// validation remains active until a variadic pass-through boundary is reached. +func TestOptionalVariadicArgsRejectInvalidOptionsBeforeTail(t *testing.T) { + for _, tc := range []struct { + name string + arg string + want string + }{ + { + name: "unknown", + arg: "-unknown=value", + want: "unknown option \"unknown\" for target \"OptionalAndVariadic\"\n", + }, + { + name: "missing equals", + arg: "-prefix", + want: "invalid option \"-prefix\" for target \"OptionalAndVariadic\", expected -name=value format\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + stderr := &bytes.Buffer{} + code := Invoke(Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: &bytes.Buffer{}, + Args: []string{"optionalandvariadic", tc.arg}, + }) + if code != 2 { + t.Fatalf("expected code 2, got %d; stderr: %s", code, stderr) + } + if got := stderr.String(); got != tc.want { + t.Fatalf("expected error %q, got %q", tc.want, got) + } + }) + } +} + +// TestDoubleDashRemainsInvalidForOptionalNonVariadicTarget verifies that the +// target-level separator does not change optional-only target semantics. +func TestDoubleDashRemainsInvalidForOptionalNonVariadicTarget(t *testing.T) { + stderr := &bytes.Buffer{} + code := Invoke(Invocation{ + Dir: "./testdata/optargs", + Stderr: stderr, + Stdout: &bytes.Buffer{}, + Args: []string{"greet", "World", "--"}, + }) + if code != 2 { + t.Fatalf("expected code 2, got %d; stderr: %s", code, stderr) + } + want := "invalid option \"--\" for target \"Greet\", expected -name=value format\n" + if got := stderr.String(); got != want { + t.Fatalf("expected error %q, got %q", want, got) + } +} + +// TestOptionalVariadicArgsHelp verifies source-parsed help for optional flags +// followed by the local variadic pass-through boundary. +func TestOptionalVariadicArgsHelp(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + code := Invoke(Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + Help: true, + Args: []string{"optionalandvariadic"}, + }) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + want := `OptionalAndVariadic prints an optional prefix and pass-through arguments. + +Usage: + + mage optionalandvariadic [-prefix=] [--] [...] + +Flags: + + -prefix= + +Pass-through: + + The first non-option token starts . To start with a "-" token, use --; the separator is omitted and all following tokens are passed unchanged. + +` + if got := stdout.String(); got != want { + t.Fatalf("expected output %q, got %q", want, got) + } +} + +func TestVariadicArgsHelp(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + Help: true, + Args: []string{"variadic"}, + } + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + want := `Variadic prints all remaining arguments. + +Usage: + + mage variadic [...] + +Aliases: v + +` + if got := stdout.String(); got != want { + t.Fatalf("expected output %q, got %q", want, got) + } +} + +func TestVariadicDefaultTarget(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + } + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + if got, want := stdout.String(), "variadic:[]\n"; got != want { + t.Fatalf("expected output %q, got %q", want, got) + } +} + +func TestVariadicArgsWithFixedPrefixAndPreviousTarget(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + Args: []string{"fixed", "before", "collect", "prefix", "-flag", "fixed", "after"}, + } + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + want := "fixed:before\ncollect:prefix:[\"-flag\" \"fixed\" \"after\"]\n" + if got := stdout.String(); got != want { + t.Fatalf("expected output %q, got %q", want, got) + } +} + +func TestVariadicTargetForms(t *testing.T) { + for _, tc := range []struct { + name string + args []string + want string + }{ + { + name: "alias", + args: []string{"v", "one", "two"}, + want: "variadic:[\"one\" \"two\"]\n", + }, + { + name: "namespace", + args: []string{"tools:collect", "prefix", "one", "two"}, + want: "tools:collect:prefix:[\"one\" \"two\"]\n", + }, + { + name: "imported", + args: []string{"shared:collect", "prefix", "one", "two"}, + want: "shared:collect:prefix:[\"one\" \"two\"]\n", + }, + { + name: "fixed argument types", + args: []string{"types", "3", "1.5", "true", "25ms", "one", "two"}, + want: "types:3:1.5:true:25ms:[\"one\" \"two\"]\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + Args: tc.args, + } + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + if got := stdout.String(); got != tc.want { + t.Fatalf("expected output %q, got %q", tc.want, got) + } + }) + } +} + func TestBadIntArg(t *testing.T) { stderr := &bytes.Buffer{} stdout := &bytes.Buffer{} diff --git a/mage/main.go b/mage/main.go index b51ea95c..62b5e68f 100644 --- a/mage/main.go +++ b/mage/main.go @@ -633,18 +633,7 @@ func mageHelpOutput(data mainfileTemplateData, target string) (output string, co } // Build usage line matching template format. - _, _ = fmt.Fprintf(&buf, "Usage:\n\n\t%s %s", data.BinaryName, strings.ToLower(fn.TargetName())) - for _, a := range fn.RequiredArgs() { - _, _ = fmt.Fprintf(&buf, " <%s>", a.Name) - } - if fn.MultipleOptionalArgs() { - _, _ = fmt.Fprint(&buf, " []") - } else { - for _, a := range fn.OptionalArgs() { - _, _ = fmt.Fprintf(&buf, " [-%s=<%s>]", a.Name, a.Type) - } - } - _, _ = fmt.Fprint(&buf, "\n\n") + _, _ = fmt.Fprintf(&buf, "Usage:\n\n\t%s %s%s\n\n", data.BinaryName, strings.ToLower(fn.TargetName()), fn.UsageArgs()) if fn.ShowFlagDocs() { _, _ = fmt.Fprint(&buf, fn.FlagDocsString()) diff --git a/mage/template.go b/mage/template.go index 4778fca3..770d8a53 100644 --- a/mage/template.go +++ b/mage/template.go @@ -247,7 +247,7 @@ Options: _fmt.Println({{printf "%q" .Comment}}) _fmt.Println() {{end}} - _fmt.Print("Usage:\n\n\t{{$.BinaryName}} {{lower .TargetName}}{{range .RequiredArgs}} <{{.Name}}>{{end}}{{if .MultipleOptionalArgs}} []{{else}}{{range .OptionalArgs}} [-{{.Name}}=<{{.Type}}>]{{end}}{{end}}\n\n") + _fmt.Print("Usage:\n\n\t{{$.BinaryName}} {{lower .TargetName}}{{.UsageArgs}}\n\n") {{if .ShowFlagDocs}}_fmt.Print({{printf "%q" .FlagDocsString}}) {{end -}} var aliases []string @@ -269,7 +269,7 @@ Options: _fmt.Println({{printf "%q" .Comment}}) _fmt.Println() {{end}} - _fmt.Print("Usage:\n\n\t{{$.BinaryName}} {{lower .TargetName}}{{range .RequiredArgs}} <{{.Name}}>{{end}}{{if .MultipleOptionalArgs}} []{{else}}{{range .OptionalArgs}} [-{{.Name}}=<{{.Type}}>]{{end}}{{end}}\n\n") + _fmt.Print("Usage:\n\n\t{{$.BinaryName}} {{lower .TargetName}}{{.UsageArgs}}\n\n") {{if .ShowFlagDocs}}_fmt.Print({{printf "%q" .FlagDocsString}}) {{end -}} var aliases []string @@ -300,6 +300,9 @@ Options: } return } + {{- if .DefaultFunc.VariadicArgs}} + x := 0 + {{- end}} {{.DefaultFunc.ExecCode}} handleError(logger, ret) return diff --git a/mage/testdata/variadic/imported/targets.go b/mage/testdata/variadic/imported/targets.go new file mode 100644 index 00000000..e56ce62d --- /dev/null +++ b/mage/testdata/variadic/imported/targets.go @@ -0,0 +1,8 @@ +package variadicimport + +import "fmt" + +// Collect prints imported variadic arguments. +func Collect(prefix string, args ...string) { + fmt.Printf("shared:collect:%s:%q\n", prefix, args) +} diff --git a/mage/testdata/variadic/magefile.go b/mage/testdata/variadic/magefile.go new file mode 100644 index 00000000..eeb189ce --- /dev/null +++ b/mage/testdata/variadic/magefile.go @@ -0,0 +1,96 @@ +//go:build mage +// +build mage + +package main + +import ( + "context" + "fmt" + "time" + + "github.com/magefile/mage/mg" + //mage:import shared + _ "github.com/magefile/mage/mage/testdata/variadic/imported" +) + +var Default = Variadic + +var Aliases = map[string]interface{}{ + "v": Variadic, +} + +// Variadic prints all remaining arguments. +func Variadic(args ...string) { + fmt.Printf("variadic:%q\n", args) +} + +// Fixed prints one fixed argument. +func Fixed(value string) { + fmt.Printf("fixed:%s\n", value) +} + +// Collect prints its fixed prefix and all remaining arguments. +func Collect(ctx context.Context, prefix string, args ...string) error { + fmt.Printf("collect:%s:%q\n", prefix, args) + return nil +} + +// Types prints converted fixed arguments followed by variadic arguments. +func Types(count int, ratio float64, enabled bool, timeout time.Duration, args ...string) { + fmt.Printf("types:%d:%.1f:%t:%s:%q\n", count, ratio, enabled, timeout, args) +} + +// Tools groups variadic targets. +type Tools mg.Namespace + +// Collect prints namespace arguments. +func (Tools) Collect(prefix string, args ...string) { + fmt.Printf("tools:collect:%s:%q\n", prefix, args) +} + +// OptionalAndVariadic prints an optional prefix and pass-through arguments. +func OptionalAndVariadic(prefix *string, args ...string) { + value := "" + if prefix != nil { + value = *prefix + } + fmt.Printf("optional:%s:%q\n", value, args) +} + +// OptionalTypes prints fixed, optional, and pass-through arguments. +func OptionalTypes( + ctx context.Context, + name string, + text *string, // text value + count *int, // count value + ratio *float64, // ratio value + enabled *bool, // enabled value + timeout *time.Duration, // timeout value + args ...string, +) error { + _ = ctx + textValue := "" + countValue := "" + ratioValue := "" + enabledValue := "" + timeoutValue := "" + if text != nil { + textValue = *text + } + if count != nil { + countValue = fmt.Sprint(*count) + } + if ratio != nil { + ratioValue = fmt.Sprint(*ratio) + } + if enabled != nil { + enabledValue = fmt.Sprint(*enabled) + } + if timeout != nil { + timeoutValue = timeout.String() + } + fmt.Printf("optionaltypes:%s:%s:%s:%s:%s:%s:%q\n", name, textValue, countValue, ratioValue, enabledValue, timeoutValue, args) + return nil +} + +func VariadicInt(args ...int) {} diff --git a/mage/testdata/variadic_dupe_alias/magefile.go b/mage/testdata/variadic_dupe_alias/magefile.go new file mode 100644 index 00000000..a9420998 --- /dev/null +++ b/mage/testdata/variadic_dupe_alias/magefile.go @@ -0,0 +1,12 @@ +//go:build mage +// +build mage + +package main + +var Aliases = map[string]interface{}{ + "BUILD": Existing, +} + +func Existing() {} + +func Build(args ...string) {} diff --git a/mage/testdata/variadic_dupe_import/imported/targets.go b/mage/testdata/variadic_dupe_import/imported/targets.go new file mode 100644 index 00000000..02c59601 --- /dev/null +++ b/mage/testdata/variadic_dupe_import/imported/targets.go @@ -0,0 +1,3 @@ +package imported + +func Build() {} diff --git a/mage/testdata/variadic_dupe_import/magefile.go b/mage/testdata/variadic_dupe_import/magefile.go new file mode 100644 index 00000000..e33125e8 --- /dev/null +++ b/mage/testdata/variadic_dupe_import/magefile.go @@ -0,0 +1,11 @@ +//go:build mage +// +build mage + +package main + +import ( + //mage:import + _ "github.com/magefile/mage/mage/testdata/variadic_dupe_import/imported" +) + +func Build(args ...string) {} diff --git a/mage/variadic_test.go b/mage/variadic_test.go new file mode 100644 index 00000000..0ccd1b4e --- /dev/null +++ b/mage/variadic_test.go @@ -0,0 +1,259 @@ +package mage + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func TestVariadicTargetDiscovery(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + List: true, + } + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + want := `Targets: + collect prints its fixed prefix and all remaining arguments. + fixed prints one fixed argument. + optionalAndVariadic prints an optional prefix and pass-through arguments. + optionalTypes prints fixed, optional, and pass-through arguments. + shared:collect prints imported variadic arguments. + tools:collect prints namespace arguments. + types prints converted fixed arguments followed by variadic arguments. + variadic* prints all remaining arguments. + +* default target +` + if got := stdout.String(); got != want { + t.Fatalf("expected output %q, got %q", want, got) + } +} + +// TestUnsupportedNonStringVariadicTargetStaysHidden verifies that Ticket 2 does +// not make unsupported variadic element types discoverable. +func TestUnsupportedNonStringVariadicTargetStaysHidden(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/variadic", + Stderr: stderr, + Stdout: stdout, + Help: true, + Args: []string{"variadicint"}, + } + code := Invoke(inv) + if code != 2 { + t.Fatalf("expected code 2, got %d; stdout: %s; stderr: %s", code, stdout, stderr) + } + want := "Unknown target: \"variadicint\"\n" + if got := stderr.String(); got != want { + t.Fatalf("expected error %q, got %q", want, got) + } +} + +func TestVariadicArgsThroughParseAndRun(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + code := ParseAndRun(stdout, stderr, nil, []string{ + "-d", "testdata/variadic", + "fixed", "before", + "collect", "prefix", "-flag", "--", "fixed", "after", + }) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + want := "fixed:before\ncollect:prefix:[\"-flag\" \"--\" \"fixed\" \"after\"]\n" + if got := stdout.String(); got != want { + t.Fatalf("expected output %q, got %q", want, got) + } +} + +// TestOptionalVariadicArgsThroughParseAndRun verifies the target-level separator +// through the public raw-command-line entry point. +func TestOptionalVariadicArgsThroughParseAndRun(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + code := ParseAndRun(stdout, stderr, nil, []string{ + "-d", "testdata/variadic", + "optionalandvariadic", "-prefix=chosen", "--", "-unknown=value", "fixed", + }) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + want := "optional:chosen:[\"-unknown=value\" \"fixed\"]\n" + if got := stdout.String(); got != want { + t.Fatalf("expected output %q, got %q", want, got) + } +} + +// TestOptionalVariadicChangePreservesExistingDispatchThroughParseAndRun verifies +// optional-only and fixed-arity multiple-target compatibility. +func TestOptionalVariadicChangePreservesExistingDispatchThroughParseAndRun(t *testing.T) { + for _, tc := range []struct { + name string + args []string + want string + }{ + { + name: "optional-only target followed by another target", + args: []string{ + "-d", "testdata/optargs", + "say", "hello", "-cap", "announce", "world", + }, + want: "HELLO\nAnnouncement: world\n", + }, + { + name: "fixed-arity multiple targets", + args: []string{ + "-d", "testdata/args", + "status", "say", "hi", "bob", + }, + want: "status\nsaying hi bob\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + code := ParseAndRun(stdout, stderr, nil, tc.args) + if code != 0 { + t.Fatalf("expected code 0, got %d; stderr: %s", code, stderr) + } + if got := stdout.String(); got != tc.want { + t.Fatalf("expected output %q, got %q", tc.want, got) + } + }) + } +} + +func TestVariadicHelpMatchesCompiledBinary(t *testing.T) { + dir := "./testdata/variadic" + name := filepath.Join(t.TempDir(), "mage_variadic_help_test") + if runtime.GOOS == "windows" { + name += ".exe" + } + + stderr := &bytes.Buffer{} + code := Invoke(Invocation{ + Dir: dir, + Stdout: os.Stdout, + Stderr: stderr, + CompileOut: name, + }) + if code != 0 { + t.Fatalf("compile failed with code %d: %s", code, stderr) + } + + for _, tc := range []struct { + target string + contains []string + notContains []string + }{ + { + target: "collect", + contains: []string{"BINARY collect [...]"}, + notContains: []string{ + "BINARY collect [--]", + "Pass-through:", + }, + }, + { + target: "optionalandvariadic", + contains: []string{ + "BINARY optionalandvariadic [-prefix=] [--] [...]", + "The first non-option token starts ", + "the separator is omitted and all following tokens are passed unchanged", + }, + }, + } { + t.Run(tc.target, func(t *testing.T) { + stdout := &bytes.Buffer{} + stderr.Reset() + code := Invoke(Invocation{ + Dir: dir, + Stdout: stdout, + Stderr: stderr, + Help: true, + Args: []string{tc.target}, + }) + if code != 0 { + t.Fatalf("mage help failed with code %d: %s", code, stderr) + } + mageOutput := stdout.String() + + stdout.Reset() + stderr.Reset() + cmd := exec.CommandContext(context.Background(), name, "-h", tc.target) + cmd.Env = os.Environ() + cmd.Stdout = stdout + cmd.Stderr = stderr + if err := cmd.Run(); err != nil { + t.Fatalf("compiled binary help failed: %v; stderr: %s", err, stderr) + } + compiledOutput := stdout.String() + + binaryBase := filepath.Base(name) + normalizedMage := strings.ReplaceAll(mageOutput, "\tmage ", "\tBINARY ") + normalizedCompiled := strings.ReplaceAll(compiledOutput, "\t"+binaryBase+" ", "\tBINARY ") + if normalizedMage != normalizedCompiled { + t.Fatalf("help output mismatch:\nmage: %q\ncompiled: %q", mageOutput, compiledOutput) + } + for _, want := range tc.contains { + if !strings.Contains(normalizedMage, want) { + t.Fatalf("help missing %q: %q", want, mageOutput) + } + } + for _, unwanted := range tc.notContains { + if strings.Contains(normalizedMage, unwanted) { + t.Fatalf("help unexpectedly contains %q: %q", unwanted, mageOutput) + } + } + }) + } +} + +func TestVariadicTargetDuplicateDiagnostics(t *testing.T) { + for _, tc := range []struct { + name string + dir string + want string + }{ + { + name: "alias", + dir: "./testdata/variadic_dupe_alias", + want: `alias "BUILD" duplicates existing target(s): .Build`, + }, + { + name: "imported target", + dir: "./testdata/variadic_dupe_import", + want: `"build" target has multiple definitions`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + stderr := &bytes.Buffer{} + code := Invoke(Invocation{ + Dir: tc.dir, + Stdout: &bytes.Buffer{}, + Stderr: stderr, + List: true, + }) + if code != 1 { + t.Fatalf("expected code 1, got %d; stderr: %s", code, stderr) + } + if got := stderr.String(); !strings.Contains(got, tc.want) { + t.Fatalf("expected error containing %q, got %q", tc.want, got) + } + }) + } +} diff --git a/parse/parse.go b/parse/parse.go index b618511d..78239dc0 100644 --- a/parse/parse.go +++ b/parse/parse.go @@ -78,6 +78,7 @@ func (s Functions) Swap(i, j int) { type Arg struct { Name, Type string Optional bool + Variadic bool Comment string } @@ -111,7 +112,7 @@ func (f Function) TargetName() string { func (f Function) NumRequiredArgs() int { n := 0 for _, a := range f.Args { - if !a.Optional { + if !a.Optional && !a.Variadic { n++ } } @@ -122,13 +123,47 @@ func (f Function) NumRequiredArgs() int { func (f Function) RequiredArgs() []Arg { var out []Arg for _, a := range f.Args { - if !a.Optional { + if !a.Optional && !a.Variadic { out = append(out, a) } } return out } +// VariadicArgs returns the terminal variadic argument, when present. +func (f Function) VariadicArgs() []Arg { + var out []Arg + for _, a := range f.Args { + if a.Variadic { + out = append(out, a) + } + } + return out +} + +// UsageArgs returns the command-line usage suffix for the function's arguments. +func (f Function) UsageArgs() string { + var out strings.Builder + for _, a := range f.RequiredArgs() { + _, _ = fmt.Fprintf(&out, " <%s>", a.Name) + } + if f.MultipleOptionalArgs() { + _, _ = fmt.Fprint(&out, " []") + } else { + for _, a := range f.OptionalArgs() { + _, _ = fmt.Fprintf(&out, " [-%s=<%s>]", a.Name, a.Type) + } + } + variadic := f.VariadicArgs() + if f.HasOptionalArgs() && len(variadic) > 0 { + _, _ = fmt.Fprint(&out, " [--]") + } + for _, a := range variadic { + _, _ = fmt.Fprintf(&out, " [<%s>...]", a.Name) + } + return out.String() +} + // OptionalArgs returns only the optional arguments. func (f Function) OptionalArgs() []Arg { var out []Arg @@ -169,7 +204,7 @@ func (f Function) MultipleOptionalArgs() bool { // condensed to [] in the usage line) or when any optional arg // has a doc comment. func (f Function) ShowFlagDocs() bool { - if f.MultipleOptionalArgs() { + if f.MultipleOptionalArgs() || (f.HasOptionalArgs() && len(f.VariadicArgs()) > 0) { return true } for _, a := range f.Args { @@ -180,8 +215,9 @@ func (f Function) ShowFlagDocs() bool { return false } -// FlagDocsString returns a formatted string documenting optional arguments. -// It aligns comments to the same column based on the longest flag name. +// FlagDocsString returns a formatted string documenting optional arguments +// and, when combined with a variadic argument, the pass-through boundary. It +// aligns comments to the same column based on the longest flag name. func (f Function) FlagDocsString() string { opts := f.OptionalArgs() if len(opts) == 0 { @@ -212,6 +248,10 @@ func (f Function) FlagDocsString() string { } } _, _ = buf.WriteString("\n") + variadic := f.VariadicArgs() + if len(variadic) > 0 { + _, _ = fmt.Fprintf(&buf, "Pass-through:\n\n\tThe first non-option token starts <%s>. To start with a \"-\" token, use --; the separator is omitted and all following tokens are passed unchanged.\n\n", variadic[0].Name) + } return buf.String() } @@ -234,6 +274,9 @@ func (f Function) ExecCode() string { if arg.Optional { continue } + if arg.Variadic { + continue + } switch arg.Type { case "string": _, _ = fmt.Fprintf(&parseargs, ` @@ -296,7 +339,15 @@ func (f Function) ExecCode() string { } _, _ = fmt.Fprint(&parseargs, ` - for x < len(args.Args) && _strings.HasPrefix(args.Args[x], "-") { + for x < len(args.Args) && _strings.HasPrefix(args.Args[x], "-") {`) + if len(f.VariadicArgs()) > 0 { + _, _ = fmt.Fprint(&parseargs, ` + if args.Args[x] == "--" { + x++ + break + }`) + } + _, _ = fmt.Fprint(&parseargs, ` _optArg := args.Args[x] _eqIdx := _strings.Index(_optArg, "=") var _optName, _optVal string @@ -379,6 +430,17 @@ func (f Function) ExecCode() string { }`, f.TargetName()) } + // Phase 4: The terminal variadic argument owns every token left after + // required arguments and optional flags have been consumed. + for x, arg := range f.Args { + if !arg.Variadic { + continue + } + _, _ = fmt.Fprintf(&parseargs, ` + arg%d := args.Args[x:] + x = len(args.Args)`, x) + } + out := parseargs.String() + ` wrapFn := func(ctx _context.Context) error { ` @@ -391,7 +453,11 @@ func (f Function) ExecCode() string { args = append(args, "ctx") } for x := 0; x < len(f.Args); x++ { - args = append(args, fmt.Sprintf("arg%d", x)) + arg := fmt.Sprintf("arg%d", x) + if f.Args[x].Variadic { + arg += "..." + } + args = append(args, arg) } out += strings.Join(args, ", ") out += ")" @@ -418,6 +484,9 @@ func PrimaryPackage(gocmd, path string, files []string, multiline bool) (*PkgInf setDefault(info) setAliases(info) + if err := checkVariadicAliasDupes(info); err != nil { + return nil, err + } return info, nil } @@ -432,16 +501,6 @@ func checkDupes(info *PkgInfo, imports []*Import) error { funcs[target] = append(funcs[target], f) } } - for alias, f := range info.Aliases { - if len(funcs[alias]) != 0 { - var ids []string - for _, f := range funcs[alias] { - ids = append(ids, f.ID()) - } - return fmt.Errorf("alias %q duplicates existing target(s): %s", alias, strings.Join(ids, ", ")) - } - funcs[alias] = append(funcs[alias], f) - } var dupes []string for target, list := range funcs { if len(list) > 1 { @@ -464,6 +523,43 @@ func checkDupes(info *PkgInfo, imports []*Import) error { return errors.New(strings.Join(errs, "\n")) } +// checkVariadicAliasDupes reports aliases that conflict with targets made +// discoverable by variadic argument support. Fixed-arity alias collisions keep +// their historical behavior. +func checkVariadicAliasDupes(info *PkgInfo) error { + variadicTargets := map[string][]*Function{} + addVariadicTargets := func(funcs Functions) { + for _, f := range funcs { + if len(f.VariadicArgs()) > 0 { + target := strings.ToLower(f.TargetName()) + variadicTargets[target] = append(variadicTargets[target], f) + } + } + } + addVariadicTargets(info.Funcs) + for _, imp := range info.Imports { + addVariadicTargets(imp.Info.Funcs) + } + aliases := make([]string, 0, len(info.Aliases)) + for alias := range info.Aliases { + aliases = append(aliases, alias) + } + sort.Strings(aliases) + for _, alias := range aliases { + funcs := variadicTargets[strings.ToLower(alias)] + if len(funcs) == 0 { + continue + } + ids := make([]string, 0, len(funcs)) + for _, f := range funcs { + ids = append(ids, f.ID()) + } + sort.Strings(ids) + return fmt.Errorf("alias %q duplicates existing target(s): %s", alias, strings.Join(ids, ", ")) + } + return nil +} + // Package compiles information about a mage package. func Package(path string, files []string, multiline bool) (*PkgInfo, error) { start := time.Now() @@ -1092,6 +1188,20 @@ func funcType(ft *ast.FuncType, fieldComments map[*ast.Field]string) (*Function, } for ; x < len(ft.Params.List); x++ { param := ft.Params.List[x] + if ellipsis, ok := param.Type.(*ast.Ellipsis); ok { + if x != len(ft.Params.List)-1 { + return nil, errors.New("variadic argument must be last") + } + t := fmt.Sprint(ellipsis.Elt) + typ, ok := argTypes[t] + if !ok || typ != "string" { + return nil, fmt.Errorf("unsupported variadic argument type: %s", t) + } + for _, name := range param.Names { + f.Args = append(f.Args, Arg{Name: name.Name, Type: typ, Variadic: true}) + } + continue + } optional := false paramType := param.Type // Check for pointer types (optional arguments) diff --git a/parse/parse_test.go b/parse/parse_test.go index 91bdc9f3..14bb94fc 100644 --- a/parse/parse_test.go +++ b/parse/parse_test.go @@ -357,3 +357,90 @@ func TestOptionalArgs(t *testing.T) { } } } + +func TestVariadicStringArgs(t *testing.T) { + info, err := PrimaryPackage("go", "./testdata", []string{"variadic.go"}, false) + if err != nil { + t.Fatal(err) + } + + expected := []Function{ + { + Name: "OptionalAndVariadic", + Args: []Arg{ + {Name: "prefix", Type: "string", Optional: true}, + {Name: "args", Type: "string", Variadic: true}, + }, + }, + { + Name: "OptionalTypes", + IsError: true, + IsContext: true, + Synopsis: "exercises every supported pointer-style optional argument type before a terminal variadic string argument.", + Comment: "OptionalTypes exercises every supported pointer-style optional argument type before a terminal variadic string argument.", + Args: []Arg{ + {Name: "name", Type: "string"}, + {Name: "text", Type: "string", Optional: true}, + {Name: "count", Type: "int", Optional: true}, + {Name: "ratio", Type: "float64", Optional: true}, + {Name: "enabled", Type: "bool", Optional: true}, + {Name: "timeout", Type: "time.Duration", Optional: true}, + {Name: "args", Type: "string", Variadic: true}, + }, + }, + { + Name: "Variadic", + Args: []Arg{ + {Name: "args", Type: "string", Variadic: true}, + }, + }, + { + Name: "VariadicWithPrefix", + IsError: true, + IsContext: true, + Args: []Arg{ + {Name: "name", Type: "string"}, + {Name: "args", Type: "string", Variadic: true}, + }, + }, + { + Name: "Run", + Receiver: "VariadicNamespace", + Args: []Arg{ + {Name: "args", Type: "string", Variadic: true}, + }, + }, + } + + if len(info.Funcs) != len(expected) { + t.Fatalf("expected %d funcs, got %d: %#v", len(expected), len(info.Funcs), info.Funcs) + } + for _, fn := range expected { + found := false + for _, infoFn := range info.Funcs { + if reflect.DeepEqual(fn, *infoFn) { + found = true + break + } + } + if !found { + t.Errorf("expected:\n%#v\n\nto be in parsed funcs", fn) + } + } +} + +// TestFixedTargetAliasCollisionRemainsSupported verifies that variadic target +// discovery does not reject an existing fixed-target alias configuration. +func TestFixedTargetAliasCollisionRemainsSupported(t *testing.T) { + info, err := PrimaryPackage("go", "./testdata", []string{"fixed_alias_collision.go"}, false) + if err != nil { + t.Fatal(err) + } + alias, ok := info.Aliases["BUILD"] + if !ok { + t.Fatal("expected BUILD alias") + } + if alias.Name != "Existing" { + t.Fatalf("expected BUILD to alias Existing, got %s", alias.Name) + } +} diff --git a/parse/testdata/fixed_alias_collision.go b/parse/testdata/fixed_alias_collision.go new file mode 100644 index 00000000..db4dc817 --- /dev/null +++ b/parse/testdata/fixed_alias_collision.go @@ -0,0 +1,16 @@ +//go:build mage +// +build mage + +package main + +// Aliases preserves the historical behavior where an alias may shadow a +// fixed-arity target with the same case-insensitive name. +var Aliases = map[string]interface{}{ + "BUILD": Existing, +} + +// Existing is the target selected through the BUILD alias. +func Existing() {} + +// Build is the fixed-arity target shadowed by the BUILD alias. +func Build() {} diff --git a/parse/testdata/variadic.go b/parse/testdata/variadic.go new file mode 100644 index 00000000..e7d98b0a --- /dev/null +++ b/parse/testdata/variadic.go @@ -0,0 +1,46 @@ +//go:build mage +// +build mage + +package main + +import ( + "context" + "time" + + "github.com/magefile/mage/mg" +) + +func Variadic(args ...string) {} + +func VariadicWithPrefix(ctx context.Context, name string, args ...string) error { + return nil +} + +type VariadicNamespace mg.Namespace + +func (VariadicNamespace) Run(args ...string) {} + +func OptionalAndVariadic(prefix *string, args ...string) {} + +// OptionalTypes exercises every supported pointer-style optional argument type +// before a terminal variadic string argument. +func OptionalTypes( + ctx context.Context, + name string, + text *string, + count *int, + ratio *float64, + enabled *bool, + timeout *time.Duration, + args ...string, +) error { + return nil +} + +func VariadicInt(args ...int) {} + +func VariadicFloat64(args ...float64) {} + +func VariadicBool(args ...bool) {} + +func VariadicDuration(args ...time.Duration) {} diff --git a/site/content/targets/_index.en.md b/site/content/targets/_index.en.md index f427cce8..21d69fe5 100644 --- a/site/content/targets/_index.en.md +++ b/site/content/targets/_index.en.md @@ -5,7 +5,9 @@ weight = 10 A target is any exported function that has an optional first argument of context.Context, has either no return or just an error return, and where the arguments are all of type `string`, `int`, `float64`, `bool`, or `time.Duration`. Pointer types of these (`*string`, `*int`, `*float64`, `*bool`, `*time.Duration`) are -also accepted and treated as optional arguments (see [Optional Arguments](#optional-arguments) below). +also accepted and treated as optional arguments (see [Optional Arguments](#optional-arguments) below). A target +may end with one `...string` argument, which receives the remaining command-line tokens. Pointer-style optional +arguments may precede that variadic argument. e.g. these are all acceptable targets @@ -15,6 +17,8 @@ func Install(ctx context.Context) error func Run(what string) error func Exec(ctx context.Context, name string, count int, debug bool, timeout time.Duration) error func Greet(name string, greeting *string) +func RunAll(ctx context.Context, prefix string, args ...string) error +func RunWithOptions(ctx context.Context, prefix *string, args ...string) error ``` A target is effectively a subcommand of mage while running mage in @@ -36,6 +40,50 @@ You can intersperse multiple targets with arguments as you'd expect: `mage run foo.exe exec somename 5 true 100ms` +### Variadic arguments + +A target may declare one terminal `...string` argument. After Mage consumes the +target's fixed arguments and any pointer-style optional flags, it passes the +variadic tail unchanged and in order. The variadic argument may be empty. + +```go +func Run(prefix string, args ...string) { + fmt.Printf("prefix=%s args=%q\n", prefix, args) +} +``` + +```plain +$ mage run go test ./... -race +prefix=go args=["test" "./..." "-race"] +``` + +A variadic target is terminal for that Mage invocation. Fixed-arity targets may +run before it, but tokens after its name and fixed arguments are never parsed as +later targets—even when a token matches another target name. Tokens beginning +with `-`, including `--`, are ordinary values for a variadic target without +pointer-style optional arguments. + +When a variadic target has pointer-style optional arguments, Mage parses its +optional flags before starting the variadic tail. The first non-option token +starts the tail implicitly. If the first tail token begins with `-`, use `--` to +end option parsing explicitly; that separator is omitted, and every later token +is passed through unchanged. See [Flags](#flags-v1160) for examples and the +exact scope of this separator. + +The function signature is the argument-ownership boundary. Mage does not guess +the boundary from known target names, require a global `--` separator, or expose +the remaining values through a global or shared extra-arguments accessor. This +keeps a command's meaning stable when other targets are added or renamed. + +This support makes existing exported functions ending in `...string`, including +functions that combine pointer-style optional arguments with `...string`, +visible as targets. Such functions can now appear in `mage -l` and `mage -h`, +and can expose an existing case-insensitive target or alias name conflict. +Rename an exported helper or make it unexported if it is not intended to be a +Mage target. + +Variadic arguments of other types are not targets. + ### Flags (v1.16.0+) You can define flags (optional arguments) by using pointer types for any of the @@ -72,6 +120,45 @@ signature. Required arguments are always positional, while optional arguments us the `-name=value` flag syntax and can appear in any order after the required arguments. +An optional target may also end with a terminal `...string` argument: + +```go +func Run(ctx context.Context, name string, verbose *bool, args ...string) error { + verboseValue := "" + if verbose != nil { + verboseValue = fmt.Sprint(*verbose) + } + fmt.Printf("name=%s verbose=%s args=%q\n", name, verboseValue, args) + return nil +} +``` + +The first non-option token starts the variadic tail, so flags must come first. +Once the tail starts, Mage preserves every remaining token without trying to +parse more flags or targets: + +```plain +$ mage run worker -verbose input.txt -literal build +name=worker verbose=true args=["input.txt" "-literal" "build"] +``` + +If the tail itself must start with a token beginning in `-`, place a literal +`--` before it. The first `--` ends option parsing and is not passed to the +target; everything after it, including unknown flags, repeated `--` tokens, and +target names, is passed unchanged: + +```plain +$ mage run worker -- -literal -- build +name=worker verbose= args=["-literal" "--" "build"] +``` + +Before that boundary, unknown or malformed options retain the usual target +option errors. This target-level `--` is recognized only while a target that +combines pointer-style optional arguments and `...string` is parsing options. +For a variadic target without optional arguments, `--` remains an ordinary +variadic value. Non-variadic targets retain their existing option-error or +subsequent-target dispatch behavior. + ## Errors If the function has an error return, errors returned from the function will @@ -91,6 +178,8 @@ then once foo is done, bar, then once bar is done, baz). Dependencies run using mg.Deps will still only run once per mage execution, so if each of the targets depend on the same function, that function will only be run once for all targets. If any target panics or returns an error, no later targets will be run. +Because a variadic target consumes every remaining token, it must be the final +target in a multiple-target invocation. ## Contexts and Cancellation