From 185510de5f419a867a97b73525bab94b181fde85 Mon Sep 17 00:00:00 2001
From: Burak Ok <burakok@microsoft.com>
Date: Wed, 21 Dec 2022 17:11:32 +0100
Subject: [PATCH] Command: Add new field ErrorOnShadowedFlags and error on
 shadowed flag in ExecuteC

---
 command.go      | 14 +++++++
 command_test.go | 97 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

diff --git a/command.go b/command.go
index 27385c9b..f0ed416e 100644
--- a/command.go
+++ b/command.go
@@ -244,6 +244,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
@@ -1065,6 +1068,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
diff --git a/command_test.go b/command_test.go
index 18011325..5e7c01d5 100644
--- a/command_test.go
+++ b/command_test.go
@@ -898,6 +898,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}