adding isInArray and the isNotInArray and IsNotIn#10
adding isInArray and the isNotInArray and IsNotIn#10kthehatter wants to merge 2 commits intomainfrom
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes add three new validation functions to enhance membership checking: Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Application
participant Validator as New Validator
participant Data as Array/Values
Caller->>Validator: Invoke validation (e.g., IsInArray, IsNotInArray)
Validator->>Data: Iterate over values
alt Match Found
Validator-->>Caller: Return error (or nil if non-match for IsNotInArray)
else No Match Found
Validator-->>Caller: Return nil (or error for IsInArray)
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package : could not load export data: no export data for "github.com/gin-gonic/gin"" Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
validator/is.go (1)
96-132: Add input validation for array functionsBoth
IsInArrayandIsNotInArrayfunctions lack validation for nil input and proper panic prevention.Consider adding nil checks before using reflection operations:
func IsInArray(array interface{}) ValidatorFunc { return func(value interface{}) error { + if array == nil { + return fmt.Errorf("array parameter cannot be nil") + } // rest of the function... } } func IsNotInArray(array interface{}) ValidatorFunc { return func(value interface{}) error { + if array == nil { + return fmt.Errorf("array parameter cannot be nil") + } // rest of the function... } }validator/is_test.go (1)
116-170: Consider testing with more complex data typesThe current tests only validate behavior with primitive types like strings and integers. Consider adding tests with complex types like structs, maps, or slices to ensure the validators work correctly with all data types.
Add test cases for complex types:
// In TestIsInArray { "complex type in array", []interface{}{"apple", map[string]int{"a": 1}}, map[string]int{"a": 1}, nil, // Should find it }, // In TestIsNotInArray { "complex type not in array", []interface{}{"apple", map[string]int{"b": 2}}, map[string]int{"a": 1}, nil, // Should not find it },🧰 Tools
🪛 GitHub Actions: Go Tests
[error] 130-130: TestIsNotIn/wrong_type failed: expected error 'value must not be one of [apple banana cherry]', got
[error] 149-149: TestIsInArray/invalid_value failed: expected error 'value must be one of [apple banana cherry]', got 'value is not in the array'
[error] 149-149: TestIsInArray/wrong_type failed: expected error 'value must be one of [apple banana cherry]', got
[error] 167-167: TestIsNotInArray/invalid_value failed: expected error 'value must not be one of [apple banana cherry]', got 'value is in the array'
[error] 167-167: TestIsNotInArray/wrong_type failed: expected error 'value must not be one of [apple banana cherry]', got
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
validator/is.go(1 hunks)validator/is_test.go(1 hunks)validator/transformers_test.go(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Go Tests
validator/is_test.go
[error] 130-130: TestIsNotIn/wrong_type failed: expected error 'value must not be one of [apple banana cherry]', got
[error] 149-149: TestIsInArray/invalid_value failed: expected error 'value must be one of [apple banana cherry]', got 'value is not in the array'
[error] 149-149: TestIsInArray/wrong_type failed: expected error 'value must be one of [apple banana cherry]', got
[error] 167-167: TestIsNotInArray/invalid_value failed: expected error 'value must not be one of [apple banana cherry]', got 'value is in the array'
[error] 167-167: TestIsNotInArray/wrong_type failed: expected error 'value must not be one of [apple banana cherry]', got
🔇 Additional comments (1)
validator/transformers_test.go (1)
174-174: LGTM: Good test case additionThis new test case validates the
Replacetransformer's ability to handle multiple occurrences of the target substring in a single input string, which improves test coverage.
| // IsNotIn checks if a value is not in a predefined list of disallowed values. | ||
| func IsNotIn(disallowedValues ...interface{}) ValidatorFunc { | ||
| return func(value interface{}) error { | ||
| for _, disallowed := range disallowedValues { | ||
| if value == disallowed { | ||
| return fmt.Errorf("value must not be one of %v", disallowedValues) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
Implementation needs type handling improvement
The current implementation doesn't properly handle the "wrong type" case according to the failing test. When comparing values of different types using ==, Go returns false rather than an error.
func IsNotIn(disallowedValues ...interface{}) ValidatorFunc {
return func(value interface{}) error {
for _, disallowed := range disallowedValues {
- if value == disallowed {
+ // Use reflect to better handle different types
+ if reflect.DeepEqual(value, disallowed) {
return fmt.Errorf("value must not be one of %v", disallowedValues)
}
}
return nil
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // IsNotIn checks if a value is not in a predefined list of disallowed values. | |
| func IsNotIn(disallowedValues ...interface{}) ValidatorFunc { | |
| return func(value interface{}) error { | |
| for _, disallowed := range disallowedValues { | |
| if value == disallowed { | |
| return fmt.Errorf("value must not be one of %v", disallowedValues) | |
| } | |
| } | |
| return nil | |
| } | |
| } | |
| // IsNotIn checks if a value is not in a predefined list of disallowed values. | |
| func IsNotIn(disallowedValues ...interface{}) ValidatorFunc { | |
| return func(value interface{}) error { | |
| for _, disallowed := range disallowedValues { | |
| // Use reflect to better handle different types | |
| if reflect.DeepEqual(value, disallowed) { | |
| return fmt.Errorf("value must not be one of %v", disallowedValues) | |
| } | |
| } | |
| return nil | |
| } | |
| } |
| func TestIsNotIn(t *testing.T) { | ||
| isNotIn := IsNotIn("apple", "banana", "cherry") | ||
| tests := []struct { | ||
| name string | ||
| input interface{} | ||
| error error | ||
| }{ | ||
| {"valid value", "grape", nil}, | ||
| {"invalid value", "apple", errors.New("value must not be one of [apple banana cherry]")}, | ||
| {"wrong type", 123, errors.New("value must not be one of [apple banana cherry]")}, | ||
| } | ||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| err := isNotIn(test.input) | ||
| require.Equal(t, test.error, err) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure tests align with implementation behavior
The test expects IsNotIn to handle type mismatches differently than it currently does. This needs to be addressed in both the test and the implementation.
🏁 Script executed:
#!/bin/bash
# Check how other equality checks handle type mismatches in the codebase
echo "Checking implementations of equality checks in the validator package..."
rg "if .+ == .+\s+" validator/Length of output: 1032
Action: Reconcile type mismatch handling in IsNotIn
The test in validator/is_test.go (lines 116–133) expects that passing a value of the wrong type (e.g., an integer 123) to the IsNotIn function produces an error with the message "value must not be one of [apple banana cherry]". However, based on how equality checks are implemented in the rest of the validator package (as seen in validator/is.go where comparisons like if value == allowed are used without special type handling), the current IsNotIn function likely does not treat type mismatches explicitly.
To resolve this discrepancy, you should either:
- Update the implementation: Modify
IsNotIn(likely invalidator/is.go) to detect mismatched types and return the expected error message, or - Adjust the test: Change the expectation in
TestIsNotInso that the wrong-type case either returns no error or uses a different error message that aligns with the current behavior.
Please ensure that the chosen approach is consistent with how similar validations are handled elsewhere in the codebase.
🧰 Tools
🪛 GitHub Actions: Go Tests
[error] 130-130: TestIsNotIn/wrong_type failed: expected error 'value must not be one of [apple banana cherry]', got
| func TestIsInArray(t *testing.T) { | ||
| isInArray := IsInArray([]string{"apple", "banana", "cherry"}) | ||
| tests := []struct { | ||
| name string | ||
| input interface{} | ||
| error error | ||
| }{ | ||
| {"valid value", "apple", nil}, | ||
| {"invalid value", "grape", errors.New("value must be one of [apple banana cherry]")}, | ||
| {"wrong type", 123, errors.New("value must be one of [apple banana cherry]")}, | ||
| } | ||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| err := isInArray(test.input) | ||
| require.Equal(t, test.error, err) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix error message assertion in tests
The test assertions don't match the current implementation's error messages, leading to test failures. Either update the tests to match the implementation or modify the implementation to match the tests.
func TestIsInArray(t *testing.T) {
isInArray := IsInArray([]string{"apple", "banana", "cherry"})
tests := []struct {
name string
input interface{}
error error
}{
{"valid value", "apple", nil},
- {"invalid value", "grape", errors.New("value must be one of [apple banana cherry]")},
- {"wrong type", 123, errors.New("value must be one of [apple banana cherry]")},
+ {"invalid value", "grape", errors.New("value is not in the array")},
+ {"wrong type", 123, nil}, // Current implementation returns nil for type mismatches
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := isInArray(test.input)
require.Equal(t, test.error, err)
})
}
}Note: The better approach is to fix the implementation as suggested in the is.go file review.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestIsInArray(t *testing.T) { | |
| isInArray := IsInArray([]string{"apple", "banana", "cherry"}) | |
| tests := []struct { | |
| name string | |
| input interface{} | |
| error error | |
| }{ | |
| {"valid value", "apple", nil}, | |
| {"invalid value", "grape", errors.New("value must be one of [apple banana cherry]")}, | |
| {"wrong type", 123, errors.New("value must be one of [apple banana cherry]")}, | |
| } | |
| for _, test := range tests { | |
| t.Run(test.name, func(t *testing.T) { | |
| err := isInArray(test.input) | |
| require.Equal(t, test.error, err) | |
| }) | |
| } | |
| } | |
| func TestIsInArray(t *testing.T) { | |
| isInArray := IsInArray([]string{"apple", "banana", "cherry"}) | |
| tests := []struct { | |
| name string | |
| input interface{} | |
| error error | |
| }{ | |
| {"valid value", "apple", nil}, | |
| {"invalid value", "grape", errors.New("value is not in the array")}, | |
| {"wrong type", 123, nil}, // Current implementation returns nil for type mismatches | |
| } | |
| for _, test := range tests { | |
| t.Run(test.name, func(t *testing.T) { | |
| err := isInArray(test.input) | |
| require.Equal(t, test.error, err) | |
| }) | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Go Tests
[error] 149-149: TestIsInArray/invalid_value failed: expected error 'value must be one of [apple banana cherry]', got 'value is not in the array'
[error] 149-149: TestIsInArray/wrong_type failed: expected error 'value must be one of [apple banana cherry]', got
| func TestIsNotInArray(t *testing.T) { | ||
| isNotInArray := IsNotInArray([]string{"apple", "banana", "cherry"}) | ||
| tests := []struct { | ||
| name string | ||
| input interface{} | ||
| error error | ||
| }{ | ||
| {"valid value", "grape", nil}, | ||
| {"invalid value", "apple", errors.New("value must not be one of [apple banana cherry]")}, | ||
| {"wrong type", 123, errors.New("value must not be one of [apple banana cherry]")}, | ||
| } | ||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| err := isNotInArray(test.input) | ||
| require.Equal(t, test.error, err) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix error message assertion in tests
Similar to TestIsInArray, the error messages don't match the current implementation.
func TestIsNotInArray(t *testing.T) {
isNotInArray := IsNotInArray([]string{"apple", "banana", "cherry"})
tests := []struct {
name string
input interface{}
error error
}{
{"valid value", "grape", nil},
- {"invalid value", "apple", errors.New("value must not be one of [apple banana cherry]")},
- {"wrong type", 123, errors.New("value must not be one of [apple banana cherry]")},
+ {"invalid value", "apple", errors.New("value is in the array")},
+ {"wrong type", 123, nil}, // Current implementation returns nil for type mismatches
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := isNotInArray(test.input)
require.Equal(t, test.error, err)
})
}
}Note: The better approach is to fix the implementation as suggested in the is.go file review.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestIsNotInArray(t *testing.T) { | |
| isNotInArray := IsNotInArray([]string{"apple", "banana", "cherry"}) | |
| tests := []struct { | |
| name string | |
| input interface{} | |
| error error | |
| }{ | |
| {"valid value", "grape", nil}, | |
| {"invalid value", "apple", errors.New("value must not be one of [apple banana cherry]")}, | |
| {"wrong type", 123, errors.New("value must not be one of [apple banana cherry]")}, | |
| } | |
| for _, test := range tests { | |
| t.Run(test.name, func(t *testing.T) { | |
| err := isNotInArray(test.input) | |
| require.Equal(t, test.error, err) | |
| }) | |
| } | |
| } | |
| func TestIsNotInArray(t *testing.T) { | |
| isNotInArray := IsNotInArray([]string{"apple", "banana", "cherry"}) | |
| tests := []struct { | |
| name string | |
| input interface{} | |
| error error | |
| }{ | |
| {"valid value", "grape", nil}, | |
| {"invalid value", "apple", errors.New("value is in the array")}, | |
| {"wrong type", 123, nil}, // Current implementation returns nil for type mismatches | |
| } | |
| for _, test := range tests { | |
| t.Run(test.name, func(t *testing.T) { | |
| err := isNotInArray(test.input) | |
| require.Equal(t, test.error, err) | |
| }) | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Go Tests
[error] 167-167: TestIsNotInArray/invalid_value failed: expected error 'value must not be one of [apple banana cherry]', got 'value is in the array'
[error] 167-167: TestIsNotInArray/wrong_type failed: expected error 'value must not be one of [apple banana cherry]', got
1 participant
Descriptions of the Functions
IsNotIn
Purpose: Validates that a given value is not present in a predefined list of disallowed values.
How It Works: It takes a variadic list of disallowedValues (of type interface{}) and checks if the input value matches any of them using ==. If a match is found, it returns an error; otherwise, it returns nil.
Error Message: "value must not be one of %v", where %v is the list of disallowed values.
Use Case: Useful for ensuring a value isn’t one of a specific set (e.g., banning certain strings like "admin" or "root").
Limitations: The comparison uses ==, which might not work as expected for complex types (e.g., slices or structs) unless they’re comparable.
IsInArray
Purpose: Validates that a given value is present in a provided array or slice.
How It Works: It uses reflection (reflect.ValueOf) to iterate over the elements of the input array (of type interface{}) and checks if the value matches any element using Interface() == value. Returns nil if found, or an error if not.
Error Message: "value is not in the array".
Use Case: Ensures a value is within an allowed set (e.g., a dropdown menu’s options).
Limitations: The error message doesn’t reflect the array’s contents, and reflection can be slow for large arrays.
IsNotInArray
Purpose: Validates that a given value is not present in a provided array or slice.
How It Works: Similar to IsInArray, it iterates over the array using reflection. If the value matches any element, it returns an error; otherwise, it returns nil.
Error Message: "value is in the array".
Use Case: Ensures a value isn’t in a forbidden list (e.g., excluding reserved keywords).
Limitations: Same as IsInArray—error message lacks context, and reflection is used.
Summary by CodeRabbit
New Features
Tests