Merge 185510de5f into ceb39aba25

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 {

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}