From 7158dc2f3c2bc211ca0e3fcda77c2cbd7dce6751 Mon Sep 17 00:00:00 2001 From: MaxHuang22 Date: Wed, 8 Apr 2026 15:11:36 +0800 Subject: [PATCH] fix: reject positional arguments in shortcuts (#227) * fix: reject positional arguments in shortcuts with clear error Shortcuts silently ignored positional arguments (e.g. `lark-cli docs +search "hello"`), causing empty results. Add Args validator to all declarative shortcuts so cobra prints usage and a clear error message telling users to pass values via flags instead. Change-Id: I7579f9c871138cf91dd5f5d8c1d51bda3f77a1db * fix: address PR review comments - Remove unused *Shortcut parameter from rejectPositionalArgs - Show all positional args in error message instead of only the first - Add test case for multiple positional arguments Change-Id: Ifea92d09ddabcd35fbf2db98d9888d18af59b894 --- shortcuts/common/runner.go | 14 +++++++ shortcuts/common/runner_args_test.go | 58 ++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 shortcuts/common/runner_args_test.go diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index 6eef2b45..4ee0147e 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -470,6 +470,7 @@ func (s Shortcut) mountDeclarative(parent *cobra.Command, f *cmdutil.Factory) { cmd := &cobra.Command{ Use: shortcut.Command, Short: shortcut.Description, + Args: rejectPositionalArgs(), RunE: func(cmd *cobra.Command, _ []string) error { return runShortcut(cmd, f, &shortcut, botOnly) }, @@ -685,6 +686,19 @@ func handleShortcutDryRun(f *cmdutil.Factory, rctx *RuntimeContext, s *Shortcut) return nil } +// rejectPositionalArgs returns a cobra.PositionalArgs that rejects any +// positional arguments. The error is intentionally a plain error (not +// ExitError) so that cobra prints usage and the root handler prints a +// simple "Error:" line instead of a JSON envelope. +func rejectPositionalArgs() cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return nil + } + return fmt.Errorf("positional arguments are not supported (got %q); pass values via flags", args) + } +} + func registerShortcutFlags(cmd *cobra.Command, s *Shortcut) { for _, fl := range s.Flags { desc := fl.Desc diff --git a/shortcuts/common/runner_args_test.go b/shortcuts/common/runner_args_test.go new file mode 100644 index 00000000..f04999da --- /dev/null +++ b/shortcuts/common/runner_args_test.go @@ -0,0 +1,58 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package common + +import ( + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func TestRejectPositionalArgs_WithArgs(t *testing.T) { + t.Parallel() + + validator := rejectPositionalArgs() + + err := validator(&cobra.Command{}, []string{"hello"}) + if err == nil { + t.Fatal("expected error for positional arg, got nil") + } + if !strings.Contains(err.Error(), "positional arguments are not supported") { + t.Errorf("expected positional args rejection message, got: %v", err) + } + if !strings.Contains(err.Error(), `"hello"`) { + t.Errorf("expected the positional arg value in error, got: %v", err) + } +} + +func TestRejectPositionalArgs_MultipleArgs(t *testing.T) { + t.Parallel() + + validator := rejectPositionalArgs() + + err := validator(&cobra.Command{}, []string{"hello", "world"}) + if err == nil { + t.Fatal("expected error for multiple positional args, got nil") + } + if !strings.Contains(err.Error(), "positional arguments are not supported") { + t.Errorf("unexpected error message: %v", err) + } + if !strings.Contains(err.Error(), "hello") || !strings.Contains(err.Error(), "world") { + t.Errorf("expected all positional args in error, got: %v", err) + } +} + +func TestRejectPositionalArgs_NoArgs(t *testing.T) { + t.Parallel() + + validator := rejectPositionalArgs() + + if err := validator(&cobra.Command{}, nil); err != nil { + t.Fatalf("expected no error for nil args, got: %v", err) + } + if err := validator(&cobra.Command{}, []string{}); err != nil { + t.Fatalf("expected no error for empty args, got: %v", err) + } +}