diff --git a/pkg/ide/ideparse/parse.go b/pkg/ide/ideparse/parse.go index cedf2b69fc..ae9b1820f3 100644 --- a/pkg/ide/ideparse/parse.go +++ b/pkg/ide/ideparse/parse.go @@ -386,38 +386,45 @@ func ParseOptions(options []string, ideOptions ide.Options) (map[string]config.O } func validateOptionValue(ideOption ide.Option, key, value string) error { - if ideOption.ValidationPattern != "" { - matcher, err := regexp.Compile(ideOption.ValidationPattern) - if err != nil { - return err - } + if err := validateOptionPattern(ideOption, key, value); err != nil { + return err + } - if !matcher.MatchString(value) { - if ideOption.ValidationMessage != "" { - return fmt.Errorf("%s", ideOption.ValidationMessage) - } + if len(ideOption.Enum) == 0 || slices.Contains(ideOption.Enum, value) { + return nil + } - return fmt.Errorf( - "invalid value %q for option %q, has to match the following regEx: %s", - value, - key, - ideOption.ValidationPattern, - ) - } + return fmt.Errorf( + "invalid value %q for option %q, has to match one of the following values: %v", + value, + key, + ideOption.Enum, + ) +} + +func validateOptionPattern(ideOption ide.Option, key, value string) error { + if ideOption.ValidationPattern == "" { + return nil } - if len(ideOption.Enum) > 0 { - if !slices.Contains(ideOption.Enum, value) { - return fmt.Errorf( - "invalid value %q for option %q, has to match one of the following values: %v", - value, - key, - ideOption.Enum, - ) - } + matcher, err := regexp.Compile(ideOption.ValidationPattern) + if err != nil { + return err + } + + if matcher.MatchString(value) { + return nil + } + if ideOption.ValidationMessage != "" { + return fmt.Errorf("%s", ideOption.ValidationMessage) } - return nil + return fmt.Errorf( + "invalid value %q for option %q, has to match the following regEx: %s", + value, + key, + ideOption.ValidationPattern, + ) } func detect() string { diff --git a/pkg/ide/ideparse/parse_test.go b/pkg/ide/ideparse/parse_test.go index 2b79ee3660..cf11181ed9 100644 --- a/pkg/ide/ideparse/parse_test.go +++ b/pkg/ide/ideparse/parse_test.go @@ -1,9 +1,13 @@ package ideparse import ( + "errors" + "reflect" + "regexp/syntax" "testing" "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/ide" "github.com/devsy-org/devsy/pkg/provider" ) @@ -93,3 +97,305 @@ func TestRefreshIDEOptions_EmptyIDEKeepsExisting(t *testing.T) { t.Errorf("returned workspace.IDE.Name = %q, want %q", got.IDE.Name, ideOpenVSCode) } } + +const ( + testOptionKey = "OPTION" + testLettersPattern = `^[a-z]+$` + testAllowedValue = "allowed" + testSecondEnumValue = "Second" + testInvalidPattern = `[abc` +) + +type optionValidationCase struct { + name string + option ide.Option + key string + value string + wantErr string +} + +func TestValidateOptionValue(t *testing.T) { + t.Run("unconstrained empty value ignores message", func(t *testing.T) { + assertValidationCases(t, []optionValidationCase{{ + option: ide.Option{ValidationMessage: "ignored"}, + key: testOptionKey, + value: "", + }}) + }) + t.Run("regex behavior", testValidateRegexCases) + t.Run("enum behavior", testValidateEnumCases) + t.Run("regex precedes enum", testValidateOrderCases) +} + +func testValidateRegexCases(t *testing.T) { + assertValidationCases(t, []optionValidationCase{ + { + name: "complete match", + option: ide.Option{ValidationPattern: testLettersPattern}, + key: testOptionKey, + value: "value", + }, + { + name: "unanchored MatchString", + option: ide.Option{ValidationPattern: `abc`}, + key: testOptionKey, + value: "xabcx", + }, + { + name: "default mismatch error", + option: ide.Option{ValidationPattern: testLettersPattern}, + key: testOptionKey, + value: "123", + wantErr: `invalid value "123" for option "OPTION", has to match the following regEx: ^[a-z]+$`, + }, + { + name: "custom message is literal with percent signs", + option: ide.Option{ + ValidationPattern: `^ok$`, + ValidationMessage: "bad %s value: 100%", + }, + key: testOptionKey, + value: "no", + wantErr: "bad %s value: 100%", + }, + }) +} + +func testValidateEnumCases(t *testing.T) { + values := []string{"First", testSecondEnumValue} + assertValidationCases(t, []optionValidationCase{ + { + name: "accepts exact value", + option: ide.Option{Enum: values}, + key: testOptionKey, + value: testSecondEnumValue, + }, + { + name: "case sensitive and display order preserved", + option: ide.Option{Enum: values}, + key: testOptionKey, + value: "second", + wantErr: `invalid value "second" for option "OPTION", has to match one of the following values: [First Second]`, + }, + }) +} + +func testValidateOrderCases(t *testing.T) { + assertValidationCases(t, []optionValidationCase{ + { + name: "regex failure precedes enum failure", + option: ide.Option{ + ValidationPattern: `^match$`, + Enum: []string{testAllowedValue}, + }, + key: testOptionKey, + value: "other", + wantErr: `invalid value "other" for option "OPTION", has to match the following regEx: ^match$`, + }, + { + name: "regex success is followed by enum rejection", + option: ide.Option{ + ValidationPattern: testLettersPattern, + Enum: []string{testAllowedValue}, + }, + key: testOptionKey, + value: "other", + wantErr: `invalid value "other" for option "OPTION", has to match one of the following values: [allowed]`, + }, + { + name: "regex success is followed by enum acceptance", + option: ide.Option{ + ValidationPattern: testLettersPattern, + Enum: []string{testAllowedValue}, + }, + key: testOptionKey, + value: testAllowedValue, + }, + }) +} + +func assertValidationCases(t *testing.T, cases []optionValidationCase) { + t.Helper() + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateOptionValue(tc.option, tc.key, tc.value) + assertError(t, err, tc.wantErr) + }) + } +} + +func TestValidateOptionValueInvalidPattern(t *testing.T) { + option := ide.Option{ + ValidationPattern: testInvalidPattern, + ValidationMessage: "custom validation message", + } + err := validateOptionValue(option, testOptionKey, "value") + syntaxErr, ok := errors.AsType[*syntax.Error](err) + if !ok { + t.Fatalf("validateOptionValue() error = %T (%v), want *syntax.Error", err, err) + } + if err.Error() != "error parsing regexp: missing closing ]: `[abc`" { + t.Errorf("validateOptionValue() error = %q, want regexp syntax error", err) + } + if syntaxErr.Expr != testInvalidPattern { + t.Errorf("syntax error expression = %q, want %q", syntaxErr.Expr, testInvalidPattern) + } +} + +func TestParseOptions(t *testing.T) { + t.Run("normalization and values", testParseOptionValues) + t.Run("input errors", testParseOptionInputErrors) + t.Run("validation order", testParseOptionValidation) + t.Run("invalid pattern preserves syntax error", testParseInvalidPattern) +} + +type parseOptionsCase struct { + name string + options []string + definitions ide.Options + want map[string]config.OptionValue + wantErr string +} + +func testParseOptionValues(t *testing.T) { + assertParseOptionsCases(t, []parseOptionsCase{ + { + name: "trimmed key is uppercased", + options: []string{" editor =value"}, + definitions: ide.Options{"EDITOR": {}}, + want: map[string]config.OptionValue{ + "EDITOR": {Value: "value", UserProvided: true}, + }, + }, + { + name: "value whitespace and extra equals are preserved", + options: []string{testOptionKey + "= value with spaces =tail "}, + definitions: ide.Options{testOptionKey: {}}, + want: map[string]config.OptionValue{ + testOptionKey: {Value: " value with spaces =tail ", UserProvided: true}, + }, + }, + { + name: "empty value is allowed", + options: []string{testOptionKey + "="}, + definitions: ide.Options{testOptionKey: {}}, + want: map[string]config.OptionValue{ + testOptionKey: {Value: "", UserProvided: true}, + }, + }, + { + name: "duplicate key uses last value", + options: []string{testOptionKey + "=first", "option=last"}, + definitions: ide.Options{testOptionKey: {}}, + want: map[string]config.OptionValue{ + testOptionKey: {Value: "last", UserProvided: true}, + }, + }, + { + name: "nil definitions allow no options", + definitions: nil, + want: map[string]config.OptionValue{}, + }, + }) +} + +func testParseOptionInputErrors(t *testing.T) { + assertParseOptionsCases(t, []parseOptionsCase{ + { + name: "missing equals is rejected", + options: []string{testOptionKey}, + definitions: ide.Options{testOptionKey: {}}, + wantErr: `invalid option "OPTION", expected format KEY=VALUE`, + }, + { + name: "unknown option has deterministic one-item list", + options: []string{"UNKNOWN=value"}, + definitions: ide.Options{"KNOWN": {}}, + wantErr: `invalid option "UNKNOWN", allowed options are: [KNOWN]`, + }, + }) +} + +func testParseOptionValidation(t *testing.T) { + assertParseOptionsCases(t, []parseOptionsCase{ + { + name: "validation error returns nil map after earlier valid option", + options: []string{"FIRST=valid", "SECOND=invalid"}, + definitions: ide.Options{ + "FIRST": {}, + "SECOND": {ValidationPattern: `^valid$`}, + }, + wantErr: `invalid value "invalid" for option "SECOND", has to match the following regEx: ^valid$`, + }, + { + name: "regex success can still fail enum", + options: []string{testOptionKey + "=other"}, + definitions: ide.Options{ + testOptionKey: { + ValidationPattern: testLettersPattern, + Enum: []string{testAllowedValue}, + }, + }, + wantErr: `invalid value "other" for option "OPTION", has to match one of the following values: [allowed]`, + }, + { + name: "regex failure takes precedence when enum accepts", + options: []string{testOptionKey + "=" + testAllowedValue}, + definitions: ide.Options{ + testOptionKey: {ValidationPattern: `^other$`, Enum: []string{testAllowedValue}}, + }, + wantErr: `invalid value "allowed" for option "OPTION", has to match the following regEx: ^other$`, + }, + }) +} + +func testParseInvalidPattern(t *testing.T) { + got, err := ParseOptions( + []string{testOptionKey + "=value"}, + ide.Options{ + testOptionKey: { + ValidationPattern: testInvalidPattern, + ValidationMessage: "custom message", + }, + }, + ) + _, ok := errors.AsType[*syntax.Error](err) + if !ok { + t.Fatalf("ParseOptions() error = %T (%v), want *syntax.Error", err, err) + } + if got != nil { + t.Errorf("ParseOptions() map = %#v, want nil on error", got) + } +} + +func assertParseOptionsCases(t *testing.T, cases []parseOptionsCase) { + t.Helper() + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := ParseOptions(tc.options, tc.definitions) + assertError(t, err, tc.wantErr) + if tc.wantErr != "" { + if got != nil { + t.Errorf("ParseOptions() map = %#v, want nil on error", got) + } + return + } + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("ParseOptions() = %#v, want %#v", got, tc.want) + } + }) + } +} + +func assertError(t *testing.T, err error, want string) { + t.Helper() + if want == "" { + if err != nil { + t.Fatalf("error = %v, want nil", err) + } + return + } + if err == nil || err.Error() != want { + t.Fatalf("error = %v, want %q", err, want) + } +}