refactor: surface errors for malformed conf struct tag directives#53
Closed
asadijabar wants to merge 1 commit intomainfrom
Closed
refactor: surface errors for malformed conf struct tag directives#53asadijabar wants to merge 1 commit intomainfrom
asadijabar wants to merge 1 commit intomainfrom
Conversation
asadijabar
force-pushed
the
refactor/fix-silent-tag-misconfiguration
branch
from
7ebf6d9 to
39d0ca8
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves developer feedback by surfacing malformed conf struct tag directives as validation errors (instead of silently ignoring them), introducing a new invalid_tag error code and propagating tag parse errors into field validation.
Changes:
- Added
ErrCodeInvalidTag("invalid_tag") for malformed/unrecognized tag directives. - Extended tag parsing to collect
parseErrorsfor invalidrequired/secretvalues and unknown directives, and surfaced them during validation. - Updated/added tests to assert
parseErrorsbehavior andinvalid_tagerrors for unparseablemin/maxconstraints.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
errors.go |
Introduces the invalid_tag error code for malformed tag directives. |
binding.go |
Adds parseErrors to tagConfig and records unknown/invalid directives during tag parsing (with cache-safe cloning). |
validate.go |
Emits invalid_tag errors for tag parse errors and for unparseable min/max constraints in type-specific validators. |
binding_test.go |
Updates expectations to include parseErrors and asserts them in the test loop. |
validate_test.go |
Adds coverage for invalid min/max parsing and for surfacing parseErrors as validation errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+23
to
+30
| // Surface tag parse errors (malformed directives, unknown directives, etc.) | ||
| for _, msg := range tags.parseErrors { | ||
| errors = append(errors, FieldError{ | ||
| FieldPath: fieldPath, | ||
| Code: ErrCodeInvalidTag, | ||
| Message: msg, | ||
| }) | ||
| } |
asadijabar
added a commit
that referenced
this pull request
Mar 19, 2026
Move min/max tag parseability checks before required/zero-value early returns in validateFieldWithPresence, so malformed constraints are always surfaced. The validateXxxMinMax functions now only do value comparison (parse errors are caught upfront by validateMinMaxParseable). Addresses code review feedback from PR #53. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
asadijabar
added a commit
that referenced
this pull request
Mar 19, 2026
Move min/max tag parseability checks before required/zero-value early returns in validateFieldWithPresence, so malformed constraints are always surfaced. The validateXxxMinMax functions now only do value comparison (parse errors are caught upfront by validateMinMaxParseable). Addresses code review feedback from PR #53.
asadijabar
force-pushed
the
refactor/fix-silent-tag-misconfiguration
branch
from
eac2a9f to
67992f3
Compare
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. Malformed min/max constraints are checked upfront (before early returns) so they are reported even when value validation is skipped.
asadijabar
force-pushed
the
refactor/fix-silent-tag-misconfiguration
branch
from
67992f3 to
afd5866
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
confstruct tag directives (e.g.,conf:"min:abc",conf:"required:yes",conf:"envv:VAR") were previously silently ignored, causing constraints to be bypassed without any feedback to the developerErrCodeInvalidTagand surfaces these asFieldErrorvalues during validationparseErrorsfield totagConfigto accumulate tag-level errors fromparseTagand propagate them throughvalidateFieldWithPresenceChanges
errors.go: AddErrCodeInvalidTag = "invalid_tag"error codebinding.go: AddparseErrorstotagConfig; report invalidrequired/secretvalues and unknown directivesvalidate.go: SurfaceparseErrorsduring validation; returninvalid_tagerrors for unparseablemin/maxin all fourvalidateXxxMinMaxfunctionsbinding_test.go: Update existing tests to expectparseErrors; addparseErrorsassertion to test loopvalidate_test.go: AddTestValidateField_InvalidMinMax(8 cases) andTestValidateField_ParseErrorsTest plan
go test -race ./...— all passmake vet/make fmt— clean