Skip to content

Commit 39d0ca8

Browse files
asadijabarasadijabar
committed
refactor: surface errors for malformed conf struct tag directives
Previously, malformed tag directives were silently ignored: - `conf:"min:abc"` on an int field silently skipped the min constraint - `conf:"required:yes"` silently defaulted to true - `conf:"envv:VAR"` (typo) was silently dropped This change introduces ErrCodeInvalidTag and surfaces these as FieldError values during validation, so developers get clear feedback about misconfigured struct tags instead of silent constraint bypass.
1 parent ad8866f commit 39d0ca8

5 files changed

Lines changed: 202 additions & 38 deletions

File tree

‎binding.go‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,33 @@ import (
1414

1515
// tagConfig holds parsed directives from a struct field's `conf` tag.
1616
type tagConfig struct {
17-
env string // Environment variable name (env:VAR_NAME)
18-
name string // Custom key path (name:custom.path)
19-
prefix string // Prefix for nested structs (prefix:foo)
20-
defValue string // Default value (default:value)
21-
min string // Minimum constraint (min:N)
22-
max string // Maximum constraint (max:M)
23-
oneof []string // Allowed values (oneof:a,b,c)
24-
required bool // Field is required (required or required:true)
25-
secret bool // Field is secret (secret or secret:true)
26-
hasDefault bool // Whether a default directive was present
17+
env string // Environment variable name (env:VAR_NAME)
18+
name string // Custom key path (name:custom.path)
19+
prefix string // Prefix for nested structs (prefix:foo)
20+
defValue string // Default value (default:value)
21+
min string // Minimum constraint (min:N)
22+
max string // Maximum constraint (max:M)
23+
oneof []string // Allowed values (oneof:a,b,c)
24+
required bool // Field is required (required or required:true)
25+
secret bool // Field is secret (secret or secret:true)
26+
hasDefault bool // Whether a default directive was present
27+
parseErrors []string // Errors from malformed tag directives
2728
}
2829

2930
var tagConfigCache sync.Map
3031
var optionalTypePkgPath = reflect.TypeOf(Optional[int]{}).PkgPath()
3132

3233
func cloneTagConfig(cfg tagConfig) tagConfig {
33-
if len(cfg.oneof) == 0 {
34+
if len(cfg.oneof) == 0 && len(cfg.parseErrors) == 0 {
3435
return cfg
3536
}
3637

37-
cfg.oneof = append([]string(nil), cfg.oneof...)
38+
if len(cfg.oneof) > 0 {
39+
cfg.oneof = append([]string(nil), cfg.oneof...)
40+
}
41+
if len(cfg.parseErrors) > 0 {
42+
cfg.parseErrors = append([]string(nil), cfg.parseErrors...)
43+
}
3844
return cfg
3945
}
4046

@@ -111,8 +117,9 @@ func parseTag(tag string) tagConfig {
111117
} else if value == "false" {
112118
cfg.required = false
113119
} else {
114-
// Invalid value, default to true for safety
115120
cfg.required = true
121+
cfg.parseErrors = append(cfg.parseErrors,
122+
fmt.Sprintf("invalid required value %q: use true or false", value))
116123
}
117124
case "secret":
118125
// No value or explicit "true" means true
@@ -121,9 +128,13 @@ func parseTag(tag string) tagConfig {
121128
} else if value == "false" {
122129
cfg.secret = false
123130
} else {
124-
// Invalid value, default to true for safety
125131
cfg.secret = true
132+
cfg.parseErrors = append(cfg.parseErrors,
133+
fmt.Sprintf("invalid secret value %q: use true or false", value))
126134
}
135+
default:
136+
cfg.parseErrors = append(cfg.parseErrors,
137+
fmt.Sprintf("unknown directive %q", name))
127138
}
128139
}
129140

