-
Notifications
You must be signed in to change notification settings - Fork 2
support integer types #81
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds normalized type identifiers (, , , ) to unify validation logic, expands numeric (all integer) support, introduces gt/lt operations, enhances multi-value parsing (space or comma), updates code generation, tests, and README.
- Normalizes type handling across analyzer and code generator (operations & condition tables now keyed by normalized types).
- Expands integer coverage in validators and tests; adds gt/lt and multi-separator parsing for multi-value tags.
- Introduces helper utilities for normalized/base type conversions and updates README with new status indicators.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/endtoend/validator__.go | Generated validators for all integer types exercising new operations. |
| tests/endtoend/numeric.go | Generated numeric E2E test cases covering all integer variants and operations. |
| tests/endtoend/main.go | Invokes new numeric test suite. |
| tests/endtoend/generate_tests/numeric.tpl | Template for generating numeric struct tests. |
| tests/endtoend/generate_tests/generate_numeric_tests_main.go | Generates numeric test file using normalized type expansion. |
| tests/cmpbenchtests/generate_cmp_benchtests_main.go | Switches from html/template to text/template (appropriate for code gen). |
| internal/parser/parser_test.go | Adds float32 parsing coverage. |
| internal/common/types_test.go | Tests new normalization and expansion helpers. |
| internal/common/types.go | Adds normalization utilities and reverse expansion; updates IsGoType set. |
| internal/common/normalized_base_type_test.go | Tests NormalizedBaseType String method. |
| internal/common/normalized_base_type.go | Defines NormalizedBaseType enum. |
| internal/codegenerator/test_elements.go | Looks up conditions via normalized type string. |
| internal/codegenerator/get_test_elements_numeric_test.go | Adds broad numeric operation tests using normalized expansion. |
| internal/codegenerator/get_test_elements_errors_test.go | Adjusts invalid operation test input. |
| internal/codegenerator/condition_table.go | Re-keys condition mappings to normalized type tokens; adds gt/lt and numeric coverage. |
| internal/analyzer/parser_validation_test.go | Tests multi-value parsing with commas. |
| internal/analyzer/parser_validation.go | Supports comma or space separated multi-values. |
| internal/analyzer/operations_test.go | Refactors tests to normalized types and iterates over expansions. |
| internal/analyzer/operations.go | Normalized operation/type matrix; adds gt/lt and broader integer support. |
| internal/analyzer/analyzer_test.go | Adds int variants for field-to-field operations. |
| internal/analyzer/analyzer.go | Uses normalized type string for operation validation. |
| README.md | Updates validation capability matrix (integer scope, partial statuses). |
| Makefile | Generates numeric tests prior to E2E run. |
Comments suppressed due to low confidence (3)
internal/analyzer/operations.go:1
- Tests (operations_test.go) expect support for slices and arrays of integers (types "[]" and "[N]") for required, in, and nin, but these normalized types are absent from the ValidTypes maps here. This mismatch will cause those tests to fail with invalid type errors. Add "[]" (and for in/nin also "[N]") to the corresponding ValidTypes maps (and ensure matching condition table entries) or remove the test cases if not intended to be supported yet.
package analyzer
internal/codegenerator/condition_table.go:68
- Support for integer slices/arrays ("[]", "[N]") is referenced in tests (operations_test.go) but there are no corresponding ConditionByType entries here for required, in, or nin. Even if added to operations.go, lookups will still fail without matching condition templates. Add entries for "[]" and "[N]" (using numeric slice containment helpers) and for "[]" in required with the same non-empty length check as string slices.
"required": {
Name: "required",
ConditionByType: map[string]ConditionTable{
"<STRING>": {
operation: `obj.{{.Name}} != ""`,
concatOperator: "",
errorMessage: "{{.Name}} is required",
},
"<INT>": {
operation: `obj.{{.Name}} != 0`,
concatOperator: "",
errorMessage: "{{.Name}} is required",
},
"[]<STRING>": {
operation: `len(obj.{{.Name}}) != 0`,
concatOperator: "",
errorMessage: "{{.Name}} must not be empty",
},
"map[<STRING>]": {
operation: `len(obj.{{.Name}}) >= 1`,
concatOperator: "",
errorMessage: "{{.Name}} must not be empty",
},
"map[<INT>]": {
operation: `len(obj.{{.Name}}) >= 1`,
concatOperator: "",
errorMessage: "{{.Name}} must not be empty",
},
},
},
internal/codegenerator/condition_table.go:292
- Support for integer slices/arrays ("[]", "[N]") is referenced in tests (operations_test.go) but there are no corresponding ConditionByType entries here for required, in, or nin. Even if added to operations.go, lookups will still fail without matching condition templates. Add entries for "[]" and "[N]" (using numeric slice containment helpers) and for "[]" in required with the same non-empty length check as string slices.
"in": {
Name: "in",
ConditionByType: map[string]ConditionTable{
"<STRING>": {
operation: `obj.{{.Name}} == "{{.Target}}"`,
concatOperator: "||",
errorMessage: "{{.Name}} must be one of {{.Targets}}",
},
"<INT>": {
operation: `obj.{{.Name}} == {{.Target}}`,
concatOperator: "||",
errorMessage: "{{.Name}} must be one of {{.Targets}}",
},
"[]<STRING>": {
operation: `types.SliceOnlyContains(obj.{{.Name}}, {{.TargetsAsStringSlice}})`,
concatOperator: "",
errorMessage: "{{.Name}} elements must be one of {{.Targets}}",
},
"[N]<STRING>": {
operation: `types.SliceOnlyContains(obj.{{.Name}}[:], {{.TargetsAsStringSlice}})`,
concatOperator: "",
errorMessage: "{{.Name}} elements must be one of {{.Targets}}",
},
"map[<STRING>]": {
operation: `types.MapOnlyContains(obj.{{.Name}}, {{.TargetsAsStringSlice}})`,
concatOperator: "",
errorMessage: "{{.Name}} elements must be one of {{.Targets}}",
},
"map[<INT>]": {
operation: `types.MapOnlyContains(obj.{{.Name}}, {{.TargetsAsNumericSlice}})`,
concatOperator: "",
errorMessage: "{{.Name}} elements must be one of {{.Targets}}",
},
},
},
"nin": {
Name: "nin",
ConditionByType: map[string]ConditionTable{
"<STRING>": {
operation: `obj.{{.Name}} != "{{.Target}}"`,
concatOperator: "&&",
errorMessage: "{{.Name}} must not be one of {{.Targets}}",
},
"<INT>": {
operation: `obj.{{.Name}} != {{.Target}}`,
concatOperator: "&&",
errorMessage: "{{.Name}} must not be one of {{.Targets}}",
},
"[]<STRING>": {
operation: `types.SliceNotContains(obj.{{.Name}}, {{.TargetsAsStringSlice}})`,
concatOperator: "",
errorMessage: "{{.Name}} elements must not be one of {{.Targets}}",
},
"[N]<STRING>": {
operation: `types.SliceNotContains(obj.{{.Name}}[:], {{.TargetsAsStringSlice}})`,
concatOperator: "",
errorMessage: "{{.Name}} elements must not be one of {{.Targets}}",
},
"map[<STRING>]": {
operation: `types.MapNotContains(obj.{{.Name}}, {{.TargetsAsStringSlice}})`,
concatOperator: "",
errorMessage: "{{.Name}} elements must not be one of {{.Targets}}",
},
"map[<INT>]": {
operation: `types.MapNotContains(obj.{{.Name}}, {{.TargetsAsNumericSlice}})`,
concatOperator: "",
errorMessage: "{{.Name}} elements must not be one of {{.Targets}}",
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 8 comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Instead of 'type NumericTypeuint struct' generates 'NumericTypeUint'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
internal/analyzer/analyzer.go:1
- The map key is stored using common.KeyPath(package, structName) but later looked up with only fdType.BaseType; if KeyPath returns anything other than the bare struct name (e.g. adds package), the lookup will fail and custom structs with validations may be incorrectly treated as unsupported types. Store and look up using a consistent key (either just struct name in both places or always KeyPath) or add an additional direct structName key when populating the map.
package analyzer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Closes #59
This PR introduces significant improvements to how validation operations are defined and handled across the codebase. The changes standardize type representations, expand support for numeric types, and enhance parsing and testing of validation tags. These updates make the validator more robust, consistent, and easier to extend for future types and operations.
Type System and Operation Support
string,uint8,bool) with generic placeholders (<STRING>,<INT>,<BOOL>) throughout the validation logic and code generation tables. This enables broader and more consistent support for all integer types and simplifies future expansions. [1] [2] [3]gt,lt) and extended existing operations (eq,neq,gte,lte, etc.) to all integer types, not justuint8. This is reflected in both the operation definitions and code generation logic. [1] [2]Validation and Parsing Enhancements
Testing and Internal Logic
These changes collectively make the validation system more extensible, reliable, and easier to maintain.