Skip to content

JGC-434 - Fix parsing of undeclared plugins args #1513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ehl-jf merged 1 commit into from
Jan 5, 2026
+12 −245
Merged

JGC-434 - Fix parsing of undeclared plugins args #1513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 12 additions & 24 deletions plugins/components/conversionlayer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,14 +456,20 @@ func getValueForStringFlag(f StringFlag, baseContext *cli.Context) (string, erro
return value, nil
}
// We try to find the flag value in the command arguments.
flagIndex, flagValue := findFlag(f.Name, baseContext.Args())
flagIndex, _, flagValue, _ := coreutils.FindFlag(f.Name, baseContext.Args())
if flagIndex == -1 {
flagIndex, _, flagValue, _ = coreutils.FindFlag("--"+f.Name, baseContext.Args())
}

if flagIndex != -1 {
return flagValue, nil
}

if f.DefaultValue != "" {
// Empty but has a default value defined.
return f.DefaultValue, nil
}

if f.Mandatory {
// Empty but mandatory.
return "", errors.New("Mandatory flag '" + f.Name + "' is missing")
Expand All @@ -477,32 +483,14 @@ func getValueForBoolFlag(f BoolFlag, baseContext *cli.Context) bool {
}

// We try to find the flag value in the command arguments.
flagIndex, flagValue := findFlag(f.Name, baseContext.Args())
flagIndex, flagValue, _ := coreutils.FindBooleanFlag(f.Name, baseContext.Args())
if flagIndex == -1 {
flagIndex, flagValue, _ = coreutils.FindBooleanFlag("--"+f.Name, baseContext.Args())
}

if flagIndex != -1 {
switch strings.ToLower(flagValue) {
case "true":
return true
case "false":
return false
case "":
return true
default:
return false
}
return flagValue
}

return f.DefaultValue
}

func findFlag(flagName string, args []string) (flagIndex int, flagValue string) {
var err error
flagIndex, _, flagValue, err = coreutils.FindFlag(flagName, args)
if err != nil {
return
}
if flagIndex == -1 {
flagIndex, _, flagValue, _ = coreutils.FindFlag("--"+flagName, args)
}
return flagIndex, flagValue
}
221 changes: 0 additions & 221 deletions plugins/components/conversionlayer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,125 +308,6 @@ func TestGetValueForStringFlag(t *testing.T) {
assert.Equal(t, finalValue, expected)
}

func TestGetValueForStringFlag_FindFlag(t *testing.T) {
t.Run("FlagNotSetInContext_FoundInArgs_WithValue", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description")
// Test findFlag directly to verify it finds the flag in args
testArgs := []string{"arg1", "--string-flag=test-value", "arg2"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "test-value", flagValue, "Flag value should be extracted correctly")
})

t.Run("FlagNotSetInContext_FoundInArgs_WithSpaceSeparatedValue", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description")
// Test that findFlag can find flag with space-separated value
testArgs := []string{"arg1", "--string-flag", "test-value", "arg2"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "test-value", flagValue, "Flag value should be extracted from next argument")
})

t.Run("FlagNotSetInContext_FoundInArgs_OverridesDefault", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description", WithStrDefaultValue("default-value"))
// Test that when flag is found in args, it overrides default
testArgs := []string{"--string-flag=arg-value"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "arg-value", flagValue, "Flag value from args should override default")
assert.NotEqual(t, f.DefaultValue, flagValue, "Flag value should not be the default")
})

t.Run("FlagNotSetInContext_NotFoundInArgs_UsesDefault", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description", WithStrDefaultValue("default-value"))
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
flagSet.String(f.Name, "", "")
assert.NoError(t, flagSet.Parse([]string{}))
baseContext := cli.NewContext(nil, flagSet, nil)

// Flag not in args, should use default
testArgs := []string{"arg1", "arg2"}
flagIndex, _ := findFlag(f.Name, testArgs)
assert.Equal(t, -1, flagIndex, "Flag should not be found in args")
// When flagIndex == -1, function should return default value
finalValue, err := getValueForStringFlag(f, baseContext)
assert.NoError(t, err)
assert.Equal(t, f.DefaultValue, finalValue, "Should return default value when flag not found")
})

t.Run("FlagNotSetInContext_NotFoundInArgs_Mandatory_ReturnsError", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description", SetMandatory())
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
flagSet.String(f.Name, "", "")
assert.NoError(t, flagSet.Parse([]string{}))
baseContext := cli.NewContext(nil, flagSet, nil)

// Flag not in args and mandatory, should return error
testArgs := []string{"arg1", "arg2"}
flagIndex, _ := findFlag(f.Name, testArgs)
assert.Equal(t, -1, flagIndex, "Flag should not be found in args")
// When flagIndex == -1 and mandatory, function should return error
_, err := getValueForStringFlag(f, baseContext)
assert.Error(t, err, "Should return error for mandatory flag when not found")
assert.Contains(t, err.Error(), "Mandatory flag")
assert.Contains(t, err.Error(), f.Name)
})

t.Run("FlagNotSetInContext_NotFoundInArgs_Optional_ReturnsEmpty", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description")
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
flagSet.String(f.Name, "", "")
assert.NoError(t, flagSet.Parse([]string{}))
baseContext := cli.NewContext(nil, flagSet, nil)

// Flag not in args and optional, should return empty
testArgs := []string{"arg1", "arg2"}
flagIndex, _ := findFlag(f.Name, testArgs)
assert.Equal(t, -1, flagIndex, "Flag should not be found in args")
// When flagIndex == -1 and optional, function should return empty string
finalValue, err := getValueForStringFlag(f, baseContext)
assert.NoError(t, err)
assert.Empty(t, finalValue, "Should return empty string for optional flag when not found")
})

