diff --git a/binding.go b/binding.go index 1a1c9c7..4ff8d79 100644 --- a/binding.go +++ b/binding.go @@ -14,27 +14,33 @@ import ( // tagConfig holds parsed directives from a struct field's `conf` tag. type tagConfig struct { - env string // Environment variable name (env:VAR_NAME) - name string // Custom key path (name:custom.path) - prefix string // Prefix for nested structs (prefix:foo) - defValue string // Default value (default:value) - min string // Minimum constraint (min:N) - max string // Maximum constraint (max:M) - oneof []string // Allowed values (oneof:a,b,c) - required bool // Field is required (required or required:true) - secret bool // Field is secret (secret or secret:true) - hasDefault bool // Whether a default directive was present + env string // Environment variable name (env:VAR_NAME) + name string // Custom key path (name:custom.path) + prefix string // Prefix for nested structs (prefix:foo) + defValue string // Default value (default:value) + min string // Minimum constraint (min:N) + max string // Maximum constraint (max:M) + oneof []string // Allowed values (oneof:a,b,c) + required bool // Field is required (required or required:true) + secret bool // Field is secret (secret or secret:true) + hasDefault bool // Whether a default directive was present + parseErrors []string // Errors from malformed tag directives } var tagConfigCache sync.Map var optionalTypePkgPath = reflect.TypeOf(Optional[int]{}).PkgPath() func cloneTagConfig(cfg tagConfig) tagConfig { - if len(cfg.oneof) == 0 { + if len(cfg.oneof) == 0 && len(cfg.parseErrors) == 0 { return cfg } - cfg.oneof = append([]string(nil), cfg.oneof...) + if len(cfg.oneof) > 0 { + cfg.oneof = append([]string(nil), cfg.oneof...) + } + if len(cfg.parseErrors) > 0 { + cfg.parseErrors = append([]string(nil), cfg.parseErrors...) + } return cfg } @@ -111,8 +117,9 @@ func parseTag(tag string) tagConfig { } else if value == "false" { cfg.required = false } else { - // Invalid value, default to true for safety cfg.required = true + cfg.parseErrors = append(cfg.parseErrors, + fmt.Sprintf("invalid required value %q: use true or false", value)) } case "secret": // No value or explicit "true" means true @@ -121,9 +128,13 @@ func parseTag(tag string) tagConfig { } else if value == "false" { cfg.secret = false } else { - // Invalid value, default to true for safety cfg.secret = true + cfg.parseErrors = append(cfg.parseErrors, + fmt.Sprintf("invalid secret value %q: use true or false", value)) } + default: + cfg.parseErrors = append(cfg.parseErrors, + fmt.Sprintf("unknown directive %q", name)) } } diff --git a/binding_test.go b/binding_test.go index dc9606d..0936a09 100644 --- a/binding_test.go +++ b/binding_test.go @@ -116,8 +116,9 @@ func TestBinding_ParseTag(t *testing.T) { name: "default with comma terminates directive", tag: "default:a,b,c", expected: tagConfig{ - defValue: "a", - hasDefault: true, + defValue: "a", + hasDefault: true, + parseErrors: []string{`unknown directive "b"`, `unknown directive "c"`}, }, }, { @@ -364,14 +365,16 @@ func TestBinding_ParseTag(t *testing.T) { name: "required with invalid value defaults to true", tag: "required:invalid", expected: tagConfig{ - required: true, + required: true, + parseErrors: []string{`invalid required value "invalid": use true or false`}, }, }, { name: "required with numeric value defaults to true", tag: "required:1", expected: tagConfig{ - required: true, + required: true, + parseErrors: []string{`invalid required value "1": use true or false`}, }, }, { @@ -399,14 +402,16 @@ func TestBinding_ParseTag(t *testing.T) { name: "secret with invalid value defaults to true", tag: "secret:invalid", expected: tagConfig{ - secret: true, + secret: true, + parseErrors: []string{`invalid secret value "invalid": use true or false`}, }, }, { name: "secret with yes defaults to true", tag: "secret:yes", expected: tagConfig{ - secret: true, + secret: true, + parseErrors: []string{`invalid secret value "yes": use true or false`}, }, }, { @@ -574,29 +579,35 @@ func TestBinding_ParseTag(t *testing.T) { // Unknown directives { - name: "unknown directive ignored", + name: "unknown directive reported", tag: "unknown:value,env:VAR", expected: tagConfig{ - env: "VAR", + env: "VAR", + parseErrors: []string{`unknown directive "unknown"`}, }, }, { name: "multiple unknown directives", tag: "foo:bar,env:VAR,baz:qux,required", expected: tagConfig{ - env: "VAR", - required: true, + env: "VAR", + required: true, + parseErrors: []string{`unknown directive "foo"`, `unknown directive "baz"`}, }, }, { - name: "only unknown directives", - tag: "unknown:value,another:thing", - expected: tagConfig{}, + name: "only unknown directives", + tag: "unknown:value,another:thing", + expected: tagConfig{ + parseErrors: []string{`unknown directive "unknown"`, `unknown directive "another"`}, + }, }, { - name: "typo in directive name", - tag: "envv:VAR,requiired:true", // intentional typos to test silent ignore - expected: tagConfig{}, + name: "typo in directive name", + tag: "envv:VAR,requiired:true", + expected: tagConfig{ + parseErrors: []string{`unknown directive "envv"`, `unknown directive "requiired"`}, + }, }, // Edge cases @@ -655,6 +666,9 @@ func TestBinding_ParseTag(t *testing.T) { if result.secret != tt.expected.secret { t.Errorf("secret: got %v, want %v", result.secret, tt.expected.secret) } + if !reflect.DeepEqual(result.parseErrors, tt.expected.parseErrors) { + t.Errorf("parseErrors: got %v, want %v", result.parseErrors, tt.expected.parseErrors) + } }) } } diff --git a/errors.go b/errors.go index f5c4af9..d7114c7 100644 --- a/errors.go +++ b/errors.go @@ -13,6 +13,7 @@ const ( ErrCodeOneOf = "oneof" // Value is not in the allowed set ErrCodeInvalidType = "invalid_type" // Type conversion failed ErrCodeUnknownKey = "unknown_key" // Configuration key doesn't map to any field (strict mode) + ErrCodeInvalidTag = "invalid_tag" // Struct tag directive is malformed or unrecognized ) // ValidationError aggregates field-level validation failures. diff --git a/validate.go b/validate.go index 3a57839..468890d 100644 --- a/validate.go +++ b/validate.go @@ -20,6 +20,19 @@ func validateFieldWithPresence(fieldValue reflect.Value, fieldPath string, tags var errors []FieldError fieldPresent := presentFields != nil && presentFields[fieldPath] + // Surface tag parse errors (malformed directives, unknown directives, etc.) + for _, msg := range tags.parseErrors { + errors = append(errors, FieldError{ + FieldPath: fieldPath, + Code: ErrCodeInvalidTag, + Message: msg, + }) + } + + // Validate min/max tag parseability upfront so malformed constraints + // are reported even when value validation is skipped (e.g., zero/missing values). + errors = append(errors, validateMinMaxParseable(fieldValue, fieldPath, tags)...) + // Check required constraint if tags.required { if presentFields != nil { @@ -187,6 +200,70 @@ func isZeroValue(v reflect.Value) bool { } } +// validateMinMaxParseable checks that min/max tag values can be parsed for the given field kind. +// Called before early returns so malformed constraints are always reported. +func validateMinMaxParseable(fieldValue reflect.Value, fieldPath string, tags tagConfig) []FieldError { + var errors []FieldError + + kind := fieldValue.Kind() + switch kind { + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + if tags.min != "" { + if _, err := strconv.ParseInt(tags.min, 10, 64); err != nil { + errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag, + Message: fmt.Sprintf("invalid min constraint %q: must be a valid integer", tags.min)}) + } + } + if tags.max != "" { + if _, err := strconv.ParseInt(tags.max, 10, 64); err != nil { + errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag, + Message: fmt.Sprintf("invalid max constraint %q: must be a valid integer", tags.max)}) + } + } + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + if tags.min != "" { + if _, err := strconv.ParseUint(tags.min, 10, 64); err != nil { + errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag, + Message: fmt.Sprintf("invalid min constraint %q: must be a valid unsigned integer", tags.min)}) + } + } + if tags.max != "" { + if _, err := strconv.ParseUint(tags.max, 10, 64); err != nil { + errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag, + Message: fmt.Sprintf("invalid max constraint %q: must be a valid unsigned integer", tags.max)}) + } + } + case reflect.Float32, reflect.Float64: + if tags.min != "" { + if _, err := strconv.ParseFloat(tags.min, 64); err != nil { + errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag, + Message: fmt.Sprintf("invalid min constraint %q: must be a valid number", tags.min)}) + } + } + if tags.max != "" { + if _, err := strconv.ParseFloat(tags.max, 64); err != nil { + errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag, + Message: fmt.Sprintf("invalid max constraint %q: must be a valid number", tags.max)}) + } + } + case reflect.String: + if tags.min != "" { + if _, err := strconv.Atoi(tags.min); err != nil { + errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag, + Message: fmt.Sprintf("invalid min constraint %q: must be a valid integer", tags.min)}) + } + } + if tags.max != "" { + if _, err := strconv.Atoi(tags.max); err != nil { + errors = append(errors, FieldError{FieldPath: fieldPath, Code: ErrCodeInvalidTag, + Message: fmt.Sprintf("invalid max constraint %q: must be a valid integer", tags.max)}) + } + } + } + + return errors +} + // validateIntMinMax validates min/max constraints for signed integer types. func validateIntMinMax(fieldValue reflect.Value, fieldPath string, tags tagConfig) []FieldError { var errors []FieldError diff --git a/validate_test.go b/validate_test.go index 258e291..b096285 100644 --- a/validate_test.go +++ b/validate_test.go @@ -491,3 +491,109 @@ func TestIsZeroValue(t *testing.T) { }) } } + +func TestValidateField_InvalidMinMax(t *testing.T) { + tests := []struct { + name string + value any + tags tagConfig + }{ + { + name: "int with non-numeric min", + value: 5, + tags: tagConfig{min: "abc"}, + }, + { + name: "int with non-numeric max", + value: 5, + tags: tagConfig{max: "xyz"}, + }, + { + name: "uint with non-numeric min", + value: uint(5), + tags: tagConfig{min: "abc"}, + }, + { + name: "uint with negative min", + value: uint(5), + tags: tagConfig{min: "-1"}, + }, + { + name: "float with non-numeric min", + value: 5.0, + tags: tagConfig{min: "abc"}, + }, + { + name: "float with non-numeric max", + value: 5.0, + tags: tagConfig{max: "abc"}, + }, + { + name: "string with non-numeric min length", + value: "hello", + tags: tagConfig{min: "abc"}, + }, + { + name: "string with non-numeric max length", + value: "hello", + tags: tagConfig{max: "abc"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fieldValue := reflect.ValueOf(tt.value) + errors := validateField(fieldValue, "TestField", tt.tags) + + if len(errors) == 0 { + t.Fatal("expected invalid_tag error, got none") + } + if errors[0].Code != ErrCodeInvalidTag { + t.Errorf("expected error code %q, got %q", ErrCodeInvalidTag, errors[0].Code) + } + }) + } +} + +func TestValidateField_ParseErrors(t *testing.T) { + tags := tagConfig{ + parseErrors: []string{`unknown directive "foo"`}, + } + fieldValue := reflect.ValueOf("hello") + errors := validateField(fieldValue, "TestField", tags) + + if len(errors) != 1 { + t.Fatalf("expected 1 error, got %d: %v", len(errors), errors) + } + if errors[0].Code != ErrCodeInvalidTag { + t.Errorf("expected error code %q, got %q", ErrCodeInvalidTag, errors[0].Code) + } + if errors[0].Message != `unknown directive "foo"` { + t.Errorf("unexpected message: %s", errors[0].Message) + } +} + +func TestValidateField_InvalidMinMax_EarlyReturnPath(t *testing.T) { + // Malformed min/max must be reported even when value validation is skipped + // (e.g., required field with zero value triggers early return). + tags := tagConfig{required: true, min: "abc"} + fieldValue := reflect.ValueOf(0) + errors := validateFieldWithPresence(fieldValue, "TestField", tags, nil) + + hasRequired := false + hasInvalidTag := false + for _, e := range errors { + if e.Code == ErrCodeRequired { + hasRequired = true + } + if e.Code == ErrCodeInvalidTag { + hasInvalidTag = true + } + } + if !hasRequired { + t.Error("expected required error") + } + if !hasInvalidTag { + t.Error("expected invalid_tag error for malformed min, even on early return") + } +}