diff --git a/internal/app/boolean_flag_contract_test.go b/internal/app/boolean_flag_contract_test.go new file mode 100644 index 00000000..203011f4 --- /dev/null +++ b/internal/app/boolean_flag_contract_test.go @@ -0,0 +1,228 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + stderrors "errors" + "reflect" + "sort" + "strings" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pipeline" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pipeline/handlers" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +func TestAllDistributionBooleanFlagTypesNormalizeDetachedLiterals(t *testing.T) { + root := NewSchemaSourceRootCommand() + unique := make(map[string]pipeline.FlagInfo) + var visit func(*cobra.Command) + visit = func(command *cobra.Command) { + for _, spec := range pipeline.FlagInfoFromCommand(command) { + if spec.Type != "bool" && spec.Type != "boolean" { + continue + } + key := strings.Join([]string{spec.Name, spec.Shorthand, spec.Type}, "\x00") + unique[key] = spec + } + for _, child := range command.Commands() { + visit(child) + } + } + visit(root) + + keys := make([]string, 0, len(unique)) + for key := range unique { + keys = append(keys, key) + } + sort.Strings(keys) + if len(keys) < 80 { + t.Fatalf("boolean flag contract coverage is unexpectedly small: %d", len(keys)) + } + + for _, key := range keys { + spec := unique[key] + for _, value := range []string{"true", "false"} { + t.Run(spec.Name+"/"+value, func(t *testing.T) { + ctx := &pipeline.Context{ + Command: "dws contract probe", + Args: []string{"--" + spec.Name, value}, + FlagSpecs: []pipeline.FlagInfo{spec}, + } + if err := (handlers.BoolValueHandler{}).Handle(ctx); err != nil { + t.Fatalf("BoolValueHandler.Handle() error = %v", err) + } + want := []string{"--" + spec.Name + "=" + value} + if !reflect.DeepEqual(ctx.Args, want) { + t.Fatalf("normalized args = %v, want %v", ctx.Args, want) + } + + flags := pflag.NewFlagSet(spec.Name, pflag.ContinueOnError) + flags.Bool(spec.Name, false, "") + if err := flags.Parse(ctx.Args); err != nil { + t.Fatalf("pflag rejected normalized args %v: %v", ctx.Args, err) + } + got, err := flags.GetBool(spec.Name) + if err != nil || got != (value == "true") || !flags.Changed(spec.Name) { + t.Fatalf("parsed %s = %v, changed=%v, error=%v", spec.Name, got, flags.Changed(spec.Name), err) + } + }) + } + } + t.Logf("verified detached boolean syntax for %d distinct distribution flag contracts", len(keys)) +} + +func TestBooleanSyntaxPreservesDefaultsRequiredAndChangedContracts(t *testing.T) { + tests := []struct { + name string + path string + flag string + value string + wantDefault string + wantValue string + }{ + {name: "root default false", path: "chat bot find", flag: "dry-run", value: "false", wantDefault: "false", wantValue: "false"}, + {name: "root mock default false", path: "chat bot find", flag: "mock", value: "true", wantDefault: "false", wantValue: "true"}, + {name: "local force default false", path: "upgrade", flag: "force", value: "false", wantDefault: "false", wantValue: "false"}, + {name: "local default true", path: "sheet find", flag: "match-case", value: "false", wantDefault: "true", wantValue: "false"}, + {name: "required explicit false", path: "contact dept create", flag: "create-dept-group", value: "false", wantDefault: "false", wantValue: "false"}, + {name: "changed false remains explicit", path: "sheet csv-put", flag: "allow-overwrite", value: "false", wantDefault: "false", wantValue: "false"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + root := NewSchemaSourceRootCommand() + leaf := resolveParamLeaf(root, test.path) + if leaf == nil { + t.Fatalf("command %q is not runnable", test.path) + } + flag := booleanContractFlag(leaf, test.flag) + if flag == nil || flag.DefValue != test.wantDefault || flag.Changed { + t.Fatalf("initial --%s contract = %#v, want default %q and unchanged", test.flag, flag, test.wantDefault) + } + + pathArgs := strings.Fields(test.path) + rawArgs := append(append([]string(nil), pathArgs...), "--"+test.flag, test.value) + ctx, err := pipeline.RunPreParseArgs(root, newPipelineEngine(), rawArgs) + if err != nil { + t.Fatalf("RunPreParseArgs(%v) error = %v", rawArgs, err) + } + if ctx == nil { + t.Fatal("RunPreParseArgs returned nil context") + } + flagArgs := ctx.Args[len(pathArgs):] + if err := leaf.ParseFlags(flagArgs); err != nil { + t.Fatalf("ParseFlags(%v) error = %v", flagArgs, err) + } + flag = booleanContractFlag(leaf, test.flag) + if flag == nil || flag.Value.String() != test.wantValue || !flag.Changed { + t.Fatalf("final --%s contract = %#v, want value %q and changed", test.flag, flag, test.wantValue) + } + }) + } +} + +func TestDetachedDryRunValuesReachTheExpectedFinalDispatchBoundary(t *testing.T) { + base := []string{ + "mail", "folder", "update", + "--email", "fixture@example.com", "--id", "folder-1", "--name", "Fixture Folder", + } + + bareArgs := append(append([]string(nil), base...), "--dry-run") + _, barePreview, bareAttempts, bareErr := executeParamAliasDryRunE2E(t, bareArgs...) + if bareErr != nil || !barePreview.DryRun || barePreview.Executed || len(bareAttempts) != 0 { + t.Fatalf("bare dry-run = preview:%#v attempts:%#v error:%v", barePreview, bareAttempts, bareErr) + } + + trueArgs := append(append([]string(nil), base...), "--dry-run", "TRUE") + trueCtx, truePreview, trueAttempts, trueErr := executeParamAliasDryRunE2E(t, trueArgs...) + if trueErr != nil || !reflect.DeepEqual(truePreview, barePreview) || len(trueAttempts) != 0 { + t.Fatalf("detached true = context:%#v preview:%#v attempts:%#v error:%v", trueCtx, truePreview, trueAttempts, trueErr) + } + if !hasBooleanCorrection(trueCtx, "--dry-run TRUE", "--dry-run=true") { + t.Fatalf("detached true correction = %#v", trueCtx) + } + + falseCases := []struct { + name string + args []string + }{ + {name: "detached", args: append(append([]string(nil), base...), "--dry-run", "false")}, + {name: "explicit", args: append(append([]string(nil), base...), "--dry-run=false")}, + } + var wantAttempts []any + for _, test := range falseCases { + t.Run(test.name, func(t *testing.T) { + ctx, _, attempts, err := executeParamAliasDryRunE2E(t, test.args...) + if err == nil || !strings.Contains(err.Error(), "dry-run reached the injected command runner") { + t.Fatalf("dry-run=false dispatch error = %v", err) + } + if len(attempts) != 1 || attempts[0].DryRun { + t.Fatalf("dry-run=false attempts = %#v", attempts) + } + if test.name == "detached" && !hasBooleanCorrection(ctx, "--dry-run false", "--dry-run=false") { + t.Fatalf("detached false correction = %#v", ctx) + } + serialized := []any{attempts[0].CanonicalProduct, attempts[0].Tool, attempts[0].Params, attempts[0].DryRun} + if wantAttempts == nil { + wantAttempts = serialized + } else if !reflect.DeepEqual(serialized, wantAttempts) { + t.Fatalf("detached and explicit false dispatch differ\nwant=%#v\ngot=%#v", wantAttempts, serialized) + } + }) + } +} + +func TestContradictoryBooleanValuesFailBeforeDestructiveDispatch(t *testing.T) { + caller := ¶mAliasCaptureCaller{} + ctx, err := executeParamAliasE2E(t, caller, + "mail", "thread", "trash", + "--email", "user@example.com", "--id", "conversation-1", + "--yes", "true", "--yes=false", + ) + var conflict *pipeline.BoolValueConflictError + if !stderrors.As(err, &conflict) { + t.Fatalf("conflicting confirmation error = %v, want BoolValueConflictError (ctx=%#v)", err, ctx) + } + if conflict.Flag != "yes" || !reflect.DeepEqual(conflict.Values, []string{"false", "true"}) { + t.Fatalf("conflict = %#v", conflict) + } + if len(caller.calls) != 0 { + t.Fatalf("conflicting confirmation reached destructive dispatch: %#v", caller.calls) + } +} + +func booleanContractFlag(command *cobra.Command, name string) *pflag.Flag { + if command == nil { + return nil + } + if flag := command.Flags().Lookup(name); flag != nil { + return flag + } + return command.InheritedFlags().Lookup(name) +} + +func hasBooleanCorrection(ctx *pipeline.Context, original, corrected string) bool { + if ctx == nil { + return false + } + for _, correction := range ctx.Corrections { + if correction.Handler == "boolvalue" && correction.Original == original && correction.Corrected == corrected { + return true + } + } + return false +} diff --git a/internal/app/flag_error_test.go b/internal/app/flag_error_test.go index af9e11f1..c3580e24 100644 --- a/internal/app/flag_error_test.go +++ b/internal/app/flag_error_test.go @@ -145,3 +145,23 @@ func TestReviewedFlagProtectionAndInstallerEdges(t *testing.T) { t.Fatalf("unreviewed flag protection = %q, %q, %v", flag, protection, ok) } } + +func TestReviewedFlagProtectionInstallerPreservesLocalHandler(t *testing.T) { + root := NewRootCommand() + cmd := mustFindCommand(t, root, "contact", "dept", "list-children") + handler := cmd.FlagErrorFunc() + + unreviewed := handler(cmd, fmt.Errorf("unknown flag: --not-reviewed")) + var structured *apperrors.Error + if stderrors.As(unreviewed, &structured) { + t.Fatalf("unreviewed error bypassed the command's local handler: %#v", structured) + } + if !strings.HasSuffix(unreviewed.Error(), "See 'dws contact dept list-children --help' for usage.") { + t.Fatalf("local handler output = %q", unreviewed) + } + + guarded := handler(cmd, fmt.Errorf("unknown flag: --name")) + if !stderrors.As(guarded, &structured) || structured.Reason != "blocked_flag" { + t.Fatalf("reviewed guard did not use the central handler: %#v", guarded) + } +} diff --git a/internal/app/preparse_presentation_test.go b/internal/app/preparse_presentation_test.go index 1879cb16..361d8d11 100644 --- a/internal/app/preparse_presentation_test.go +++ b/internal/app/preparse_presentation_test.go @@ -15,6 +15,7 @@ package app import ( "bytes" + stderrors "errors" "io" "strings" "testing" @@ -72,12 +73,19 @@ func TestPreParseConflictHonorsErrorPresentationFlags(t *testing.T) { t.Fatalf("PreParse error verbosity = %v, want debug", got) } - err = apperrors.NewValidation( - err.Error(), - apperrors.WithReason("parameter_conflict"), - apperrors.WithHint("Remove the duplicate alias/canonical spelling and pass the parameter exactly once."), - apperrors.WithCause(err), - ) + err = newPreParseValidationError(err) + var structured *apperrors.Error + if !stderrors.As(err, &structured) { + t.Fatalf("PreParse validation error = %T, want *errors.Error", err) + } + if strings.Contains(structured.Message, "pipeline") || strings.Contains(structured.Message, "semantic-alias") || + strings.Contains(structured.Cause.Error(), "pipeline") || strings.Contains(structured.Cause.Error(), "semantic-alias") { + t.Fatalf("internal pipeline identity leaked to user error: message=%q cause=%q", structured.Message, structured.Cause) + } + var conflict *pipeline.FlagConflictError + if !stderrors.As(err, &conflict) { + t.Fatalf("PreParse validation error lost FlagConflictError: %v", err) + } var output bytes.Buffer if printErr := printExecutionError(root, &output, &output, err); printErr != nil { t.Fatalf("printExecutionError() error = %v", printErr) diff --git a/internal/app/root.go b/internal/app/root.go index 7a022bda..88b799c8 100644 --- a/internal/app/root.go +++ b/internal/app/root.go @@ -115,12 +115,7 @@ func Execute() (exitCode int) { // This corrects model-generated errors like --userId → --user-id // and --limit100 → --limit 100. if err := rootRunPreParse(root, engine); err != nil { - err = apperrors.NewValidation( - err.Error(), - apperrors.WithReason("parameter_conflict"), - apperrors.WithHint("Remove the duplicate alias/canonical spelling and pass the parameter exactly once."), - apperrors.WithCause(err), - ) + err = newPreParseValidationError(err) _ = printExecutionError(root, os.Stdout, os.Stderr, err) return apperrors.ExitCode(err) } @@ -145,6 +140,22 @@ func Execute() (exitCode int) { return 0 } +// newPreParseValidationError keeps pipeline handler identity in internal logs +// while exposing only the underlying parameter-domain error to CLI users. +func newPreParseValidationError(err error) error { + userErr := err + var handlerErr *pipeline.HandlerError + if stderrors.As(err, &handlerErr) && handlerErr.Unwrap() != nil { + userErr = handlerErr.Unwrap() + } + return apperrors.NewValidation( + userErr.Error(), + apperrors.WithReason("parameter_conflict"), + apperrors.WithHint("Remove the duplicate alias/canonical spelling and pass the parameter exactly once."), + apperrors.WithCause(userErr), + ) +} + func isUnknownCommandError(err error) bool { return err != nil && strings.Contains(err.Error(), "unknown command") } @@ -519,7 +530,13 @@ func installReviewedFlagProtectionHandlers(root *cobra.Command) { var visit func(*cobra.Command) visit = func(cmd *cobra.Command) { if entry, ok := cli.LookupParamAlias(cmd.CommandPath()); ok && (len(entry.Blocked) > 0 || len(entry.Ambiguous) > 0) { - cmd.SetFlagErrorFunc(flagErrorWithSuggestions) + previous := cmd.FlagErrorFunc() + cmd.SetFlagErrorFunc(func(current *cobra.Command, err error) error { + if _, _, guarded := reviewedFlagProtection(current, err.Error()); guarded { + return flagErrorWithSuggestions(current, err) + } + return previous(current, err) + }) } for _, child := range cmd.Commands() { visit(child) @@ -1328,8 +1345,9 @@ func newPipelineEngine() *pipeline.Engine { // resolves reviewed synonyms to the real flag (--keyword → --query), // then sticky splits glued values (--limit100 → --limit 100), then // paramname fixes near-miss typos (--limt → --limit). Boolvalue runs - // last so every spelling of `--yes false` becomes `--yes=false` before - // pflag can interpret the bare bool as true. + // last so detached values for every real boolean flag (for example + // `--dry-run false`) become explicit `--flag=false` tokens before pflag + // can interpret the bare flag as true. handlers.AliasHandler{}, handlers.SemanticAliasHandler{ // Inject the build-time reduced alias table with native types so diff --git a/internal/pipeline/cobra.go b/internal/pipeline/cobra.go index a830cdf2..de6a788f 100644 --- a/internal/pipeline/cobra.go +++ b/internal/pipeline/cobra.go @@ -14,6 +14,9 @@ package pipeline import ( + "errors" + "fmt" + "io" "log/slog" "os" "strings" @@ -82,7 +85,9 @@ func RunPreParseArgs(root *cobra.Command, engine *Engine, rawArgs []string) (*Co // --format/--debug/--verbose affect this early error exactly as they do // errors returned after Cobra parsing. No credentials, profiles, output // paths, or execution controls are applied here. - primeEarlyErrorPresentation(root, target, ctx.Args) + if presentationErr := primeEarlyErrorPresentation(root, target, ctx.Args); presentationErr != nil { + slog.Debug("pipeline pre-parse presentation flags", "error", presentationErr) + } slog.Debug("pipeline pre-parse", "error", err) return ctx, err } @@ -138,10 +143,6 @@ func argsForCommandTraversal(root *cobra.Command, rawArgs []string) []string { return out } -func persistentFlagToken(flags *pflag.FlagSet, argument string) (*pflag.Flag, bool, bool) { - return newFlagTokenMatcher(flags).matchTraversalToken(argument) -} - // separatedBoolValue recognises model-friendly `--flag false` and exact // shorthand `-f false` spellings without mistaking an already attached value // or a shorthand cluster for a detached value. pflag otherwise treats a bare @@ -259,83 +260,55 @@ func (m *flagTokenMatcher) matchTraversalToken(argument string) (*pflag.Flag, bo return m.byShorthand[string(shorthands[0])], true, true } -func primeEarlyErrorPresentation(root, target *cobra.Command, rawArgs []string) { +func primeEarlyErrorPresentation(root, target *cobra.Command, rawArgs []string) error { if root == nil || target == nil || len(rawArgs) == 0 { - return + return nil } rootFlags := root.PersistentFlags() - presentation := map[string]bool{"format": true, "debug": true, "verbose": true} - matcher := newFlagTokenMatcher(target.Flags(), target.InheritedFlags()) - for index := 0; index < len(rawArgs); index++ { - argument := rawArgs[index] - if argument == "--" { - break - } + presentationFlags := pflag.NewFlagSet("early-error-presentation", pflag.ContinueOnError) + presentationFlags.SetOutput(io.Discard) + presentationFlags.ParseErrorsWhitelist.UnknownFlags = true + presentationFlags.SetNormalizeFunc(func(_ *pflag.FlagSet, name string) pflag.NormalizedName { + return pflag.NormalizedName(cmdutil.Morph(name)) + }) - if strings.HasPrefix(argument, "--") { - match := matcher.matchLongToken(argument) - if !match.recognized || match.flag == nil { - continue - } - value, hasValue := match.value, match.hasValue - if !hasValue && match.flag.NoOptDefVal != "" { - if index+1 < len(rawArgs) { - if normalized, ok := separatedBoolValue(argument, rawArgs[index+1], match.flag, false); ok { - index++ - value, hasValue = normalized, true - } else { - value, hasValue = match.flag.NoOptDefVal, true - } - } else { - value, hasValue = match.flag.NoOptDefVal, true - } - } else if !hasValue && index+1 < len(rawArgs) { - index++ - value, hasValue = rawArgs[index], true - } - if presentation[match.flag.Name] && hasValue { - _ = rootFlags.Set(match.flag.Name, value) - } + names := make([]string, 0, 3) + if source := rootFlags.Lookup("format"); source != nil { + value, err := rootFlags.GetString("format") + if err != nil { + return fmt.Errorf("read presentation flag --format: %w", err) + } + presentationFlags.StringP("format", source.Shorthand, value, source.Usage) + names = append(names, "format") + } + for _, name := range []string{"debug", "verbose"} { + source := rootFlags.Lookup(name) + if source == nil { continue } + value, err := rootFlags.GetBool(name) + if err != nil { + return fmt.Errorf("read presentation flag --%s: %w", name, err) + } + presentationFlags.BoolP(name, source.Shorthand, value, source.Usage) + names = append(names, name) + } + // Keep the existing contract in which a PreParse conflict wins over help; + // registering help prevents pflag's special unknown-help early return. + presentationFlags.BoolP("help", "h", false, "") - if !strings.HasPrefix(argument, "-") || argument == "-" { + parseErr := presentationFlags.Parse(rawArgs) + errs := []error{parseErr} + for _, name := range names { + if !presentationFlags.Changed(name) { continue } - shorthands := []rune(strings.TrimPrefix(argument, "-")) - for shorthandIndex, shorthand := range shorthands { - flag := matcher.byShorthand[string(shorthand)] - if flag == nil { - break - } - value, hasValue := "", false - if flag.NoOptDefVal != "" { - if shorthandIndex == 0 && len(shorthands) == 1 && index+1 < len(rawArgs) { - if normalized, ok := separatedBoolValue(argument, rawArgs[index+1], flag, false); ok { - index++ - value, hasValue = normalized, true - } else { - value, hasValue = flag.NoOptDefVal, true - } - } else { - value, hasValue = flag.NoOptDefVal, true - } - } else if shorthandIndex+1 < len(shorthands) { - value = string(shorthands[shorthandIndex+1:]) - value = strings.TrimPrefix(value, "=") - hasValue = true - } else if index+1 < len(rawArgs) { - index++ - value, hasValue = rawArgs[index], true - } - if presentation[flag.Name] && hasValue { - _ = rootFlags.Set(flag.Name, value) - } - if flag.NoOptDefVal == "" { - break - } + value := presentationFlags.Lookup(name).Value.String() + if err := rootFlags.Set(name, value); err != nil { + errs = append(errs, fmt.Errorf("apply presentation flag --%s: %w", name, err)) } } + return errors.Join(errs...) } // FlagInfoFromCommand extracts FlagInfo entries from a Cobra diff --git a/internal/pipeline/cobra_test.go b/internal/pipeline/cobra_test.go index 3a12055c..78d8d5bb 100644 --- a/internal/pipeline/cobra_test.go +++ b/internal/pipeline/cobra_test.go @@ -211,8 +211,8 @@ func TestRunPreParsePrimesPresentationFlagsForEarlyErrors(t *testing.T) { wantDebug: true, }, { - name: "corrected leading flags", - args: []string{"--dryRun", "--formt=pretty", "--verbos", "child", "--name", "demo"}, + name: "normalized presentation names", + args: []string{"--dryRun", "--FORMAT=pretty", "--Verbose", "child", "--name", "demo"}, wantFormat: "pretty", wantVerbose: true, }, @@ -223,8 +223,8 @@ func TestRunPreParsePrimesPresentationFlagsForEarlyErrors(t *testing.T) { wantVerbose: true, }, { - name: "detached boolean presentation values", - args: []string{"--debug", "true", "-v", "false", "child", "--name", "demo"}, + name: "explicit boolean presentation values", + args: []string{"--debug=true", "-v=false", "child", "--name", "demo"}, wantFormat: "json", wantDebug: true, wantVerbose: false, @@ -258,6 +258,32 @@ func TestRunPreParsePrimesPresentationFlagsForEarlyErrors(t *testing.T) { } } +func TestRunPreParseKeepsPrimaryErrorWhenPresentationParsingFails(t *testing.T) { + root := &cobra.Command{Use: "dws", SilenceErrors: true, SilenceUsage: true} + root.PersistentFlags().StringP("format", "f", "json", "") + root.PersistentFlags().Bool("debug", false, "") + root.PersistentFlags().BoolP("verbose", "v", false, "") + child := &cobra.Command{Use: "child"} + child.Flags().String("name", "", "") + root.AddCommand(child) + + primaryErr := errors.New("primary pre-parse failure") + engine := NewEngine() + engine.Register(newStub("fail", PreParse, func(*Context) error { return primaryErr })) + + ctx, err := RunPreParseArgs(root, engine, []string{ + "child", "--name", "demo", "--format", "table", "--debug=maybe", + }) + if ctx == nil || !errors.Is(err, primaryErr) { + t.Fatalf("RunPreParseArgs() = %#v, %v; want primary error", ctx, err) + } + format, _ := root.PersistentFlags().GetString("format") + debug, _ := root.PersistentFlags().GetBool("debug") + if format != "table" || debug { + t.Fatalf("partially valid presentation values = format:%q debug:%v", format, debug) + } +} + func TestCommandTraversalFlagTokenEdges(t *testing.T) { raw := []string{"child"} if got := argsForCommandTraversal(nil, raw); !reflect.DeepEqual(got, raw) { @@ -273,7 +299,7 @@ func TestCommandTraversalFlagTokenEdges(t *testing.T) { t.Fatalf("double-dash traversal args = %v", got) } - if flag, inline, matched := persistentFlagToken(nil, "--verbose"); flag != nil || inline || matched { + if flag, inline, matched := newFlagTokenMatcher(nil).matchTraversalToken("--verbose"); flag != nil || inline || matched { t.Fatalf("nil flag set matched: %#v, %v, %v", flag, inline, matched) } if flag, inline, matched := (*flagTokenMatcher)(nil).matchTraversalToken(""); flag != nil || inline || matched { @@ -282,13 +308,20 @@ func TestCommandTraversalFlagTokenEdges(t *testing.T) { if match := (*flagTokenMatcher)(nil).matchLongToken("--verbose"); match.recognized { t.Fatalf("nil long matcher matched: %#v", match) } - if flag, inline, matched := persistentFlagToken(root.PersistentFlags(), "-x"); flag != nil || inline || matched { + if flag, inline, matched := newFlagTokenMatcher(root.PersistentFlags()).matchTraversalToken("-x"); flag != nil || inline || matched { t.Fatalf("unknown shorthand matched: %#v, %v, %v", flag, inline, matched) } - flag, inline, matched := persistentFlagToken(root.PersistentFlags(), "-vv") + flag, inline, matched := newFlagTokenMatcher(root.PersistentFlags()).matchTraversalToken("-vv") if !matched || !inline || flag == nil || flag.Name != "verbose" { t.Fatalf("boolean shorthand cluster = %#v, %v, %v", flag, inline, matched) } + + duplicate := pflag.NewFlagSet("duplicate", pflag.ContinueOnError) + duplicate.Bool("verbose", false, "") + matcher := newFlagTokenMatcher(root.PersistentFlags(), duplicate) + if len(matcher.byName) != 2 || matcher.byName["verbose"] != root.PersistentFlags().Lookup("verbose") { + t.Fatalf("duplicate flag precedence = %#v", matcher.byName) + } } func TestSeparatedBoolValueRecognition(t *testing.T) { @@ -328,7 +361,9 @@ func TestSeparatedBoolValueRecognition(t *testing.T) { } func TestPrimeEarlyErrorPresentationEdges(t *testing.T) { - primeEarlyErrorPresentation(nil, nil, nil) + if err := primeEarlyErrorPresentation(nil, nil, nil); err != nil { + t.Fatalf("nil presentation priming error = %v", err) + } root := &cobra.Command{Use: "root"} root.PersistentFlags().StringP("format", "f", "json", "") @@ -338,10 +373,12 @@ func TestPrimeEarlyErrorPresentationEdges(t *testing.T) { child.Flags().String("name", "", "") root.AddCommand(child) - primeEarlyErrorPresentation(root, child, []string{ + if err := primeEarlyErrorPresentation(root, child, []string{ "child", "--unknown", "value", "-x", "--name", "demo", "-f", "table", "-v", "maybe", "--", "--debug", - }) + }); err != nil { + t.Fatalf("presentation priming error = %v", err) + } format, _ := root.PersistentFlags().GetString("format") debug, _ := root.PersistentFlags().GetBool("debug") verbose, _ := root.PersistentFlags().GetBool("verbose") @@ -349,3 +386,70 @@ func TestPrimeEarlyErrorPresentationEdges(t *testing.T) { t.Fatalf("presentation after edge argv = format:%q debug:%v verbose:%v", format, debug, verbose) } } + +func TestPrimeEarlyErrorPresentationReportsParseAndContractErrors(t *testing.T) { + t.Run("invalid presentation value is reported after applying valid values", func(t *testing.T) { + root := &cobra.Command{Use: "root"} + root.PersistentFlags().StringP("format", "f", "json", "") + root.PersistentFlags().Bool("debug", false, "") + root.PersistentFlags().BoolP("verbose", "v", false, "") + child := &cobra.Command{Use: "child"} + root.AddCommand(child) + + err := primeEarlyErrorPresentation(root, child, []string{"child", "--format", "table", "--debug=maybe"}) + if err == nil || !strings.Contains(err.Error(), "invalid argument") { + t.Fatalf("invalid presentation error = %v", err) + } + format, _ := root.PersistentFlags().GetString("format") + debug, _ := root.PersistentFlags().GetBool("debug") + if format != "table" || debug { + t.Fatalf("partially valid presentation values = format:%q debug:%v", format, debug) + } + }) + + for _, test := range []struct { + name string + add func(*pflag.FlagSet) + want string + }{ + {name: "format type drift", add: func(flags *pflag.FlagSet) { flags.Bool("format", false, "") }, want: "read presentation flag --format"}, + {name: "debug type drift", add: func(flags *pflag.FlagSet) { + flags.String("format", "json", "") + flags.String("debug", "", "") + }, want: "read presentation flag --debug"}, + } { + t.Run(test.name, func(t *testing.T) { + root := &cobra.Command{Use: "root"} + test.add(root.PersistentFlags()) + err := primeEarlyErrorPresentation(root, root, []string{"--format", "table"}) + if err == nil || !strings.Contains(err.Error(), test.want) { + t.Fatalf("presentation contract error = %v, want %q", err, test.want) + } + }) + } + + t.Run("apply error is returned", func(t *testing.T) { + root := &cobra.Command{Use: "root"} + value := &rejectingPresentationString{value: "json"} + root.PersistentFlags().VarP(value, "format", "f", "") + err := primeEarlyErrorPresentation(root, root, []string{"--format", "table"}) + if err == nil || !strings.Contains(err.Error(), "apply presentation flag --format") { + t.Fatalf("presentation apply error = %v", err) + } + }) + + t.Run("missing presentation flags are optional", func(t *testing.T) { + root := &cobra.Command{Use: "root"} + if err := primeEarlyErrorPresentation(root, root, []string{"--unknown", "value"}); err != nil { + t.Fatalf("optional presentation flags error = %v", err) + } + }) +} + +type rejectingPresentationString struct { + value string +} + +func (v *rejectingPresentationString) Set(string) error { return errors.New("rejected") } +func (v *rejectingPresentationString) String() string { return v.value } +func (*rejectingPresentationString) Type() string { return "string" } diff --git a/internal/pipeline/engine.go b/internal/pipeline/engine.go index 87496191..ad8ad9a4 100644 --- a/internal/pipeline/engine.go +++ b/internal/pipeline/engine.go @@ -24,6 +24,23 @@ type Engine struct { handlers map[Phase][]Handler } +// HandlerError preserves the pipeline location of a handler failure for logs +// and diagnostics while keeping the underlying domain error available to +// user-facing adapters through Unwrap. +type HandlerError struct { + Phase Phase + Handler string + Cause error +} + +func (e *HandlerError) Error() string { + return fmt.Sprintf("pipeline %s handler %q: %v", e.Phase, e.Handler, e.Cause) +} + +func (e *HandlerError) Unwrap() error { + return e.Cause +} + // NewEngine creates a pipeline engine with no registered handlers. func NewEngine() *Engine { return &Engine{ @@ -64,7 +81,7 @@ func (e *Engine) HasHandlers(phase Phase) bool { func (e *Engine) RunPhase(phase Phase, ctx *Context) error { for _, h := range e.handlers[phase] { if err := h.Handle(ctx); err != nil { - return fmt.Errorf("pipeline %s handler %q: %w", phase, h.Name(), err) + return &HandlerError{Phase: phase, Handler: h.Name(), Cause: err} } } return nil diff --git a/internal/pipeline/engine_test.go b/internal/pipeline/engine_test.go index fc55597d..002cd86e 100644 --- a/internal/pipeline/engine_test.go +++ b/internal/pipeline/engine_test.go @@ -168,6 +168,10 @@ func TestRunPhaseErrorAbortsChain(t *testing.T) { if !strings.Contains(err.Error(), "fail") { t.Errorf("error should contain handler name, got %q", err.Error()) } + var handlerErr *HandlerError + if !errors.As(err, &handlerErr) || handlerErr.Phase != PreParse || handlerErr.Handler != "fail" || handlerErr.Unwrap() != boom { + t.Fatalf("handler error = %#v, want pre-parse/fail wrapping boom", handlerErr) + } if !h1.called { t.Error("h1 should have been called") } @@ -265,6 +269,15 @@ func TestContextFlagProtectionAndConflictError(t *testing.T) { if !strings.Contains(err, `for --user on "dws demo run": --user-id, --uid`) { t.Fatalf("FlagConflictError.Error() = %q", err) } + + boolErr := (&BoolValueConflictError{ + Command: "dws demo run", + Flag: "--yes", + Values: []string{"true", "false"}, + }).Error() + if !strings.Contains(boolErr, `for --yes on "dws demo run": false, true`) { + t.Fatalf("BoolValueConflictError.Error() = %q", boolErr) + } } func TestPhaseString(t *testing.T) { diff --git a/internal/pipeline/flagmatch.go b/internal/pipeline/flagmatch.go index c823b4a5..a51816a7 100644 --- a/internal/pipeline/flagmatch.go +++ b/internal/pipeline/flagmatch.go @@ -79,8 +79,6 @@ func SplitStickyFlag(argument string, specByName map[string]FlagInfo) (StickyFla return StickyFlagPair{}, false } - bestLength := 0 - bestFlag := "" for index := len(bare) - 1; index >= 1; index-- { prefix := bare[:index] matchedFlag := "" @@ -91,27 +89,23 @@ func SplitStickyFlag(argument string, specByName map[string]FlagInfo) (StickyFla matchedFlag = normalized } } - if matchedFlag != "" && index > bestLength { - bestLength = index - bestFlag = matchedFlag - break + if matchedFlag == "" { + continue } - } - if bestFlag == "" { - return StickyFlagPair{}, false - } - suffix := bare[bestLength:] - spec := specByName[bestFlag] - if !cmdutil.SuffixLooksLikeValue(suffix, spec.Type, spec.Format, spec.Enum) { - return StickyFlagPair{}, false + suffix := bare[index:] + spec := specByName[matchedFlag] + if !cmdutil.SuffixLooksLikeValue(suffix, spec.Type, spec.Format, spec.Enum) { + return StickyFlagPair{}, false + } + inline := false + if spec.Type == "bool" || spec.Type == "boolean" { + suffix, _ = cmdutil.NormalizeBoolLiteral(suffix) + inline = true + } + return StickyFlagPair{Flag: "--" + matchedFlag, Value: suffix, Inline: inline}, true } - inline := false - if spec.Type == "bool" || spec.Type == "boolean" { - suffix, _ = cmdutil.NormalizeBoolLiteral(suffix) - inline = true - } - return StickyFlagPair{Flag: "--" + bestFlag, Value: suffix, Inline: inline}, true + return StickyFlagPair{}, false } // FuzzyMatchFlag returns the unique closest real long flag within the diff --git a/internal/pipeline/handlers/boolvalue.go b/internal/pipeline/handlers/boolvalue.go index 3dab0ba6..1df5ef6f 100644 --- a/internal/pipeline/handlers/boolvalue.go +++ b/internal/pipeline/handlers/boolvalue.go @@ -14,42 +14,49 @@ package handlers import ( + "sort" "strings" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/pipeline" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/cmdutil" ) -// BoolValueHandler makes an explicit, whitespace-separated boolean value -// unambiguous before pflag parsing. pflag interprets `--yes false` as a bare +// BoolValueHandler gives every real Cobra boolean flag one consistent input +// grammar before pflag parsing. pflag interprets `--yes false` as a bare // `--yes` (true) plus a positional `false`; rewriting the pair to -// `--yes=false` preserves the value the caller actually supplied. +// `--yes=false` preserves the value the caller actually supplied. The same +// rule applies to default-true, required, local, and inherited boolean flags: +// the handler preserves an explicit value and never substitutes a default. // // This handler runs after all flag-name handlers so camelCase, semantic // aliases, and conservative fuzzy corrections have already reached their -// canonical names. It consumes only exact boolean literals and stops at `--`. +// canonical names. It consumes only recognised boolean literals, stops at +// `--`, and rejects contradictory values for the same canonical flag so the +// result cannot depend on argv order. type BoolValueHandler struct{} func (BoolValueHandler) Name() string { return "boolvalue" } func (BoolValueHandler) Phase() pipeline.Phase { return pipeline.PreParse } func (BoolValueHandler) Handle(ctx *pipeline.Context) error { - if len(ctx.Args) < 2 || len(ctx.FlagSpecs) == 0 { + if len(ctx.Args) == 0 || len(ctx.FlagSpecs) == 0 { return nil } - tokens := make(map[string]pipeline.FlagInfo) + longNames := make(map[string]pipeline.FlagInfo) + shortNames := make(map[string]pipeline.FlagInfo) for _, spec := range ctx.FlagSpecs { if spec.Name == "" || (spec.Type != "bool" && spec.Type != "boolean") { continue } - tokens["--"+spec.Name] = spec + longNames[spec.Name] = spec if spec.Shorthand != "" { - tokens["-"+spec.Shorthand] = spec + shortNames[spec.Shorthand] = spec } } result := make([]string, 0, len(ctx.Args)) + valuesByFlag := make(map[string]map[string]bool) for index := 0; index < len(ctx.Args); index++ { argument := ctx.Args[index] if argument == "--" { @@ -57,24 +64,99 @@ func (BoolValueHandler) Handle(ctx *pipeline.Context) error { break } - spec, matched := tokens[argument] - if !matched || ctx.IsFlagProtected(spec.Name) || index+1 >= len(ctx.Args) { - result = append(result, argument) - continue - } - normalized, ok := cmdutil.NormalizeBoolLiteral(ctx.Args[index+1]) - if !ok { + spec, inlineValue, hasInlineValue, matched := matchBooleanFlagToken(argument, longNames, shortNames) + if !matched || ctx.IsFlagProtected(cmdutil.Morph(spec.Name)) { result = append(result, argument) continue } - corrected := "--" + spec.Name + "=" + normalized - original := strings.Join(ctx.Args[index:index+2], " ") - ctx.AddCorrection("boolvalue", pipeline.PreParse, "--"+spec.Name, original, corrected, "explicit-bool") + normalized := "true" + corrected := argument + original := argument + changed := false + if hasInlineValue { + var ok bool + normalized, ok = cmdutil.NormalizeBoolLiteral(inlineValue) + if !ok { + result = append(result, argument) + continue + } + corrected = "--" + spec.Name + "=" + normalized + changed = corrected != argument + } else if index+1 < len(ctx.Args) { + if value, ok := cmdutil.NormalizeBoolLiteral(ctx.Args[index+1]); ok { + normalized = value + corrected = "--" + spec.Name + "=" + normalized + original = strings.Join(ctx.Args[index:index+2], " ") + changed = true + index++ + } + } + + if valuesByFlag[spec.Name] == nil { + valuesByFlag[spec.Name] = make(map[string]bool) + } + valuesByFlag[spec.Name][normalized] = true + if changed { + ctx.AddCorrection("boolvalue", pipeline.PreParse, "--"+spec.Name, original, corrected, "explicit-bool") + } result = append(result, corrected) - index++ } ctx.Args = result + names := make([]string, 0, len(valuesByFlag)) + for name := range valuesByFlag { + names = append(names, name) + } + sort.Strings(names) + for _, name := range names { + values := valuesByFlag[name] + if len(values) < 2 { + continue + } + list := make([]string, 0, len(values)) + for value := range values { + list = append(list, value) + } + sort.Strings(list) + return &pipeline.BoolValueConflictError{ + Command: ctx.Command, + Flag: name, + Values: list, + } + } return nil } + +// matchBooleanFlagToken recognises canonical long flags, exact shorthands, +// and their explicit =value forms. Shorthand clusters remain native pflag +// syntax and are intentionally not reinterpreted here. +func matchBooleanFlagToken(argument string, longNames, shortNames map[string]pipeline.FlagInfo) (pipeline.FlagInfo, string, bool, bool) { + if strings.HasPrefix(argument, "--") { + bare, suffix, isFlag := splitFlagToken(argument) + if !isFlag { + return pipeline.FlagInfo{}, "", false, false + } + spec, ok := longNames[bare] + if !ok { + return pipeline.FlagInfo{}, "", false, false + } + if suffix == "" { + return spec, "", false, true + } + return spec, strings.TrimPrefix(suffix, "="), true, true + } + if !strings.HasPrefix(argument, "-") || strings.HasPrefix(argument, "--") { + return pipeline.FlagInfo{}, "", false, false + } + body := strings.TrimPrefix(argument, "-") + name, value, hasValue := body, "", false + if index := strings.IndexByte(body, '='); index >= 0 { + name, value, hasValue = body[:index], body[index+1:], true + } + spec, ok := shortNames[name] + if !ok { + return pipeline.FlagInfo{}, "", false, false + } + return spec, value, hasValue, true +} diff --git a/internal/pipeline/handlers/boolvalue_test.go b/internal/pipeline/handlers/boolvalue_test.go index 890e9937..bb4b0bde 100644 --- a/internal/pipeline/handlers/boolvalue_test.go +++ b/internal/pipeline/handlers/boolvalue_test.go @@ -14,6 +14,7 @@ package handlers import ( + "errors" "reflect" "testing" @@ -23,26 +24,35 @@ import ( func TestBoolValueHandler(t *testing.T) { boolFlags := []pipeline.FlagInfo{{Name: "yes", Shorthand: "y", Type: "bool"}} tests := []struct { - name string - args []string - flags []pipeline.FlagInfo - protected []string - want []string - correction bool + name string + args []string + flags []pipeline.FlagInfo + protected []string + want []string + corrections int + conflict bool }{ - {name: "long false", args: []string{"--yes", "false"}, flags: boolFlags, want: []string{"--yes=false"}, correction: true}, - {name: "long true synonym", args: []string{"--yes", "yes"}, flags: boolFlags, want: []string{"--yes=true"}, correction: true}, - {name: "shorthand zero", args: []string{"-y", "0"}, flags: boolFlags, want: []string{"--yes=false"}, correction: true}, - {name: "preserves following flags", args: []string{"--yes", "off", "--format", "json"}, flags: boolFlags, want: []string{"--yes=false", "--format", "json"}, correction: true}, + {name: "long false", args: []string{"--yes", "false"}, flags: boolFlags, want: []string{"--yes=false"}, corrections: 1}, + {name: "long mixed-case false", args: []string{"--yes", "FaLsE"}, flags: boolFlags, want: []string{"--yes=false"}, corrections: 1}, + {name: "long true synonym", args: []string{"--yes", "yes"}, flags: boolFlags, want: []string{"--yes=true"}, corrections: 1}, + {name: "shorthand zero", args: []string{"-y", "0"}, flags: boolFlags, want: []string{"--yes=false"}, corrections: 1}, + {name: "shorthand equals synonym", args: []string{"-y=off"}, flags: boolFlags, want: []string{"--yes=false"}, corrections: 1}, + {name: "preserves following flags", args: []string{"--yes", "off", "--format", "json"}, flags: boolFlags, want: []string{"--yes=false", "--format", "json"}, corrections: 1}, {name: "explicit equals stays native", args: []string{"--yes=false"}, flags: boolFlags, want: []string{"--yes=false"}}, + {name: "explicit equals synonym normalizes", args: []string{"--yes=No"}, flags: boolFlags, want: []string{"--yes=false"}, corrections: 1}, {name: "bare bool stays native", args: []string{"--yes"}, flags: boolFlags, want: []string{"--yes"}}, {name: "invalid literal is positional", args: []string{"--yes", "maybe"}, flags: boolFlags, want: []string{"--yes", "maybe"}}, + {name: "invalid inline literal stays native", args: []string{"--yes=maybe"}, flags: boolFlags, want: []string{"--yes=maybe"}}, {name: "non bool flag is unchanged", args: []string{"--name", "false"}, flags: []pipeline.FlagInfo{{Name: "name", Type: "string"}}, want: []string{"--name", "false"}}, {name: "unknown flag is unchanged", args: []string{"--confirm", "false"}, flags: boolFlags, want: []string{"--confirm", "false"}}, {name: "shorthand cluster is unchanged", args: []string{"-vy", "false"}, flags: boolFlags, want: []string{"-vy", "false"}}, {name: "protected bool is unchanged", args: []string{"--yes", "false"}, flags: boolFlags, protected: []string{"yes"}, want: []string{"--yes", "false"}}, + {name: "protected noncanonical bool uses morphed key", args: []string{"--dry_run", "false"}, flags: []pipeline.FlagInfo{{Name: "dry_run", Type: "bool"}}, protected: []string{"dry-run"}, want: []string{"--dry_run", "false"}}, {name: "stops at double dash", args: []string{"--", "--yes", "false"}, flags: boolFlags, want: []string{"--", "--yes", "false"}}, {name: "no specs", args: []string{"--yes", "false"}, want: []string{"--yes", "false"}}, + {name: "identical repeated values remain valid", args: []string{"--yes", "--yes=true", "--yes", "yes"}, flags: boolFlags, want: []string{"--yes", "--yes=true", "--yes=true"}, corrections: 1}, + {name: "contradictory detached values fail", args: []string{"--yes", "true", "--yes", "false"}, flags: boolFlags, want: []string{"--yes=true", "--yes=false"}, corrections: 2, conflict: true}, + {name: "bare and explicit false fail", args: []string{"--yes", "--yes=false"}, flags: boolFlags, want: []string{"--yes", "--yes=false"}, conflict: true}, } for _, test := range tests { @@ -55,20 +65,25 @@ func TestBoolValueHandler(t *testing.T) { ctx.ProtectFlag(protected, pipeline.FlagProtectionBlocked) } handler := BoolValueHandler{} - if err := handler.Handle(ctx); err != nil { + err := handler.Handle(ctx) + var conflict *pipeline.BoolValueConflictError + if test.conflict { + if !errors.As(err, &conflict) { + t.Fatalf("Handle() error = %v, want BoolValueConflictError", err) + } + if conflict.Flag != "yes" || !reflect.DeepEqual(conflict.Values, []string{"false", "true"}) { + t.Fatalf("conflict = %#v", conflict) + } + } else if err != nil { t.Fatalf("Handle() error = %v", err) } if !reflect.DeepEqual(ctx.Args, test.want) { t.Fatalf("Args = %#v, want %#v", ctx.Args, test.want) } - wantCorrections := 0 - if test.correction { - wantCorrections = 1 + if len(ctx.Corrections) != test.corrections { + t.Fatalf("Corrections = %#v, want %d", ctx.Corrections, test.corrections) } - if len(ctx.Corrections) != wantCorrections { - t.Fatalf("Corrections = %#v, want %d", ctx.Corrections, wantCorrections) - } - if test.correction { + if test.corrections > 0 { correction := ctx.Corrections[0] if correction.Handler != "boolvalue" || correction.Kind != "explicit-bool" || correction.Field != "--yes" { t.Fatalf("correction metadata = %#v", correction) @@ -78,6 +93,37 @@ func TestBoolValueHandler(t *testing.T) { } } +func TestMatchBooleanFlagToken(t *testing.T) { + longNames := map[string]pipeline.FlagInfo{"yes": {Name: "yes", Shorthand: "y", Type: "bool"}} + shortNames := map[string]pipeline.FlagInfo{"y": longNames["yes"]} + tests := []struct { + argument string + value string + hasValue bool + matched bool + }{ + {argument: "--yes", matched: true}, + {argument: "--yes=false", value: "false", hasValue: true, matched: true}, + {argument: "-y", matched: true}, + {argument: "-y=true", value: "true", hasValue: true, matched: true}, + {argument: "-vy"}, + {argument: "--unknown=false"}, + {argument: "yes"}, + {argument: "--"}, + } + for _, test := range tests { + t.Run(test.argument, func(t *testing.T) { + spec, value, hasValue, matched := matchBooleanFlagToken(test.argument, longNames, shortNames) + if value != test.value || hasValue != test.hasValue || matched != test.matched { + t.Fatalf("matchBooleanFlagToken(%q) = %#v, %q, %v, %v", test.argument, spec, value, hasValue, matched) + } + if matched && spec.Name != "yes" { + t.Fatalf("matched spec = %#v", spec) + } + }) + } +} + func TestBoolValueHandlerMeta(t *testing.T) { handler := BoolValueHandler{} if handler.Name() != "boolvalue" || handler.Phase() != pipeline.PreParse { diff --git a/internal/pipeline/phase.go b/internal/pipeline/phase.go index 9668cdba..fe4eb9b9 100644 --- a/internal/pipeline/phase.go +++ b/internal/pipeline/phase.go @@ -174,6 +174,22 @@ func (e *FlagConflictError) Error() string { return fmt.Sprintf("conflicting parameter spellings for --%s on %q: %s; pass exactly one spelling", e.Canonical, e.Command, strings.Join(spellings, ", ")) } +// BoolValueConflictError is returned when one canonical boolean flag receives +// both true and false in the same argv. Rejecting contradictory values keeps +// the outcome independent of argument order while allowing repeated identical +// spellings to retain Cobra's native behaviour. +type BoolValueConflictError struct { + Command string + Flag string + Values []string +} + +func (e *BoolValueConflictError) Error() string { + values := append([]string(nil), e.Values...) + sort.Strings(values) + return fmt.Sprintf("conflicting boolean values for --%s on %q: %s; pass exactly one value", strings.TrimPrefix(e.Flag, "--"), e.Command, strings.Join(values, ", ")) +} + // FlagInfo describes a single CLI flag derived from a tool's input // schema. PreParse handlers use this to recognise valid flag names // when performing fuzzy matching or alias resolution. @@ -182,8 +198,8 @@ type FlagInfo struct { Name string // Shorthand is the optional single-character pflag shorthand (e.g. "y" - // for --yes). PreParse uses it only when an exact shorthand token is - // followed by an explicit boolean literal. + // for --yes). PreParse uses exact shorthand tokens when normalising + // explicit boolean values; shorthand clusters retain native pflag syntax. Shorthand string // PropertyName is the original schema property key (e.g. "userId"). diff --git a/pkg/cmdutil/sticky_suffix.go b/pkg/cmdutil/sticky_suffix.go index 9d370889..46970773 100644 --- a/pkg/cmdutil/sticky_suffix.go +++ b/pkg/cmdutil/sticky_suffix.go @@ -114,7 +114,7 @@ func startsWithNumericSuffix(s string) bool { // NormalizeBoolLiteral reduces model-friendly boolean spellings to the exact // values accepted by an unambiguous --flag=true/false pflag token. func NormalizeBoolLiteral(s string) (string, bool) { - switch strings.ToLower(s) { + switch strings.ToLower(strings.TrimSpace(s)) { case "true", "1", "t", "yes", "on", "y": return "true", true case "false", "0", "f", "no", "off", "n": diff --git a/pkg/cmdutil/sticky_suffix_test.go b/pkg/cmdutil/sticky_suffix_test.go index f38b3c3e..8c006327 100644 --- a/pkg/cmdutil/sticky_suffix_test.go +++ b/pkg/cmdutil/sticky_suffix_test.go @@ -15,6 +15,28 @@ package cmdutil import "testing" +func TestNormalizeBoolLiteral(t *testing.T) { + tests := []struct { + input string + want string + ok bool + }{ + {input: "TRUE", want: "true", ok: true}, + {input: " yes ", want: "true", ok: true}, + {input: "0", want: "false", ok: true}, + {input: "Off", want: "false", ok: true}, + {input: "maybe"}, + } + for _, test := range tests { + t.Run(test.input, func(t *testing.T) { + got, ok := NormalizeBoolLiteral(test.input) + if got != test.want || ok != test.ok { + t.Fatalf("NormalizeBoolLiteral(%q) = %q, %v; want %q, %v", test.input, got, ok, test.want, test.ok) + } + }) + } +} + // TestSuffixLooksLikeValue_UTF8FirstRune locks in the UTF-8 first-rune // reading contract on SuffixLooksLikeValue. The function previously read // suffix[0] (a single byte) which produced incorrect splits / matches for