t.Run("FlagSetInContext_TakesPrecedenceOverArgs", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description")
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
flagSet.String(f.Name, "", "")
contextValue := "context-value"
assert.NoError(t, flagSet.Parse([]string{"--string-flag=" + contextValue}))
baseContext := cli.NewContext(nil, flagSet, nil)

// Flag is set in context, should return context value even if args have different value
finalValue, err := getValueForStringFlag(f, baseContext)
assert.NoError(t, err)
assert.Equal(t, contextValue, finalValue, "Flag set in context should take precedence")
})

t.Run("FindFlag_WithEqualsSign", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description")
testArgs := []string{"--string-flag=value-with-equals"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "value-with-equals", flagValue, "Flag value with equals should be extracted correctly")
})

t.Run("FindFlag_WithoutEqualsSign", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description")
testArgs := []string{"--string-flag", "value-without-equals"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "value-without-equals", flagValue, "Flag value without equals should be extracted correctly")
})

t.Run("FindFlag_SimilarPrefix_DoesNotMatch", func(t *testing.T) {
f := NewStringFlag("string-flag", "Test description")
testArgs := []string{"--string-flag-other=value"}
flagIndex, _ := findFlag(f.Name, testArgs)
assert.Equal(t, -1, flagIndex, "Similar flag prefix should not match")
})
}

func TestGetValueForBoolFlag(t *testing.T) {
t.Run("FlagSetInContext_True", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
Expand Down Expand Up @@ -462,43 +343,6 @@ func TestGetValueForBoolFlag(t *testing.T) {
assert.True(t, result, "BoolT flag set in context should return true")
})

t.Run("FlagNotSetInContext_FoundInArgs_WithValueTrue", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
// Test the findFlag function directly to verify the logic
testArgs := []string{"arg1", "--bool-flag=true", "arg2"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "true", flagValue, "Flag value should be 'true'")
// When flagValue is "true", the function returns true
})

t.Run("FlagNotSetInContext_FoundInArgs_WithValueFalse", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
testArgs := []string{"arg1", "--bool-flag=false", "arg2"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "false", flagValue, "Flag value should be 'false'")
// When flagValue is "false", the function returns false
})

t.Run("FlagNotSetInContext_FoundInArgs_EmptyValue", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
testArgs := []string{"arg1", "--bool-flag", "arg2"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
// When flagValue is empty (""), the function returns true
assert.True(t, flagValue == "" || flagValue == "arg2", "Flag value should be empty or next arg")
})

t.Run("FlagNotSetInContext_FoundInArgs_InvalidValue", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
testArgs := []string{"arg1", "--bool-flag=invalid", "arg2"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "invalid", flagValue, "Flag value should be 'invalid'")
// When flagValue is not "true", "false", or "", the function returns false
})

t.Run("FlagNotSetInContext_NotFoundInArgs_DefaultTrue", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
Expand Down Expand Up @@ -532,51 +376,6 @@ func TestGetValueForBoolFlag(t *testing.T) {
assert.False(t, result, "Flag set in context should override default value")
})

t.Run("FlagNotSetInContext_NotFoundInArgs_SimilarPrefix", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
// Test that similar prefix doesn't match
testArgs := []string{"--bool-flag-other"}
flagIndex, _ := findFlag(f.Name, testArgs)
assert.Equal(t, -1, flagIndex, "Similar flag prefix should not match")
// When flagIndex == -1, the function returns default value
})

t.Run("FlagNotSetInContext_FoundInArgs_CaseInsensitiveTrue", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
testArgs := []string{"--bool-flag=TRUE"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "TRUE", flagValue, "Flag value should be 'TRUE'")
// The function uses strings.ToLower, so "TRUE" becomes "true" and returns true
})

t.Run("FlagNotSetInContext_FoundInArgs_CaseInsensitiveFalse", func(t *testing.T) {
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
testArgs := []string{"--bool-flag=FALSE"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
assert.Equal(t, "FALSE", flagValue, "Flag value should be 'FALSE'")
// The function uses strings.ToLower, so "FALSE" becomes "false" and returns false
})

t.Run("FlagValueParsing_True", func(t *testing.T) {
// Test that flagValue "true" returns true
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
testArgs := []string{"--bool-flag=true"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex)
assert.Equal(t, "true", flagValue)
})

t.Run("FlagValueParsing_False", func(t *testing.T) {
// Test that flagValue "false" returns false
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
testArgs := []string{"--bool-flag=false"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex)
assert.Equal(t, "false", flagValue)
})

t.Run("FlagValueParsing_Empty", func(t *testing.T) {
// Test that empty flagValue returns true
// When a flag is provided without a value like "--bool-flag",
Expand All @@ -599,26 +398,6 @@ func TestGetValueForBoolFlag(t *testing.T) {
// Note: findFlag may return -1 for flags without values due to FindFlag error handling
// but the important part is that the switch statement correctly handles "" -> true
})

t.Run("FlagValueParsing_InvalidValue", func(t *testing.T) {
// Test that invalid flagValue returns false
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
testArgs := []string{"--bool-flag=invalid"}
flagIndex, flagValue := findFlag(f.Name, testArgs)
assert.NotEqual(t, -1, flagIndex)
assert.Equal(t, "invalid", flagValue)
// Verify the switch statement logic: "invalid" -> false (default case)
switch strings.ToLower(flagValue) {
case "true":
assert.Fail(t, "Should not match 'true'")
case "false":
assert.Fail(t, "Should not match 'false'")
case "":
assert.Fail(t, "Should not match empty")
default:
assert.True(t, true, "Invalid flag value should result in false (default case)")
}
})
}

type DummyFlagValue struct {
Expand Down
Loading