diff --git a/command.go b/command.go index 4794e5eb..7af63583 100644 --- a/command.go +++ b/command.go @@ -257,6 +257,9 @@ type Command struct { // SuggestionsMinimumDistance defines minimum levenshtein distance to display suggestions. // Must be > 0. SuggestionsMinimumDistance int + + // ErrorOnShadowedFlags will check for any shadowed flags when calling Execute and returns and error + ErrorOnShadowedFlags bool } // Context returns underlying command context. If command was executed @@ -1145,6 +1148,17 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { cmd.ctx = c.ctx } + if c.ErrorOnShadowedFlags { + clashingFlags := cmd.getClashingFlagnames() + if len(clashingFlags) != 0 { + errorString := "Error: The following flags would shadow each other:\n" + for _, clashingFlag := range clashingFlags { + errorString += fmt.Sprintf(" - Flag '%s' with usage '%s'\n", clashingFlag.Name, clashingFlag.Usage) + } + return cmd, fmt.Errorf(errorString) + } + } + err = cmd.execute(flags) if err != nil { // Always show help if requested, even if SilenceErrors is in @@ -1739,6 +1753,41 @@ func (c *Command) LocalFlags() *flag.FlagSet { return c.lflags } +// getClashingFlagnames returns all flags which names are used more than once +func (c *Command) getClashingFlagnames() []*flag.Flag { + allFlags := make(map[string]*flag.Flag, 0) + clashingFlagsSet := make(map[*flag.Flag]interface{}, 0) + + checkFlagFunc := func(f *flag.Flag) { + savedFlag, exists := allFlags[f.Name] + if !exists { + allFlags[f.Name] = f + } else if savedFlag != f /* Different flag or not */ { + clashingFlagsSet[f] = nil + clashingFlagsSet[savedFlag] = nil + } + } + + c.VisitParents(func(parent *Command) { + if parent.pflags != nil { + parent.pflags.VisitAll(checkFlagFunc) + } + }) + + if c.pflags != nil { + c.pflags.VisitAll(checkFlagFunc) + } + + // Possibility that we Visit some of the flags which we already visited in c.pflags + c.LocalFlags().VisitAll(checkFlagFunc) + + clashingFlags := make([]*flag.Flag, 0, len(clashingFlagsSet)) + for flag := range clashingFlagsSet { + clashingFlags = append(clashingFlags, flag) + } + return clashingFlags +} + // InheritedFlags returns all flags which were inherited from parent commands. // This function does not modify the flags of the current command, it's purpose is to return the current state. func (c *Command) InheritedFlags() *flag.FlagSet { diff --git a/command_test.go b/command_test.go index 156df9eb..c9103921 100644 --- a/command_test.go +++ b/command_test.go @@ -963,6 +963,103 @@ func TestHelpCommandExecutedOnChild(t *testing.T) { checkStringContains(t, output, childCmd.Long) } +func TestErrorOnShadowedFlagsOnShadowedFlag(t *testing.T) { + parent := &Command{Use: "parent", Run: emptyRun, ErrorOnShadowedFlags: true} + child := &Command{Use: "child", Run: emptyRun} + parent.AddCommand(child) + + parent.PersistentFlags().Bool("foo", false, "parent foo usage") + parent.PersistentFlags().Bool("bar", false, "parent bar usage") + child.Flags().Bool("foo", false, "child foo usage") // This shadows parent's foo flag + child.Flags().Bool("baz", false, "child baz usage") + + _, err := executeCommand(parent, "child") + if err == nil { + t.Errorf("Expected an error") + return + } + + expectedErrorStrings := []string{ + " - Flag 'foo' with usage 'parent foo usage'", + " - Flag 'foo' with usage 'child foo usage'", + } + + errorString := err.Error() + for _, expectedErrorString := range expectedErrorStrings { + if !strings.Contains(errorString, expectedErrorString) { + t.Errorf("Expected the string \"%s\" in the error", expectedErrorString) + } + } +} + +func TestErrorOnShadowedFlagsOnSelfShadowedFlag(t *testing.T) { + parent := &Command{Use: "parent", Run: emptyRun, ErrorOnShadowedFlags: true} + + parent.PersistentFlags().Bool("foo", false, "parent persistent foo usage") + parent.Flags().Bool("foo", false, "parent foo usage") + + _, err := executeCommand(parent, "parent") + if err == nil { + t.Errorf("Expected an error") + return + } + + expectedErrorStrings := []string{ + " - Flag 'foo' with usage 'parent persistent foo usage'", + " - Flag 'foo' with usage 'parent foo usage'", + } + + errorString := err.Error() + for _, expectedErrorString := range expectedErrorStrings { + if !strings.Contains(errorString, expectedErrorString) { + t.Errorf("String %q is not present in error: %q", expectedErrorString, err) + } + } +} + +func TestErrorOnShadowedFlagsOnNoShadowedFlag(t *testing.T) { + parent := &Command{Use: "parent", Run: emptyRun, ErrorOnShadowedFlags: true} + + parent.PersistentFlags().Bool("foo", false, "parent persistent foo usage") + parent.Flags().Bool("bar", false, "parent bar usage") + + _, err := executeCommand(parent, "child") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } +} + +func TestErrorOnShadowedOnShadowedFlagParentAndChild(t *testing.T) { + parent := &Command{Use: "parent", Run: emptyRun, ErrorOnShadowedFlags: true} + child := &Command{Use: "child", Run: emptyRun} + grandchild := &Command{Use: "grandchild", Run: emptyRun} + parent.AddCommand(child) + child.AddCommand(grandchild) + + parent.PersistentFlags().Bool("foo", false, "parent foo usage") + parent.PersistentFlags().Bool("bar", false, "parent bar usage") + child.PersistentFlags().Bool("bar", false, "child bar usage") + grandchild.Flags().Bool("baz", false, "grandchild baz usage") + + _, err := executeCommand(parent, "child", "grandchild") + if err == nil { + t.Errorf("Expected an error") + return + } + + expectedErrorStrings := []string{ + " - Flag 'bar' with usage 'parent bar usage'", + " - Flag 'bar' with usage 'child bar usage'", + } + + errorString := err.Error() + for _, expectedErrorString := range expectedErrorStrings { + if !strings.Contains(errorString, expectedErrorString) { + t.Errorf("Expected the string \"%s\" in the error", expectedErrorString) + } + } +} + func TestHelpCommandExecutedOnChildWithFlagThatShadowsParentFlag(t *testing.T) { parent := &Command{Use: "parent", Run: emptyRun} child := &Command{Use: "child", Run: emptyRun}