‎binding_test.go‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,9 @@ func TestBinding_ParseTag(t *testing.T) {
116116
name: "default with comma terminates directive",
117117
tag: "default:a,b,c",
118118
expected: tagConfig{
119-
defValue: "a",
120-
hasDefault: true,
119+
defValue: "a",
120+
hasDefault: true,
121+
parseErrors: []string{`unknown directive "b"`, `unknown directive "c"`},
121122
},
122123
},
123124
{
@@ -364,14 +365,16 @@ func TestBinding_ParseTag(t *testing.T) {
364365
name: "required with invalid value defaults to true",
365366
tag: "required:invalid",
366367
expected: tagConfig{
367-
required: true,
368+
required: true,
369+
parseErrors: []string{`invalid required value "invalid": use true or false`},
368370
},
369371
},
370372
{
371373
name: "required with numeric value defaults to true",
372374
tag: "required:1",
373375
expected: tagConfig{
374-
required: true,
376+
required: true,
377+
parseErrors: []string{`invalid required value "1": use true or false`},
375378
},
376379
},
377380
{
@@ -399,14 +402,16 @@ func TestBinding_ParseTag(t *testing.T) {
399402
name: "secret with invalid value defaults to true",
400403
tag: "secret:invalid",
401404
expected: tagConfig{
402-
secret: true,
405+
secret: true,
406+
parseErrors: []string{`invalid secret value "invalid": use true or false`},
403407
},
404408
},
405409
{
406410
name: "secret with yes defaults to true",
407411
tag: "secret:yes",
408412
expected: tagConfig{
409-
secret: true,
413+
secret: true,
414+
parseErrors: []string{`invalid secret value "yes": use true or false`},
410415
},
411416
},
412417
{
@@ -574,29 +579,35 @@ func TestBinding_ParseTag(t *testing.T) {
574579

575580
// Unknown directives
576581
{
577-
name: "unknown directive ignored",
582+
name: "unknown directive reported",
578583
tag: "unknown:value,env:VAR",
579584
expected: tagConfig{
580-
env: "VAR",
585+
env: "VAR",
586+
parseErrors: []string{`unknown directive "unknown"`},
581587
},
582588
},
583589
{
584590
name: "multiple unknown directives",
585591
tag: "foo:bar,env:VAR,baz:qux,required",
586592
expected: tagConfig{
587-
env: "VAR",
588-
required: true,
593+
env: "VAR",
594+
required: true,
595+
parseErrors: []string{`unknown directive "foo"`, `unknown directive "baz"`},
589596
},
590597
},
591598
{
592-
name: "only unknown directives",
593-
tag: "unknown:value,another:thing",
594-
expected: tagConfig{},
599+
name: "only unknown directives",
600+
tag: "unknown:value,another:thing",
601+
expected: tagConfig{
602+
parseErrors: []string{`unknown directive "unknown"`, `unknown directive "another"`},
603+
},
595604
},
596605
{
597-
name: "typo in directive name",
598-
tag: "envv:VAR,requiired:true", // intentional typos to test silent ignore
599-
expected: tagConfig{},
606+
name: "typo in directive name",
607+
tag: "envv:VAR,requiired:true",
608+
expected: tagConfig{
609+
parseErrors: []string{`unknown directive "envv"`, `unknown directive "requiired"`},
610+
},
600611
},
601612

602613
// Edge cases
@@ -655,6 +666,9 @@ func TestBinding_ParseTag(t *testing.T) {
655666
if result.secret != tt.expected.secret {
656667
t.Errorf("secret: got %v, want %v", result.secret, tt.expected.secret)
657668
}
669+
if !reflect.DeepEqual(result.parseErrors, tt.expected.parseErrors) {
670+
t.Errorf("parseErrors: got %v, want %v", result.parseErrors, tt.expected.parseErrors)
671+
}
658672
})
659673
}
660674
}

‎errors.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const (
1313
ErrCodeOneOf = "oneof" // Value is not in the allowed set
1414
ErrCodeInvalidType = "invalid_type" // Type conversion failed
1515
ErrCodeUnknownKey = "unknown_key" // Configuration key doesn't map to any field (strict mode)
16+
ErrCodeInvalidTag = "invalid_tag" // Struct tag directive is malformed or unrecognized
1617
)
1718

1819
// ValidationError aggregates field-level validation failures.

‎validate.go‎

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@ func validateFieldWithPresence(fieldValue reflect.Value, fieldPath string, tags
2020
var errors []FieldError
2121
fieldPresent := presentFields != nil && presentFields[fieldPath]
2222

23+
// Surface tag parse errors (malformed directives, unknown directives, etc.)
24+
for _, msg := range tags.parseErrors {
25+
errors = append(errors, FieldError{
26+
FieldPath: fieldPath,
27+
Code: ErrCodeInvalidTag,
28+
Message: msg,
29+
})
30+
}
31+
2332
// Check required constraint
2433
if tags.required {
2534
if presentFields != nil {
@@ -194,7 +203,13 @@ func validateIntMinMax(fieldValue reflect.Value, fieldPath string, tags tagConfi
194203

195204
if tags.min != "" {
196205
minVal, err := strconv.ParseInt(tags.min, 10, 64)
197-
if err == nil && value < minVal {
206+
if err != nil {
207+
errors = append(errors, FieldError{
208+
FieldPath: fieldPath,
209+
Code: ErrCodeInvalidTag,
210+
Message: fmt.Sprintf("invalid min constraint %q: must be a valid integer", tags.min),
211+
})
212+
} else if value < minVal {
198213
errors = append(errors, FieldError{
199214
FieldPath: fieldPath,
200215
Code: ErrCodeMin,
@@ -205,7 +220,13 @@ func validateIntMinMax(fieldValue reflect.Value, fieldPath string, tags tagConfi
205220

206221
if tags.max != "" {
207222
maxVal, err := strconv.ParseInt(tags.max, 10, 64)
208-
if err == nil && value > maxVal {
223+
if err != nil {
224+
errors = append(errors, FieldError{
225+
FieldPath: fieldPath,
226+
Code: ErrCodeInvalidTag,
227+
Message: fmt.Sprintf("invalid max constraint %q: must be a valid integer", tags.max),
228+
})
229+
} else if value > maxVal {
209230
errors = append(errors, FieldError{
210231
FieldPath: fieldPath,
211232
Code: ErrCodeMax,
@@ -224,7 +245,13 @@ func validateUintMinMax(fieldValue reflect.Value, fieldPath string, tags tagConf
224245

225246
if tags.min != "" {
226247
minVal, err := strconv.ParseUint(tags.min, 10, 64)
227-
if err == nil && value < minVal {
248+
if err != nil {
249+
errors = append(errors, FieldError{
250+
FieldPath: fieldPath,
251+
Code: ErrCodeInvalidTag,
252+
Message: fmt.Sprintf("invalid min constraint %q: must be a valid unsigned integer", tags.min),
253+
})
254+
} else if value < minVal {
228255
errors = append(errors, FieldError{
229256
FieldPath: fieldPath,
230257
Code: ErrCodeMin,
@@ -235,7 +262,13 @@ func validateUintMinMax(fieldValue reflect.Value, fieldPath string, tags tagConf
235262

236263
if tags.max != "" {
237264
maxVal, err := strconv.ParseUint(tags.max, 10, 64)
238-
if err == nil && value > maxVal {
265+
if err != nil {
266+
errors = append(errors, FieldError{
267+
FieldPath: fieldPath,
268+
Code: ErrCodeInvalidTag,
269+
Message: fmt.Sprintf("invalid max constraint %q: must be a valid unsigned integer", tags.max),
270+
})
271+
} else if value > maxVal {
239272
errors = append(errors, FieldError{
240273
FieldPath: fieldPath,
241274
Code: ErrCodeMax,
@@ -254,7 +287,13 @@ func validateFloatMinMax(fieldValue reflect.Value, fieldPath string, tags tagCon
254287

255288
if tags.min != "" {
256289
minVal, err := strconv.ParseFloat(tags.min, 64)
257-
if err == nil && value < minVal {
290+
if err != nil {
291+
errors = append(errors, FieldError{
292+
FieldPath: fieldPath,
293+
Code: ErrCodeInvalidTag,
294+
Message: fmt.Sprintf("invalid min constraint %q: must be a valid number", tags.min),
295+
})
296+
} else if value < minVal {
258297
errors = append(errors, FieldError{
259298
FieldPath: fieldPath,
260299
Code: ErrCodeMin,
@@ -265,7 +304,13 @@ func validateFloatMinMax(fieldValue reflect.Value, fieldPath string, tags tagCon
265304

266305
if tags.max != "" {
267306
maxVal, err := strconv.ParseFloat(tags.max, 64)
268-
if err == nil && value > maxVal {
307+
if err != nil {
308+
errors = append(errors, FieldError{
309+
FieldPath: fieldPath,
310+
Code: ErrCodeInvalidTag,
311+
Message: fmt.Sprintf("invalid max constraint %q: must be a valid number", tags.max),
312+
})
313+
} else if value > maxVal {
269314
errors = append(errors, FieldError{
270315
FieldPath: fieldPath,
271316
Code: ErrCodeMax,
@@ -285,7 +330,13 @@ func validateStringMinMax(fieldValue reflect.Value, fieldPath string, tags tagCo
285330

286331
if tags.min != "" {
287332
minLen, err := strconv.Atoi(tags.min)
288-
if err == nil && length < minLen {
333+
if err != nil {
334+
errors = append(errors, FieldError{
335+
FieldPath: fieldPath,
336+
Code: ErrCodeInvalidTag,
337+
Message: fmt.Sprintf("invalid min constraint %q: must be a valid integer", tags.min),
338+
})
339+
} else if length < minLen {
289340
errors = append(errors, FieldError{
290341
FieldPath: fieldPath,
291342
Code: ErrCodeMin,
@@ -296,7 +347,13 @@ func validateStringMinMax(fieldValue reflect.Value, fieldPath string, tags tagCo
296347

297348
if tags.max != "" {
298349
maxLen, err := strconv.Atoi(tags.max)
299-
if err == nil && length > maxLen {
350+
if err != nil {
351+
errors = append(errors, FieldError{
352+
FieldPath: fieldPath,
353+
Code: ErrCodeInvalidTag,
354+
Message: fmt.Sprintf("invalid max constraint %q: must be a valid integer", tags.max),
355+
})
356+
} else if length > maxLen {
300357
errors = append(errors, FieldError{
301358
FieldPath: fieldPath,
302359
Code: ErrCodeMax,

0 commit comments

Comments
 (0)