gopls/internal/cmd: revamp CLI flag parsing Revamp gopls command line argument normalization and flag parsing. Separate global flags and subcommand arguments cleanly prior to flag parsing, and provide hierarchical contextual error messages for misplaced flags using parent lineage strings. For golang/go#79906 Change-Id: Iad8685503dc48ba41b851cb4e3aea172b305a5b9 Reviewed-on: https://go-review.googlesource.com/c/tools/+/794462 Reviewed-by: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/gopls/internal/cmd/cmd.go b/gopls/internal/cmd/cmd.go index b0d80d0..f4b2610 100644 --- a/gopls/internal/cmd/cmd.go +++ b/gopls/internal/cmd/cmd.go
@@ -22,8 +22,6 @@ "time" "golang.org/x/tools/gopls/internal/cache" - "golang.org/x/tools/gopls/internal/debug" - "golang.org/x/tools/gopls/internal/filecache" "golang.org/x/tools/gopls/internal/lsprpc" "golang.org/x/tools/gopls/internal/protocol" protocolcommand "golang.org/x/tools/gopls/internal/protocol/command" @@ -257,29 +255,10 @@ return value == z.Interface().(flag.Value).String() } -// Run takes the args after top level flag processing, and invokes the correct -// sub command as specified by the first argument. -// If no arguments are passed it will invoke the server sub command, as a -// temporary measure for compatibility. +// Run implements command, but should never be invoked directly. +// Main coordinates normalization and subcommand dispatching. func (app *application) Run(ctx context.Context, args ...string) error { - // In the category of "things we can do while waiting for the Go command": - // Pre-initialize the filecache, which takes ~50ms to hash the gopls - // executable, and immediately runs a gc. - filecache.Start() - - ctx = debug.WithInstance(ctx, app.OTel) - if len(args) == 0 { - s := flag.NewFlagSet(app.Name(), flag.ExitOnError) - return runCommand(ctx, s, &app.serve, args) - } - command, args := args[0], args[1:] - for _, c := range app.Commands() { - if c.Name() == command { - s := flag.NewFlagSet(app.Name(), flag.ExitOnError) - return runCommand(ctx, s, c, args) - } - } - return commandLineErrorf("Unknown command %v", command) + panic("unreachable: application.Run should never be called directly") } // Commands returns the set of commands supported by the gopls tool on the
diff --git a/gopls/internal/cmd/harness.go b/gopls/internal/cmd/harness.go index 3873bbf..66d7241 100644 --- a/gopls/internal/cmd/harness.go +++ b/gopls/internal/cmd/harness.go
@@ -8,14 +8,19 @@ "context" "flag" "fmt" + "io" "log" "os" "reflect" "runtime" "runtime/pprof" "runtime/trace" + "slices" "strings" "time" + + "golang.org/x/tools/gopls/internal/debug" + "golang.org/x/tools/gopls/internal/filecache" ) // This file defines common flags and helper functions @@ -72,56 +77,93 @@ return commandLineError(fmt.Sprintf(message, args...)) } -// Main should be invoked directly by main function. -// It will only return if there was no error. If an error -// was encountered it is printed to standard error and the -// application exits with an exit code of 2. +// Main is the main entry point for the gopls application, called by gopls main. +// It never returns. func Main() { ctx := context.Background() args := os.Args[1:] app := newApplication() - s := flag.NewFlagSet(app.Name(), flag.ExitOnError) - if err := runCommand(ctx, s, app, args); err != nil { - fmt.Fprintf(s.Output(), "%s: %v\n", app.Name(), err) - if _, printHelp := err.(commandLineError); printHelp { - // TODO(adonovan): refine this. It causes - // any command-line error to result in the full - // usage message, which typically obscures - // the actual error. - s.Usage() + cmd, globalArgs, cmdArgs, err := normalize(app, args) + if err != nil { + if cmd == nil { + cmd = app + } + fmt.Fprintf(os.Stderr, "%s: %v\n", cmdPath(cmd), err) + if isCommandLineError(err) { + printCommandHelp(os.Stderr, cmd) } os.Exit(2) } + + parseFlags(app, globalArgs) + cmdFlags := parseFlags(cmd, cmdArgs) + + err = runWithProfile(&app.ProfileFlags, func() error { + // In the category of "things we can do while waiting for the + // Go command": + + // TODO(hyangah): check if it's desirable to run filecache.Start unconditionally + // on every gopls subcommand, including CLI running with -remote or -help message. + // Pre-initialize the filecache, which takes ~50ms to hash the gopls + // executable, and immediately runs a gc. + filecache.Start() + + ctx = debug.WithInstance(ctx, app.OTel) + + return cmd.Run(ctx, cmdFlags.Args()...) + }) + if err != nil { + fmt.Fprintf(os.Stderr, "gopls: %v\n", err) + if isCommandLineError(err) { + printCommandHelp(os.Stderr, cmd) + } + os.Exit(2) + } + os.Exit(0) +} + +// isCommandLineError reports whether the error was created by [commandLineErrorf]. +func isCommandLineError(err error) bool { + _, ok := err.(commandLineError) + return ok +} + +// cmdPath returns the full command path (e.g. "gopls remote debug") for target. +func cmdPath(target command) string { + if sub, ok := target.(subcommand); ok && sub.Parent() != "" { + return sub.Parent() + " " + target.Name() + } + return target.Name() } // printHelp prints the usage and detailed help for any command to s.Output(). -func printHelp(s *flag.FlagSet, app command) { - if app.ShortHelp() != "" { - fmt.Fprintf(s.Output(), "%s\n\nUsage:\n ", app.ShortHelp()) - if sub, ok := app.(subcommand); ok && sub.Parent() != "" { - fmt.Fprintf(s.Output(), "%s [flags] %s", sub.Parent(), app.Name()) - } else { - fmt.Fprintf(s.Output(), "%s [flags]", app.Name()) - } - if usage := app.Usage(); usage != "" { - fmt.Fprintf(s.Output(), " %s", usage) - } - fmt.Fprintln(s.Output()) +func printHelp(s *flag.FlagSet, cmd command) { + if _, ok := cmd.(*application); !ok { + printCommandHelp(s.Output(), cmd) } - app.DetailedHelp(s) + cmd.DetailedHelp(s) } -// runCommand executes cmd with the provided flagset and arguments. -func runCommand(ctx context.Context, flags *flag.FlagSet, cmd command, args []string) (resultErr error) { - flags.Usage = func() { printHelp(flags, cmd) } - // addFlags returns non-nil *ProfileFlags only if cmd embeds ProfileFlags. - // Only 'application' meets this criteria. - p := addFlags(flags, reflect.StructField{}, reflect.ValueOf(cmd)) - if err := flags.Parse(args); err != nil { - return err +// printCommandHelp prints a concise usage summary for cmd to w. +func printCommandHelp(w io.Writer, cmd command) { + if _, ok := cmd.(*application); ok { + fmt.Fprintln(w, "Usage:\n gopls help [<subject>]") + return } + if short := cmd.ShortHelp(); short != "" { + fmt.Fprintf(w, "%s\n\n", short) + } + fmt.Fprintf(w, "Usage:\n gopls [flags] %s", strings.TrimPrefix(cmdPath(cmd), "gopls ")) + if usage := cmd.Usage(); usage != "" { + fmt.Fprintf(w, " %s", usage) + } + fmt.Fprintln(w) +} - if p != nil && p.CPU != "" { +// runWithProfile executes fn with active CPU, trace, memory, alloc, or block profiling +// if requested in p. +func runWithProfile(p *ProfileFlags, fn func() error) (resultErr error) { + if p.CPU != "" { f, err := os.Create(p.CPU) if err != nil { return err @@ -138,7 +180,7 @@ }() } - if p != nil && p.Trace != "" { + if p.Trace != "" { f, err := os.Create(p.Trace) if err != nil { return err @@ -156,7 +198,7 @@ }() } - if p != nil && p.Memory != "" { + if p.Memory != "" { f, err := os.Create(p.Memory) if err != nil { return err @@ -172,7 +214,7 @@ }() } - if p != nil && p.Alloc != "" { + if p.Alloc != "" { f, err := os.Create(p.Alloc) if err != nil { return err @@ -187,7 +229,7 @@ }() } - if p != nil && p.Block != "" { + if p.Block != "" { f, err := os.Create(p.Block) if err != nil { return err @@ -202,8 +244,7 @@ } }() } - - return cmd.Run(ctx, flags.Args()...) + return fn() } // addFlags scans fields of structs recursively to find things with flag tags @@ -289,3 +330,108 @@ } } } + +// parseFlags creates, configures, and parses a FlagSet for cmd using args. +// If parsing fails or help is requested, it prints contextual help and exits. +func parseFlags(cmd command, args []string) *flag.FlagSet { + // We use ContinueOnError and discard initial error output so we can intercept flag errors + // and produce contextual, user-friendly diagnostic messages rather than standard Go flag usage. + fs := flag.NewFlagSet(cmd.Name(), flag.ContinueOnError) + fs.SetOutput(io.Discard) + addCommandFlags(fs, cmd) + err := fs.Parse(args) + if err == nil { + return fs + } + + if err == flag.ErrHelp { + // POSIX convention requires writing explicit help requests + // (-h/-help) to stdout on exit 0. + fs.SetOutput(os.Stdout) + printHelp(fs, cmd) + os.Exit(0) + } + + fs.SetOutput(os.Stderr) + // When standard flag parsing fails due to an undefined flag, + // inspect command hierarchy so we can guide the user + // if they misplaced a flag before or after a subcommand. + if prefix := "flag provided but not defined: -"; strings.HasPrefix(err.Error(), prefix) { + checkMisplacedFlag(fs, cmd, strings.TrimPrefix(err.Error(), prefix)) + } + + // Fallback diagnostic for general flag syntax errors + // or truly unknown flags. + fmt.Fprintf(os.Stderr, "%s: %v\n", cmdPath(cmd), err) + printCommandHelp(os.Stderr, cmd) + os.Exit(2) + return nil +} + +// findCommandByName searches the command tree starting from root for a command named name. +func findCommandByName(root command, name string) command { + if root.Name() == name { + return root + } + for _, sub := range getSubcommands(root) { + if sub.Name() == name { + return sub + } + if found := findCommandByName(sub, name); found != nil { + return found + } + } + return nil +} + +// checkMisplacedFlag inspects ancestors and descendants to diagnose undefined flag errors. +// If a misplaced flag is found, it prints where the flag belongs and exits with code 2. +func checkMisplacedFlag(fs *flag.FlagSet, cmd command, name string) { + // Check descendants: e.g. placing a subcommand flag before specifying the subcommand. + if sub := findSubcommandWithFlag(cmd, name); sub != nil { + fmt.Fprintf(os.Stderr, "%s: flag -%s belongs to subcommand %s\n", cmdPath(cmd), name, sub.Name()) + printCommandHelp(fs.Output(), cmd) + os.Exit(2) + } + + // Walk up ancestors via lineage string: e.g. placing a global application flag after the subcommand name. + // Strict flag ordering requires parent/global flags to precede subcommands. + if sub, ok := cmd.(subcommand); ok && sub.Parent() != "" { + root := newApplication() + for _, currName := range slices.Backward(strings.Fields(sub.Parent())) { + curr := findCommandByName(root, currName) + if curr != nil && hasFlag(curr, name) { + fmt.Fprintf(os.Stderr, "%s: flag -%s must be placed before subcommand %s (after %s)\n", cmdPath(cmd), name, cmd.Name(), currName) + printCommandHelp(fs.Output(), cmd) + os.Exit(2) + } + } + } +} + +// findSubcommandWithFlag recursively searches getSubcommands(target) to check +// if flagName is registered on any child or descendant subcommand. +func findSubcommandWithFlag(target command, flagName string) command { + for _, sub := range getSubcommands(target) { + if hasFlag(sub, flagName) { + return sub + } + if found := findSubcommandWithFlag(sub, flagName); found != nil { + return found + } + } + return nil +} + +// hasFlag reports whether flagName is registered on cmd. +func hasFlag(cmd command, flagName string) bool { + fs := flag.NewFlagSet(cmd.Name(), flag.ContinueOnError) + fs.SetOutput(io.Discard) + addCommandFlags(fs, cmd) + return fs.Lookup(flagName) != nil +} + +// addCommandFlags registers the flags defined in the app struct onto the FlagSet. +func addCommandFlags(f *flag.FlagSet, app command) *ProfileFlags { + return addFlags(f, reflect.StructField{}, reflect.ValueOf(app)) +}
diff --git a/gopls/internal/cmd/help_test.go b/gopls/internal/cmd/help_test.go index 3518dce..946a28f 100644 --- a/gopls/internal/cmd/help_test.go +++ b/gopls/internal/cmd/help_test.go
@@ -41,7 +41,7 @@ } res := gopls(t, tree, args...) res.checkExit(true) // -h should result in exit 0 - got := res.stderr + got := res.stdout helpFile := filepath.Join("usage", name+".hlp") if *updateHelpFiles { if err := os.WriteFile(helpFile, []byte(got), 0666); err != nil { @@ -65,7 +65,7 @@ tree := writeTree(t, "") res := gopls(t, tree, "-v", "-h") res.checkExit(true) // -h should result in exit 0 - got := res.stderr + got := res.stdout helpFile := filepath.Join("usage", "usage-v.hlp") if *updateHelpFiles { if err := os.WriteFile(helpFile, []byte(got), 0666); err != nil { @@ -138,9 +138,16 @@ t.Run(strings.Join(test.args, " "), func(t *testing.T) { res := gopls(t, tree, test.args...) res.checkExit(test.wantSuccess) - res.checkStdout("^$") // no stdout - for _, pattern := range test.wantPatterns { - res.checkStderr(pattern) + if test.wantSuccess { + res.checkStderr("^$") // no stderr + for _, pattern := range test.wantPatterns { + res.checkStdout(pattern) + } + } else { + res.checkStdout("^$") // no stdout + for _, pattern := range test.wantPatterns { + res.checkStderr(pattern) + } } }) }
diff --git a/gopls/internal/cmd/info.go b/gopls/internal/cmd/info.go index 7dd6237..1f3a244 100644 --- a/gopls/internal/cmd/info.go +++ b/gopls/internal/cmd/info.go
@@ -62,10 +62,14 @@ } // 'gopls help cmd subcmd' is equivalent to 'gopls cmd subcmd -h'. - // The flag package prints the usage information (defined by Run) - // when it sees the -h flag. - fs := flag.NewFlagSet(cmd.Name(), flag.ExitOnError) - return runCommand(ctx, fs, h.app, append(args[:len(args):len(args)], "-h")) + // parseFlags prints the usage information when it sees the -h flag. + // + // TODO(hyangah): should we treat `gopls help cmd` and `gopls cmd -h` + // differently? For example, `gopls help cmd` can give a long help + // that explains a lot more details (DetailedHelp) than + // `gopls cmd -h` outputs (ShortHelp). + parseFlags(cmd, []string{"-h"}) + return nil } // version implements the version command.
diff --git a/gopls/internal/cmd/integration_test.go b/gopls/internal/cmd/integration_test.go index ff4b353..845b891 100644 --- a/gopls/internal/cmd/integration_test.go +++ b/gopls/internal/cmd/integration_test.go
@@ -1102,6 +1102,57 @@ } } +func TestCommandLineErrors(t *testing.T) { + testenv.NeedsGoBuild(t) + t.Parallel() + tree := writeTree(t, "") + for _, tc := range []struct { + name string + args []string + wantErrs []string + }{ + { + name: "MissingPositionalArgShowsUsage", + args: []string{"definition"}, + wantErrs: []string{"definition expects 1 argument", "Usage:\n gopls \\[flags\\] definition \\[definition-flags\\] <position>"}, + }, + { + name: "GlobalFlagAfterServe", + args: []string{"serve", "-otel=http://localhost:4318"}, + wantErrs: []string{`flag -otel must be placed before subcommand serve \(after gopls\)`}, + }, + { + name: "MisplacedContainerFlag", + args: []string{"remote", "-remote=localhost:12345", "sessions"}, + wantErrs: []string{`flag -remote must be placed before subcommand remote \(after gopls\)`}, + }, + + { + name: "UnknownSubcommandFlag", + args: []string{"-v", "execute", "-unknown"}, + wantErrs: []string{"flag provided but not defined: -unknown"}, + }, + { + name: "GlobalFlagAfterSubcommand", + args: []string{"references", "-v", "./gopls/main.go:35:8"}, + wantErrs: []string{`flag -v must be placed before subcommand references \(after gopls\)`}, + }, + { + name: "GlobalFlagAfterNestedSubcommand", + args: []string{"remote", "debug", "-v"}, + wantErrs: []string{`gopls remote debug: flag -v must be placed before subcommand debug \(after gopls\)`}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + res := gopls(t, tree, tc.args...) + res.checkExit(false) + for _, wantErr := range tc.wantErrs { + res.checkStderr(wantErr) + } + }) + } +} + // -- test framework -- func TestMain(m *testing.M) {
diff --git a/gopls/internal/cmd/normalize.go b/gopls/internal/cmd/normalize.go new file mode 100644 index 0000000..892c340 --- /dev/null +++ b/gopls/internal/cmd/normalize.go
@@ -0,0 +1,152 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cmd + +import ( + "flag" + "io" + "strings" +) + +// normalize scans command-line arguments to find the top-level subcommand, +// separating global application flags from subcommand arguments without executing FlagSet.Parse. +// +// Returns: +// - cmd: the resolved target subcommand (or &app.serve if none is specified). +// If flag validation errors, cmd is returned populated so callers can display contextual command help. +// - globalArgs: flags belonging to global application scope. +// - cmdArgs: arguments and flags belonging to cmd. +func normalize(app *application, args []string) (cmd command, globalArgs, cmdArgs []string, err error) { + silentFlagSet := func(name string, cmds ...command) *flag.FlagSet { + fs := flag.NewFlagSet(name, flag.ContinueOnError) + fs.SetOutput(io.Discard) + for _, c := range cmds { + addCommandFlags(fs, c) + } + return fs + } + + findSubcommand := func(curr command, name string) command { + for _, c := range getSubcommands(curr) { + if c.Name() == name { + return c + } + } + return nil + } + + // Flag sets to be used for flag name/type lookup. + appFlagSet := silentFlagSet(app.Name(), app) + serveFlagSet := silentFlagSet("serve", &app.serve) + + var preServeArgs []string + + i := 0 + for i < len(args) { + arg := args[i] + + if arg == "--" { + // e.g. gopls -v -- check file.go + // gopls vulncheck -- -mode=... + if i+1 < len(args) { + if matched := findSubcommand(app, args[i+1]); matched != nil { + cmd = matched + i++ // skip "--" + cmdArgs = append(cmdArgs, args[i+1:]...) + break + } + } + i++ + cmd = &app.serve + cmdArgs = append(cmdArgs, args[i:]...) + break + } + + // expect a valid subcommand if not a flag. + if arg == "-" || !strings.HasPrefix(arg, "-") { + if matched := findSubcommand(app, arg); matched != nil { + cmd = matched + i++ + cmdArgs = append(cmdArgs, args[i:]...) + break + } + break + } + + // below: arg is a string that has "-" as the prefix. + cleanArg := strings.TrimPrefix(strings.TrimPrefix(arg, "-"), "-") + name, _, hasValue := strings.Cut(cleanArg, "=") + + if name == "h" || name == "help" { // -h or -help + globalArgs = append(globalArgs, arg) + i++ + continue + } + + if appFlag := appFlagSet.Lookup(name); appFlag != nil { + consumed, err := consume(args, i, appFlag, name, hasValue) + if err != nil { + return nil, nil, nil, err + } + globalArgs = append(globalArgs, consumed...) + i += len(consumed) + continue + } + if serveFlag := serveFlagSet.Lookup(name); serveFlag != nil { + consumed, err := consume(args, i, serveFlag, name, hasValue) + if err != nil { + return nil, nil, nil, err + } + preServeArgs = append(preServeArgs, consumed...) + i += len(consumed) + continue + } + return nil, nil, nil, commandLineErrorf("unknown flag: %s", arg) + } + + if cmd == nil { + if i < len(args) { + return nil, nil, nil, commandLineErrorf("unknown command %q", args[i]) + } + cmd = &app.serve + cmdArgs = preServeArgs + } else if cmd.Name() == "serve" { + // For backwards compatibility, allow flags to be placed after serve. + cmdArgs = append(preServeArgs, cmdArgs...) + } else if len(preServeArgs) > 0 { + // All explicitly specified subcommands other than serve must + // follow strict flag ordering. + arg := preServeArgs[0] + cleanArg := strings.TrimPrefix(strings.TrimPrefix(arg, "-"), "-") + name, _, _ := strings.Cut(cleanArg, "=") + if hasFlag(cmd, name) { + return cmd, nil, nil, commandLineErrorf("flag -%s must be placed after the command %s", name, cmd.Name()) + } + if sub := findSubcommandWithFlag(app, name); sub != nil { + return cmd, nil, nil, commandLineErrorf("flag -%s belongs to subcommand %s", name, sub.Name()) + } + + return cmd, nil, nil, commandLineErrorf("flag provided but not defined: -%s", name) + } + + return cmd, globalArgs, cmdArgs, nil +} + +// consume returns the token(s) corresponding to flag f from args starting at index i. +func consume(args []string, i int, f *flag.Flag, name string, hasValue bool) ([]string, error) { + if hasValue || isBoolFlag(f) { + return args[i : i+1], nil + } + if i+1 >= len(args) { + return nil, commandLineErrorf("flag needs an argument: -%s", name) + } + return args[i : i+2], nil +} + +// isBoolFlag reports whether f is a boolean flag. +func isBoolFlag(f *flag.Flag) bool { + bf, ok := f.Value.(interface{ IsBoolFlag() bool }) + return ok && bf.IsBoolFlag() +}
diff --git a/gopls/internal/cmd/normalize_test.go b/gopls/internal/cmd/normalize_test.go new file mode 100644 index 0000000..0d45df5 --- /dev/null +++ b/gopls/internal/cmd/normalize_test.go
@@ -0,0 +1,341 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cmd + +import ( + "slices" + "strings" + "testing" +) + +// TODO(adonovan): turn this into an integration test. Tests of +// internal helper functions are not robust nor do they reflect actual +// application behavior when state (e.g. flags) is involved. +// +// Also, check that the correct usage message is generated. +func TestNormalize(t *testing.T) { + tests := []struct { + name string + args []string + wantCmd string + wantGlobalArgs []string + wantArgs []string + wantErr string + }{ + // ========================================================== + // Implicit & Explicit Serve + // ========================================================== + { + name: "VSCodeGo_Default", + args: []string{}, + wantCmd: "serve", + }, + { + name: "GlobalFlagDefaultsToServe", + args: []string{"-v"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-v"}, + }, + { + name: "VSCodeGo_RPCTrace", + args: []string{"-rpc.trace"}, + wantCmd: "serve", + wantArgs: []string{"-rpc.trace"}, + }, + { + name: "VSCodeGo_RPCTraceServe", + args: []string{"-rpc.trace", "serve"}, + wantCmd: "serve", + wantArgs: []string{"-rpc.trace"}, + }, + { + name: "XTools_ServeDebug", + args: []string{"serve", "-debug=localhost:6060"}, + wantCmd: "serve", + wantArgs: []string{"-debug=localhost:6060"}, + }, + { + name: "GlobalFlagBeforeExplicitServe", + args: []string{"-otel=http://localhost:4318", "serve"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-otel=http://localhost:4318"}, + }, + + // ========================================================== + // Non-Serve Subcommands + // ========================================================== + { + name: "OtherCases_Version", + args: []string{"version"}, + wantCmd: "version", + }, + { + name: "XTools_Help", + args: []string{"help"}, + wantCmd: "help", + }, + { + name: "XTools_References", + args: []string{"references", "./gopls/main.go:35:8"}, + wantCmd: "references", + wantArgs: []string{"./gopls/main.go:35:8"}, + }, + { + name: "ReferencesWithRemoteDebugStrict", + args: []string{"references", "-remote.debug=:0", "./gopls/main.go:35:8"}, + wantCmd: "references", + wantArgs: []string{"-remote.debug=:0", "./gopls/main.go:35:8"}, + }, + { + name: "ModernRemoteSessionsStrict_Valid", + args: []string{"remote", "sessions", "-remote=localhost:12345"}, + wantCmd: "remote", + wantArgs: []string{"sessions", "-remote=localhost:12345"}, + }, + { + name: "ModernRemoteDebugStrict_Valid", + args: []string{"remote", "debug", "-remote=localhost:8082", "localhost:8083"}, + wantCmd: "remote", + wantArgs: []string{"debug", "-remote=localhost:8082", "localhost:8083"}, + }, + { + name: "VSCodeGo_Vulncheck", + args: []string{"vulncheck", "--", "-mode=convert", "-show=color"}, + wantCmd: "vulncheck", + wantArgs: []string{"--", "-mode=convert", "-show=color"}, + }, + { + name: "OtherCases_VerboseExecuteServe", + args: []string{"-v", "execute", "serve"}, + wantCmd: "execute", + wantGlobalArgs: []string{"-v"}, + wantArgs: []string{"serve"}, + }, + { + name: "OtherCases_UnknownAppFlag", + args: []string{"-nope", "execute"}, + wantErr: "unknown flag: -nope", + }, + + // ========================================================== + // Edge cases + // ========================================================== + { + name: "AppFlagWithDoubleDash", + args: []string{"--verbose", "check", "foo.go"}, + wantCmd: "check", + wantGlobalArgs: []string{"--verbose"}, + wantArgs: []string{"foo.go"}, + }, + { + name: "AppFlagWithTripleDash", + args: []string{"---foo"}, + wantErr: "unknown flag: ---foo", + }, + { + name: "ServeFlagWithSpaceArg", + args: []string{"-listen", "localhost:3000"}, + wantCmd: "serve", + wantArgs: []string{"-listen", "localhost:3000"}, + }, + { + name: "LogfileWithSpaceServe", + args: []string{"-logfile", "serve"}, + wantCmd: "serve", + wantArgs: []string{"-logfile", "serve"}, + }, + { + name: "GlobalAndServeFlagsHoist", + args: []string{"-listen=localhost:3000", "-v"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-v"}, + wantArgs: []string{"-listen=localhost:3000"}, + }, + { + name: "GlobalAndServeFlagsHoistMixed", + args: []string{"-listen", "localhost:3000", "-v"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-v"}, + wantArgs: []string{"-listen", "localhost:3000"}, + }, + { + name: "DoubleDashPositional", + args: []string{"--", "foo"}, + wantCmd: "serve", + wantArgs: []string{"foo"}, + }, + { + name: "DoubleDashFlag", + args: []string{"--", "-v"}, + wantCmd: "serve", + wantArgs: []string{"-v"}, + }, + { + name: "UnknownFlagInServe", + args: []string{"-unknown"}, + wantErr: "unknown flag: -unknown", + }, + { + name: "GlobalFlagWithSpaceArgBeforeCheck", + args: []string{"-otel", "http://localhost", "check", "file.go"}, + wantCmd: "check", + wantGlobalArgs: []string{"-otel", "http://localhost"}, + wantArgs: []string{"file.go"}, + }, + { + name: "ImplicitServePositional", + args: []string{"-listen=localhost:3000", "foo"}, + wantErr: `unknown command "foo"`, // consistent with gopls@v0.20.0 + }, + { + name: "RemoteFlagBeforeServeCompat", + args: []string{"-remote=auto", "serve"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-remote=auto"}, + }, + { + name: "RemoteFlagDefaultsToServe", + args: []string{"-remote=auto"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-remote=auto"}, + }, + { + name: "RemoteFlagsDefaultToServe", + args: []string{"-remote=auto", "-remote.debug=localhost:8080"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-remote=auto", "-remote.debug=localhost:8080"}, + }, + + { + name: "RepeatedSubcommandFlags", + args: []string{"-listen", "localhost:3000", "-listen", "localhost:4000"}, + wantCmd: "serve", + wantArgs: []string{"-listen", "localhost:3000", "-listen", "localhost:4000"}, + }, + { + name: "BooleanFlagInlineTrue", + args: []string{"-v=true"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-v=true"}, + }, + { + name: "GlobalFlagTrailingEqual", + args: []string{"-v="}, + wantCmd: "serve", + wantGlobalArgs: []string{"-v="}, + }, + { + name: "ServeFlagTrailingEqual", + args: []string{"-listen="}, + wantCmd: "serve", + wantArgs: []string{"-listen="}, + }, + { + name: "EmptyArgumentImplicitServe", + args: []string{""}, + wantErr: `unknown command ""`, + }, + { + name: "EmptyArgumentAsGlobalFlagValue", + args: []string{"-otel", "", "check"}, + wantCmd: "check", + wantGlobalArgs: []string{"-otel", ""}, + }, + { + name: "RepeatedExplicitServeSubcommand", + args: []string{"serve", "serve"}, + wantCmd: "serve", + wantArgs: []string{"serve"}, + }, + { + name: "GlobalHelpFlagDefaultsToServe", + args: []string{"-help"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-help"}, + }, + + // ========================================================== + // Regression Tests: Reflection-based flag separation & misplaced flags + // ========================================================== + { + name: "SeparateProfileFlagFromServeFlag", + args: []string{"-profile.cpu=cpu.prof", "-listen=localhost:8080"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-profile.cpu=cpu.prof"}, + wantArgs: []string{"-listen=localhost:8080"}, + }, + { + name: "SeparateVeryVerboseAndProfileFromServeFlags", + args: []string{"-vv", "-listen", "localhost:8080", "-profile.mem=mem.prof"}, + wantCmd: "serve", + wantGlobalArgs: []string{"-vv", "-profile.mem=mem.prof"}, + wantArgs: []string{"-listen", "localhost:8080"}, + }, + { + name: "MisplacedServeListenFlagBeforeCheck", + args: []string{"-listen=localhost:8080", "check", "file.go"}, + wantCmd: "check", + wantErr: "flag -listen belongs to subcommand serve", + }, + { + name: "MisplacedServeLogfileFlagBeforeVersion", + args: []string{"-logfile=gopls.log", "version"}, + wantCmd: "version", + wantErr: "flag -logfile belongs to subcommand serve", + }, + + // ========================================================== + // Error Scenarios + // ========================================================== + { + name: "MissingGlobalFlagValue", + args: []string{"-otel"}, + wantErr: "flag needs an argument", + }, + { + name: "MissingServeFlagValue", + args: []string{"-listen"}, + wantErr: "flag needs an argument", + }, + { + name: "SubcommandFlagBeforeSubcommandFail", + args: []string{"-listen=:0", "references", "./gopls/main.go:35:8"}, + wantCmd: "references", + wantErr: "flag -listen belongs to subcommand serve", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + app := newApplication() // Ensure test isolation + t.Logf("> gopls %v", strings.Join(tc.args, " ")) + subApp, gotGlobalArgs, gotArgs, err := normalize(app, tc.args) + if tc.wantErr != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("err = %v, want error containing %q", err, tc.wantErr) + } + if tc.wantCmd != "" && subApp != nil && subApp.Name() != tc.wantCmd { + t.Errorf("normalize() cmd = %v, want %v on error", subApp.Name(), tc.wantCmd) + } + return + } + if err != nil { + t.Fatalf("dispatch failed: %v", err) + } + if subApp.Name() != tc.wantCmd { + t.Errorf("normalize() cmd = %v, want %v", subApp.Name(), tc.wantCmd) + } + if !slices.Equal(gotGlobalArgs, tc.wantGlobalArgs) { + t.Errorf("normalize() globalArgs = %v, want %v", gotGlobalArgs, tc.wantGlobalArgs) + } + if !slices.Equal(gotArgs, tc.wantArgs) { + t.Errorf("normalize() args = %v, want %v", gotArgs, tc.wantArgs) + } + }) + } +}
diff --git a/gopls/internal/cmd/serve.go b/gopls/internal/cmd/serve.go index 7df303b..2ffe269 100644 --- a/gopls/internal/cmd/serve.go +++ b/gopls/internal/cmd/serve.go
@@ -25,8 +25,7 @@ "golang.org/x/tools/internal/jsonrpc2" ) -// serve is a struct that exposes the configurable parts of the LSP and MCP -// server as flags, in the right form for tool.Main to consume. +// serve defines the flags and working state of the gopls serve command. type serve struct { Logfile string `flag:"logfile" help:"filename to log to. if value is \"auto\", then logging to a default output file is enabled"` Mode string `flag:"mode" help:"no effect"`
diff --git a/gopls/internal/cmd/subcommands.go b/gopls/internal/cmd/subcommands.go index ee5eaad..cc934bf 100644 --- a/gopls/internal/cmd/subcommands.go +++ b/gopls/internal/cmd/subcommands.go
@@ -34,8 +34,8 @@ command, args := args[0], args[1:] for _, c := range s { if c.Name() == command { - s := flag.NewFlagSet(c.Name(), flag.ExitOnError) - return runCommand(ctx, s, c, args) + fs := parseFlags(c, args) + return c.Run(ctx, fs.Args()...) } } return commandLineErrorf("unknown subcommand %v", command)