From da4990848697bcc204d4a4defc680a27f4e5efb5 Mon Sep 17 00:00:00 2001
From: Burak Ok <burakok@microsoft.com>
Date: Wed, 21 Dec 2022 16:57:06 +0100
Subject: [PATCH 1/2] Command: Added new method getClashingFlagnames
---
command.go | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/command.go b/command.go
index 1b0b17a2..27385c9b 100644
--- a/command.go
+++ b/command.go
@@ -1641,6 +1641,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.
func (c *Command) InheritedFlags() *flag.FlagSet {
c.mergePersistentFlags()
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 2/2] 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}