-
Notifications
You must be signed in to change notification settings - Fork 16
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
refactor!: renamed ZogError to ZogIssue to be more aligned with Zod. Deprecated a bunch of APIs for naming consistency. conf.ErrorFormatter removed in favor of conf.IssueFormatter #86
Conversation
WalkthroughThis pull request refactors the error handling mechanism by replacing all references to "errors" with "issues." Method return types, interface definitions, constants, and formatter functions have been updated from ZogError and related types to their corresponding ZogIssue variants. Additionally, tests and documentation have been revised to reflect the new terminology and verification strategy using helper functions. Configuration, internationalization, and internal packages have also been adjusted for consistency in issue reporting. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Schema
participant Formatter
Caller->>Schema: Call Parse/Validate method
Schema->>Formatter: Create context using IssueFormatter
Formatter-->>Schema: Format and return issue message
Schema->>Caller: Return ZogIssueList/ZogIssueMap with issues
Poem
✨ 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 (
|
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.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
internals/DataProviders.go (1)
65-65
: 🛠️ Refactor suggestionUpdate return type to match new terminology.
The function signature should be updated to return
ZogIssue
instead ofZogError
to maintain consistency with the new terminology.-func TryNewAnyDataProvider(val any) (DataProvider, ZogError) { +func TryNewAnyDataProvider(val any) (DataProvider, ZogIssue) {
🧹 Nitpick comments (34)
internals/Issues.go (4)
9-20
: Unify "error" vs. "issue" terminology in doc comments.The interface uses "ZogIssue," but doc comments still say "returns the error code for the error." Consider updating the text to consistently refer to "issue code" or "issue" for clarity.
-// returns the error code for the error. This is a unique identifier for the Issue. +// returns the issue code for the issue. This is a unique identifier for the Issue.
70-72
: Consider updating the comment referencing the interface name.Currently, the comment still states "Error interface returned from all processors," but the alias is
type ZogError = ZogIssue
. For consistency, rename it to something like "Issue interface returned from all processors."-// Error interface returned from all processors +// Issue interface returned from all processors
74-74
: Consider using ZogIssue as the parameter type.
IssueFmtFunc
currently receivesZogError
, which is an alias forZogIssue
. If backward compatibility is not critical, you could simplify the signature tofunc(e ZogIssue, p Ctx)
to reduce confusion.
78-84
: Rename "ZogError" in the String() output to maintain consistency.The struct is now
ZogErr
implementing aZogIssue
, but theString()
method still labels it asZogError{...}
. Consider updating the string literal toZogIssue{...}
for clarity.-return fmt.Sprintf("ZogError{Code: %v, Params: %v, Type: %v, Value: %v, Message: '%v', Error: %v}", +return fmt.Sprintf("ZogIssue{Code: %v, Params: %v, Type: %v, Value: %v, Message: '%v', Error: %v}",internals/types.go (1)
5-5
: Correct the minor typo in the documentation.“wraped” should be spelled “wrapped.”
-// If its an error it will be wraped inside a ZogIssue +// If its an error it will be wrapped inside a ZogIssuetutils/testIssueMessages.go (1)
14-32
: Consider improving error handling and string manipulation.
- Replace panic with a more graceful error handling mechanism.
- Optimize string manipulation by pre-compiling the message format.
func VerifyDefaultIssueMessages(t *testing.T, errs internals.ZogIssueList) { for _, err := range errs { c := err.Code() m, ok := en.Map[err.Dtype()][c] if !ok { m, ok = en.Map[err.Dtype()][zconst.IssueCodeFallback] if !ok { - panic(fmt.Sprintf("no fallback message for type %s", err.Dtype())) + t.Errorf("no fallback message for type %s", err.Dtype()) + continue } } + // Pre-compile message parts for better performance + parts := strings.Split(m, "{{") + prefix := parts[0] + postfix := prefix + if len(parts) > 1 && strings.Contains(parts[1], "}}") { + postfix = strings.Split(parts[1], "}}")[1] + } - prefix := strings.Split(m, "{{")[0] - postfix := prefix - if strings.Contains(m, "}}") { - postfix = strings.Split(m, "}}")[1] - } assert.True(t, strings.HasPrefix(err.Message(), prefix)) assert.True(t, strings.HasSuffix(err.Message(), postfix)) } }parsers/zjson/parseJson.go (2)
12-24
: Improve function documentation and error messages.The commented-out code should either be removed or properly documented with a reason for keeping it.
-// func Unmarshal(data []byte) p.DpFactory { +// Unmarshal is deprecated in favor of Decode. +// Kept for reference during migration. +// func Unmarshal(data []byte) p.DpFactory {Also, consider making the error message more descriptive:
-// return nil, &p.ZogErr{C: zconst.IssueCodeInvalidJSON, Err: errors.New("nill json body")} +// return nil, &p.ZogErr{C: zconst.IssueCodeInvalidJSON, Err: errors.New("JSON body is null or empty")}
35-52
: Improve error messages for better debugging.Consider making the error messages more descriptive and consistent with the documentation.
- return nil, &p.ZogErr{C: zconst.IssueCodeInvalidJSON, Err: errors.New("nill json body")} + return nil, &p.ZogErr{C: zconst.IssueCodeInvalidJSON, Err: errors.New("JSON body is null or empty, expected a non-empty object")}i18n/i18n.go (1)
14-19
: Update function documentation to reflect new terminology.The function documentation still refers to "ErrsMap" in the name and examples, which should be updated to maintain consistency with the new "issue" terminology.
-// Takes a map[langKey]conf.LangMap -// usage is i18n.SetLanguagesErrsMap(map[string]zconst.LangMap{ +// Takes a map[langKey]conf.LangMap +// usage is i18n.SetLanguagesIssueMap(map[string]zconst.LangMap{utilsOptions.go (1)
28-36
: Enhance IssueCode documentation with more examples.While the documentation includes a basic example, it would be helpful to add more context about when to use custom codes versus predefined ones from zconst.
// IssueCode is a function that allows you to set a custom issue code for the test. Most useful for TestFuncs: /* -z.String().TestFunc(..., z.IssueCode("just_provide_a_string" or use values in zconst)) +Example usage: +1. Using predefined codes: + z.String().TestFunc(..., z.IssueCode(zconst.IssueCodeInvalid)) + +2. Using custom codes: + z.String().TestFunc(..., z.IssueCode("custom_validation_failed")) */zhttp/zhttp.go (1)
92-94
: Remove commented-out code.The commented-out
params
function should be removed if it's no longer needed.-// func params(data url.Values) p.DataProvider { -// return form(data) -// }conf/issueFormatConf.go (1)
35-40
: Optimize string operations using strings.Builder.The current implementation using multiple
strings.ReplaceAll
calls could be optimized usingstrings.Builder
for better performance with multiple parameter substitutions.+ var builder strings.Builder + lastIndex := 0 for k, v := range e.Params() { - msg = strings.ReplaceAll(msg, "{{"+k+"}}", fmt.Sprintf("%v", v)) + placeholder := "{{" + k + "}}" + if idx := strings.Index(msg[lastIndex:], placeholder); idx != -1 { + builder.WriteString(msg[lastIndex:lastIndex+idx]) + builder.WriteString(fmt.Sprintf("%v", v)) + lastIndex = lastIndex + idx + len(placeholder) + } } - msg = strings.ReplaceAll(msg, "{{value}}", fmt.Sprintf("%v", e.Value())) + if lastIndex < len(msg) { + builder.WriteString(msg[lastIndex:]) + } - e.SetMessage(msg) + e.SetMessage(builder.String())pointers.go (2)
41-41
: Update variable name to match new terminology.The variable name
errs
should be updated toissues
to maintain consistency with the new terminology.- errs := p.NewErrsMap() + issues := p.NewErrsMap()
77-77
: Update variable name to match new terminology.The variable name
errs
should be updated toissues
to maintain consistency with the new terminology.- errs := p.NewErrsMap() + issues := p.NewErrsMap()i18n/i18n_test.go (2)
23-23
: Update test function names to match new terminology.The test function names should be updated to use "issue" instead of "error" terminology.
-func TestSetLanguagesErrsMap(t *testing.T) { +func TestSetLanguagesIssuesMap(t *testing.T) { -func TestLangErrsMapWithLangKey(t *testing.T) { +func TestLangIssuesMapWithLangKey(t *testing.T) {Also applies to: 109-109
41-41
: Update test case field names to match new terminology.The test case struct field names should be updated to use "issue" instead of "error" terminology.
- expectedErr bool - expected string + expectedIssue bool + expected stringAlso applies to: 42-42
pointers_validate_test.go (1)
99-111
: Consider adding test cases for edge cases.While the test is valid, consider adding test cases for:
- Empty slices
- Slices with all nil values
- Large slices to verify performance
internals/contexts.go (2)
9-11
: Update deprecated notice to use new terminology.The deprecated notice still uses "error" terminology. Update it to align with the new "issue" terminology.
- // Deprecated: Use Ctx.AddIssue() instead - // Please don't depend on this interface it may change + // Deprecated: Use Ctx.AddIssue() instead + // Please don't depend on this interface as it will be removed in a future version
104-110
: Remove TODO comment or create an issue.The TODO comment about handling catch should be addressed or tracked in an issue.
Would you like me to create an issue to track this TODO item?
utils.go (2)
18-67
: LGTM! Clear deprecation notices and documentation.The type declarations and deprecation notices are well-documented, providing clear migration paths from error-based to issue-based types. The examples in the documentation are particularly helpful.
Consider adding deprecation timelines to help users plan their migrations:
-// Deprecated: This will be removed in the future. Use z.ZogIssue instead +// Deprecated: This will be removed in v2.0.0. Use z.ZogIssue instead
107-162
: LGTM! Clean deprecation path with backward compatibility.The error helpers maintain backward compatibility while correctly delegating to the new Issues struct. The deprecation notices are clear and helpful.
Consider adding a migration guide link in the deprecation notices:
-// Deprecated: This will be removed in the future. +// Deprecated: This will be removed in the future. See https://zog.dev/migration for guidance.zconst/consts.go (1)
7-22
: Update example documentation to use "issue" terminology.The example in the comment block still uses "error" terminology (
restOfErrorFields
). For consistency with the new naming convention, update it to use "issue" terminology.- restOfErrorFields.. + restOfIssueFields..slices.go (1)
325-326
: Consider adding issue code constants.The issue codes are used directly. Consider extracting them into constants for better maintainability and to avoid magic strings.
+const ( + IssueCodeContains = "contains" + IssueCodeHasPrefix = "hasPrefix" + IssueCodeMatch = "match" +)Also applies to: 368-369, 414-415
string.go (1)
215-216
: Consider grouping related issue codes.Multiple validation methods define similar issue codes. Consider grouping them into a dedicated constants file or package.
+// zconst/string_issues.go +const ( + IssueCodeEmail = "email" + IssueCodeURL = "url" + IssueCodeHasPrefix = "hasPrefix" + IssueCodeHasSuffix = "hasSuffix" + // ... other string-related issue codes +)Also applies to: 234-235, 254-255, 275-276, 296-297, 317-318, 341-342, 368-369, 395-396, 414-415
boolean_test.go (1)
48-50
: Consider extracting common test verification logic.The same error verification pattern is repeated across multiple test cases. Consider extracting it into a helper function.
+func verifyTestErrors(t *testing.T, errs []p.ZogIssue, expectErr bool, name string) { + if (len(errs) > 0) != expectErr { + t.Errorf("%s -> Expected error: %v, got: %v", name, expectErr, errs) + } + if len(errs) > 0 { + tutils.VerifyDefaultIssueMessages(t, errs) + } +}Also applies to: 176-178, 231-233, 288-290, 330-332, 372-374, 422-424, 481-483
docs/docs/core-design-decisions.md (1)
13-13
: Fix grammatical issues in the sentence.The sentence has a word repetition ("is is") and could use better punctuation.
-Errors returned by you (for example in a `PreTransform` or `PostTransform` function) can be the ZogIssue interface or an error. If you return an error, it will be wrapped in a ZogIssue. ZogIssue is just a struct that wraps around an error and adds a message field which is is text that can be shown to the user. For more on this see [Errors](/errors) +Errors returned by you (for example in a `PreTransform` or `PostTransform` function) can be the ZogIssue interface or an error. If you return an error, it will be wrapped in a ZogIssue. ZogIssue is just a struct that wraps around an error and adds a message field which is text that can be shown to the user. For more on this, see [Errors](/errors).🧰 Tools
🪛 LanguageTool
[uncategorized] ~13-~13: Possible missing comma found.
Context: ...raps around an error and adds a message field which is is text that can be shown to t...(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~13-~13: Possible typo: you repeated a word.
Context: ...an error and adds a message field which is is text that can be shown to the user. For...(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~13-~13: Possible missing comma found.
Context: ...t can be shown to the user. For more on this see Errors - You should not ...(AI_HYDRA_LEO_MISSING_COMMA)
docs/docs/core-concepts/3-parsing.mdx (1)
50-51
: Improve sentence structure and punctuation.Several sentences could be improved for clarity and completeness.
- - On error all parsing and validation stops and [ZogIssues](/errors) are returned. + - On error, all parsing and validation stops, and [ZogIssues](/errors) are returned. - - On error all parsing and validation stops and [ZogIssues](/errors) are returned + - On error, all parsing and validation stops, and [ZogIssues](/errors) are returned. - - On error: aborts if the value is its nil value and returns a required [ZogIssue](/errors). + - On error: The process aborts if the value is its nil value and returns a required [ZogIssue](/errors). - - On error: validation [ZogIssues](/errors) are added to the [ZogIssueList](/errors#zogissuelist) or [ZogIssueMap](/errors#zogissuelist). All validation functions are run even if one of them fails. + - On error: Validation [ZogIssues](/errors) are added to the [ZogIssueList](/errors#zogissuelist) or [ZogIssueMap](/errors#zogissuelist). All validation functions are run even if one of them fails. - - On error you return: aborts and adds your error to the list of [ZogIssues](/errors) - - Only run on valid values. Won't run if a [ZogIssue](/errors) was created before the postTransforms + - On error you return: The process aborts and adds your error to the list of [ZogIssues](/errors). + - Only runs on valid values. Won't run if a [ZogIssue](/errors) was created before the postTransforms.Also applies to: 55-56, 58-59, 61-62, 64-66
🧰 Tools
🪛 LanguageTool
[style] ~51-~51: To form a complete sentence, be sure to include a subject.
Context: ...ZogIssues are returned. - Can be caught by catch 2. Default Check -> ...(MISSING_IT_THERE)
docs/docs/core-concepts/1-anatomy-of-schema.md (2)
51-51
: Add a comma for better readability.Add a comma after "required" to improve readability in the compound sentence.
-`schema.Required()` is a boolean that indicates if the field is required. If it is required and the data is a zero value the schema will return a [ZogIssue](/errors). +`schema.Required()` is a boolean that indicates if the field is required. If it is required and the data is a zero value, the schema will return a [ZogIssue](/errors).🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...f it is required and the data is a zero value the schema will return a [ZogIssue](/er...(AI_HYDRA_LEO_MISSING_COMMA)
55-55
: Add commas for better readability.Add commas to improve readability in the compound sentence.
-`schema.Catch(value)` sets a catch value. If this is set it will "catch" any errors or ZogIssues with the catch value. Meaning it will set the destination value to the catch value and exit. When this is triggered, no matter what error triggers it code will automatically jump to the [PostTransforms](#posttransforms). +`schema.Catch(value)` sets a catch value. If this is set, it will "catch" any errors or ZogIssues with the catch value. Meaning it will set the destination value to the catch value and exit. When this is triggered, no matter what error triggers it, code will automatically jump to the [PostTransforms](#posttransforms).🧰 Tools
🪛 LanguageTool
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...rors or ZogIssues with the catch value. Meaning it will set the destination value to th...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...riggered, no matter what error triggers it code will automatically jump to the [Po...(AI_HYDRA_LEO_MISSING_COMMA)
docs/docs/errors.md (1)
9-9
: Fix grammar and formatting issues.Several minor grammar and formatting improvements are needed:
-Since Zog does not throw issues we use the term Issues to keep consistency with Zod. +Since Zog does not throw issues, we use the term Issues to keep consistency with Zod. -The `ZogIssue` is actually an interface which also implements the issue interface so it can be used with the `issues` package. +The `ZogIssue` is actually an interface which also implements the issue interface, so it can be used with the `issues` package. -Beware you must handle all `ZogIssue` codes & types or you may get unexpected messages. +Beware, you must handle all `ZogIssue` codes & types, or you may get unexpected messages. -Really this only makes sense if you are doing i18n. Please please check out the [i18n](/packages/i18n) section for more information. +Really, this only makes sense if you are doing i18n. Please, please check out the [i18n](/packages/i18n) section for more information. -If you want to return issues to the user without the possibility of exposing internal confidential information (error that caused the issue, the path, params, etc) +If you want to return issues to the user without the possibility of exposing internal confidential information (error that caused the issue, the path, params, etc.)Also applies to: 51-51, 165-165, 216-217, 221-221
🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: Possible missing comma found.
Context: ...idation tests. Since Zog does not throw issues we use the term Issues to keep consiste...(AI_HYDRA_LEO_MISSING_COMMA)
docs/docs/core-concepts/6-validation-results.mdx (1)
7-7
: Add comma for better readability.Add a comma after the introductory phrase.
-Under the hood Zog follows the [Parsing Execution Structure](/core-concepts/parsing/#parsing-execution-structure) and does a bunch of things under the hood to make sure your data is validated correctly. +Under the hood, Zog follows the [Parsing Execution Structure](/core-concepts/parsing/#parsing-execution-structure) and does a bunch of things under the hood to make sure your data is validated correctly.🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: Possible missing comma found.
Context: ... Validation Results Examples Under the hood Zog follows the [Parsing Execution Stru...(AI_HYDRA_LEO_MISSING_COMMA)
docs/docs/configuration.md (1)
35-35
: Add comma for better readability.Add a comma after "globally".
-For information on configurating error formatting globally please refer to the [errors page](/errors#5-configure-issue-messages-globally). +For information on configurating error formatting globally, please refer to the [errors page](/errors#5-configure-issue-messages-globally).🧰 Tools
🪛 LanguageTool
[typographical] ~35-~35: Consider adding a comma here.
Context: ... configurating error formatting globally please refer to the [errors page](/errors#5-co...(PLEASE_COMMA)
struct_test.go (1)
225-229
: Consider adding test cases for empty data.While the test coverage is good, consider adding test cases for empty data to ensure the issue handling works correctly in edge cases.
// Test with missing fields errs := schema.Parse(map[string]any{}, &output) assert.NotNil(t, errs) tutils.VerifyDefaultIssueMessagesMap(t, errs) + +// Test with nil data +errs = schema.Parse(nil, &output) +assert.NotNil(t, errs) +tutils.VerifyDefaultIssueMessagesMap(t, errs)string_test.go (1)
252-264
: Consider adding more test cases for string validation.While the test coverage is good, consider adding test cases for:
- Empty strings
- Strings with special characters
- Unicode strings
field := String().Contains("contains") var dest string errs := field.Parse("not containing", &dest) assert.NotEmpty(t, errs) tutils.VerifyDefaultIssueMessages(t, errs) errs = field.Parse("this contains that", &dest) assert.Empty(t, errs) assert.Equal(t, "this contains that", dest) + +// Test empty string +errs = field.Parse("", &dest) +assert.NotEmpty(t, errs) +tutils.VerifyDefaultIssueMessages(t, errs) + +// Test special characters +errs = field.Parse("this contains! that", &dest) +assert.Empty(t, errs) +assert.Equal(t, "this contains! that", dest) + +// Test Unicode +errs = field.Parse("this contains 🚀 that", &dest) +assert.Empty(t, errs) +assert.Equal(t, "this contains 🚀 that", dest)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
boolean.go
(2 hunks)boolean_test.go
(11 hunks)boolean_validate_test.go
(9 hunks)conf/Errors.go
(0 hunks)conf/issueFormatConf.go
(1 hunks)conf/issueFormatConf_test.go
(1 hunks)docs/docs/configuration.md
(1 hunks)docs/docs/context.mdx
(1 hunks)docs/docs/core-concepts/1-anatomy-of-schema.md
(4 hunks)docs/docs/core-concepts/2-parsing-vs-validation.md
(2 hunks)docs/docs/core-concepts/3-parsing.mdx
(1 hunks)docs/docs/core-concepts/5-validate.mdx
(2 hunks)docs/docs/core-concepts/6-validation-results.mdx
(1 hunks)docs/docs/core-design-decisions.md
(1 hunks)docs/docs/errors.md
(1 hunks)docs/docs/packages/zconst.md
(2 hunks)docs/docs/zog-schemas.md
(1 hunks)errors_test.go
(2 hunks)i18n/en/en.go
(1 hunks)i18n/es/es.go
(1 hunks)i18n/i18n.go
(1 hunks)i18n/i18n_test.go
(1 hunks)internals/DataProviders.go
(3 hunks)internals/Issues.go
(6 hunks)internals/contexts.go
(6 hunks)internals/tests.go
(10 hunks)internals/types.go
(1 hunks)numbers.go
(2 hunks)numbers_test.go
(1 hunks)numbers_validate_test.go
(1 hunks)parsers/zjson/parseJson.go
(2 hunks)pointers.go
(4 hunks)pointers_test.go
(4 hunks)pointers_validate_test.go
(4 hunks)slices.go
(8 hunks)slices_test.go
(5 hunks)slices_validate_test.go
(6 hunks)string.go
(16 hunks)string_test.go
(10 hunks)string_validate_test.go
(10 hunks)struct.go
(4 hunks)struct_test.go
(3 hunks)time.go
(8 hunks)tutils/testIssueMessages.go
(1 hunks)utils.go
(4 hunks)utilsOptions.go
(3 hunks)utilsOptions_test.go
(2 hunks)zconst/consts.go
(4 hunks)zhttp/zhttp.go
(1 hunks)zhttp/zhttp_test.go
(1 hunks)zogSchema.go
(1 hunks)
💤 Files with no reviewable changes (1)
- conf/Errors.go
✅ Files skipped from review due to trivial changes (2)
- docs/docs/context.mdx
- docs/docs/zog-schemas.md
🧰 Additional context used
🪛 LanguageTool
docs/docs/core-design-decisions.md
[uncategorized] ~13-~13: Possible missing comma found.
Context: ...raps around an error and adds a message field which is is text that can be shown to t...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~13-~13: Possible typo: you repeated a word.
Context: ...an error and adds a message field which is is text that can be shown to the user. For...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~13-~13: Possible missing comma found.
Context: ...t can be shown to the user. For more on this see Errors - You should not ...
(AI_HYDRA_LEO_MISSING_COMMA)
docs/docs/core-concepts/3-parsing.mdx
[style] ~51-~51: To form a complete sentence, be sure to include a subject.
Context: ...ZogIssues are returned. - Can be caught by catch 2. Default Check -> ...
(MISSING_IT_THERE)
[style] ~59-~59: To form a complete sentence, be sure to include a subject.
Context: ...ns a required ZogIssue. - Can be caught by catch 6. Tests -> Run all ...
(MISSING_IT_THERE)
[style] ~62-~62: To form a complete sentence, be sure to include a subject.
Context: ...are run even if one of them fails. - Can be caught by catch 7. PostTransforms ->...
(MISSING_IT_THERE)
[uncategorized] ~64-~64: A comma is probably missing here.
Context: ...ll postTransforms on the value. - On error you return: aborts and adds your error ...
(MISSING_COMMA_AFTER_INTRODUCTORY_PHRASE)
docs/docs/core-concepts/1-anatomy-of-schema.md
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...f it is required and the data is a zero value the schema will return a [ZogIssue](/er...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...rors or ZogIssues with the catch value. Meaning it will set the destination value to th...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...riggered, no matter what error triggers it code will automatically jump to the [Po...
(AI_HYDRA_LEO_MISSING_COMMA)
docs/docs/errors.md
[uncategorized] ~9-~9: Possible missing comma found.
Context: ...idation tests. Since Zog does not throw issues we use the term Issues to keep consiste...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~13-~13: Possible missing comma found.
Context: ...istency with Zod. ## Issues in Zog In zog issues represent something that went wr...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...terface which also implements the issue interface so it can be used with the issues
pac...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~83-~83: In the grammatical structure ‘pronoun + be/get + used + to + verb’, the verb ‘used’ is used with the gerund.
Context: ...f issue that can occur in Zog. They are used to generate issue messages and to identify the issu...
(ADMIT_ENJOY_VB)
[uncategorized] ~83-~83: Possible missing comma found.
Context: ...ify the issue in the issue formatter. A full updated list of issue codes can be foun...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~135-~135: Possible missing comma found.
Context: ... has multiple ways of customizing issue messages as well as support for [i18n](/packages...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~165-~165: Possible missing comma found.
Context: ...atter for the entire parsing operation. Beware you must handle all ZogIssue
codes & ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~165-~165: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...must handle all ZogIssue
codes & types or you may get unexpected messages. ```go...
(COMMA_COMPOUND_SENTENCE)
[formatting] ~216-~216: Insert a comma after “Really,”.
Context: ...e the i18n package** Really this only makes sense if you are doing ...
(INTRODUCTORY_PHRASE_MISSING_COMMA)
[uncategorized] ~217-~217: Consider adding a comma between these intensifiers.
Context: ...only makes sense if you are doing i18n. Please please check out the i18n se...
(RB_RB_COMMA)
[style] ~221-~221: In American English, abbreviations like “etc.” require a period.
Context: ...hat caused the issue, the path, params, etc). ```go errs := userSchema.Parse(data,...
(ETC_PERIOD)
docs/docs/configuration.md
[typographical] ~35-~35: Consider adding a comma here.
Context: ... configurating error formatting globally please refer to the [errors page](/errors#5-co...
(PLEASE_COMMA)
docs/docs/core-concepts/6-validation-results.mdx
[uncategorized] ~7-~7: Possible missing comma found.
Context: ... Validation Results Examples Under the hood Zog follows the [Parsing Execution Stru...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/docs/errors.md
15-15: Heading levels should only increment by one level at a time
Expected: h3; Actual: h6
(MD001, heading-increment)
56-56: Hard tabs
Column: 1
(MD010, no-hard-tabs)
57-57: Hard tabs
Column: 1
(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1
(MD010, no-hard-tabs)
59-59: Hard tabs
Column: 1
(MD010, no-hard-tabs)
60-60: Hard tabs
Column: 1
(MD010, no-hard-tabs)
61-61: Hard tabs
Column: 1
(MD010, no-hard-tabs)
62-62: Hard tabs
Column: 1
(MD010, no-hard-tabs)
63-63: Hard tabs
Column: 1
(MD010, no-hard-tabs)
64-64: Hard tabs
Column: 1
(MD010, no-hard-tabs)
65-65: Hard tabs
Column: 1
(MD010, no-hard-tabs)
66-66: Hard tabs
Column: 1
(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1
(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1
(MD010, no-hard-tabs)
69-69: Hard tabs
Column: 1
(MD010, no-hard-tabs)
70-70: Hard tabs
Column: 1
(MD010, no-hard-tabs)
71-71: Hard tabs
Column: 1
(MD010, no-hard-tabs)
72-72: Hard tabs
Column: 1
(MD010, no-hard-tabs)
73-73: Hard tabs
Column: 1
(MD010, no-hard-tabs)
74-74: Hard tabs
Column: 1
(MD010, no-hard-tabs)
75-75: Hard tabs
Column: 1
(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1
(MD010, no-hard-tabs)
90-90: Hard tabs
Column: 1
(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1
(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
95-95: Hard tabs
Column: 1
(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1
(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1
(MD010, no-hard-tabs)
100-100: Hard tabs
Column: 1
(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1
(MD010, no-hard-tabs)
103-103: Hard tabs
Column: 1
(MD010, no-hard-tabs)
104-104: Hard tabs
Column: 1
(MD010, no-hard-tabs)
105-105: Hard tabs
Column: 1
(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1
(MD010, no-hard-tabs)
108-108: Hard tabs
Column: 1
(MD010, no-hard-tabs)
109-109: Hard tabs
Column: 1
(MD010, no-hard-tabs)
110-110: Hard tabs
Column: 1
(MD010, no-hard-tabs)
111-111: Hard tabs
Column: 1
(MD010, no-hard-tabs)
112-112: Hard tabs
Column: 1
(MD010, no-hard-tabs)
113-113: Hard tabs
Column: 1
(MD010, no-hard-tabs)
114-114: Hard tabs
Column: 1
(MD010, no-hard-tabs)
115-115: Hard tabs
Column: 1
(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1
(MD010, no-hard-tabs)
117-117: Hard tabs
Column: 1
(MD010, no-hard-tabs)
118-118: Hard tabs
Column: 1
(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1
(MD010, no-hard-tabs)
120-120: Hard tabs
Column: 1
(MD010, no-hard-tabs)
121-121: Hard tabs
Column: 1
(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1
(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1
(MD010, no-hard-tabs)
124-124: Hard tabs
Column: 1
(MD010, no-hard-tabs)
126-126: Hard tabs
Column: 1
(MD010, no-hard-tabs)
127-127: Hard tabs
Column: 1
(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1
(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1
(MD010, no-hard-tabs)
137-137: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (45)
errors_test.go (1)
11-12
: Good rename from ZogError to ZogIssue in test names and variable declarations.Renaming the tests and variables to reflect the new “Issue” terminology enhances consistency across the codebase.
Also applies to: 23-24
conf/issueFormatConf_test.go (3)
11-14
: LGTM! Function and type changes align with the refactoring objectives.The renaming from
TestDefaultErrorFormatter
toTestDefaultIssueFormatter
and the type change fromp.ZogError
top.ZogIssue
are consistent with the PR's goal of aligning with Zod terminology.
16-18
: Verify test cases cover all edge cases.The test cases cover:
- Default message from map
- Custom message override
- Invalid code fallback
Consider adding test cases for:
- Multiple issues in the same context
- Nested issues
21-24
: LGTM! Formatter call update is consistent.The change from
ErrorFormatter
toIssueFormatter
maintains consistency with the new terminology.utilsOptions_test.go (2)
16-27
: LGTM! Function and parameter changes are consistent.The renaming from
TestWithErrFormatter
toTestWithIssueFormatter
and the type changes top.ZogIssue
maintain consistency with the refactoring objectives.
29-36
: LGTM! MessageFunc changes align with the new type system.The update to use
p.ZogIssue
in the MessageFunc parameter maintains type consistency across the codebase.tutils/testIssueMessages.go (1)
34-38
: LGTM! Map verification function is well-structured.The function effectively iterates through the issue map and reuses the single issue verification logic.
parsers/zjson/parseJson.go (1)
26-34
: LGTM! Documentation updates are clear and helpful.The comments effectively explain the behavior for different JSON input scenarios.
utilsOptions.go (1)
53-57
: LGTM! Proper deprecation notice.The deprecation notice clearly indicates the replacement function and reason.
zhttp/zhttp.go (1)
30-34
: LGTM! Consistent type updates.The return type and error code changes align with the new issue terminology.
conf/issueFormatConf.go (1)
48-62
: LGTM! Well-documented usage example.The documentation provides a clear example of how to customize the issue formatter while maintaining fallback behavior.
i18n/en/en.go (1)
7-73
: LGTM! Comprehensive and consistent renaming.The changes to the language map are thorough and maintain consistency with the new issue-based terminology.
i18n/es/es.go (1)
7-73
: LGTM! Consistent renaming of error codes to issue codes.The Spanish translations are well-structured and grammatically correct. The changes align with the PR objective of transitioning from error to issue terminology.
pointers_validate_test.go (2)
29-31
: LGTM! Updated error formatter to issue formatter.The change aligns with the PR objective and maintains the same functionality.
134-149
: LGTM! Improved test structure using default issue messages.The test has been simplified by using the default issue messages verification utility, making it more maintainable.
boolean.go (1)
47-56
: LGTM! Consistent update of return types to use ZogIssueList.The changes to
Parse
andValidate
methods are consistent with the PR objective of transitioning from error to issue terminology while maintaining the same functionality.Also applies to: 64-73
internals/tests.go (2)
14-19
: LGTM! Field renaming aligns with the PR objective.The renaming of fields from error-related to issue-related terminology is consistent with the PR's goal of better alignment with Zod.
22-29
: LGTM! Consistent renaming of error codes to issue codes.The changes in test functions consistently replace error codes with issue codes, maintaining a uniform approach across all validation functions.
Also applies to: 35-46, 48-62, 64-78, 80-96, 98-112, 114-128, 130-144, 146-160, 162-176
pointers_test.go (2)
6-6
: LGTM! Added tutils import for better issue message verification.The addition of the tutils package enhances the robustness of issue message verification in tests.
21-21
: LGTM! Consistent update of error keys to issue keys.The changes systematically update error-related keys and message verification to use the new issue-based approach.
Also applies to: 36-37, 41-44, 187-187
zogSchema.go (1)
21-23
: LGTM! Interface return types updated to use issue-based terminology.The changes to ComplexZogSchema and PrimitiveZogSchema interfaces maintain functionality while aligning with the new issue-based approach.
Also applies to: 28-30
numbers.go (1)
62-72
: LGTM! Method signatures and context creation updated to use issue-based terminology.The changes in Parse and Validate methods consistently update error-related types to issue-related ones while maintaining functionality.
Also applies to: 80-89
utils.go (2)
75-84
: LGTM! Consistent with the new issue-based approach.The TestFunc implementation correctly uses IssueCode instead of ErrCode, maintaining consistency with the broader refactoring effort.
86-105
: LGTM! Well-structured issue handling utilities.The new Issues struct and its methods provide a clean interface for handling issue messages, with good separation of concerns between map and list handling.
time.go (2)
73-100
: LGTM! Consistent updates to error handling.The Parse and Validate methods have been correctly updated to use IssueFormatter and ZogIssueList, maintaining consistency with the new issue-based approach.
175-239
: LGTM! Consistent validation method updates.The time validation methods (After, Before, EQ) have been correctly updated to use IssueCode, maintaining consistency across all validation methods.
slices_test.go (1)
61-61
: LGTM! Improved test consistency with centralized verification.The tests have been correctly updated to use tutils.VerifyDefaultIssueMessagesMap, providing consistent verification of issue messages across all test cases.
Also applies to: 136-136, 186-186, 193-193, 200-200, 216-216
slices_validate_test.go (1)
26-26
: LGTM! Consistent validation test updates.The validation tests have been correctly updated to use tutils.VerifyDefaultIssueMessagesMap, maintaining consistency with the broader test suite changes.
Also applies to: 107-107, 123-123, 139-139, 155-155
zconst/consts.go (2)
35-54
: LGTM! Type changes are well-documented and properly deprecated.The type changes from
ZogErrCode
toZogIssueCode
are consistent with the PR objectives. The deprecation notice is clear, and the new type is well-documented.
56-192
: LGTM! Constants are properly deprecated and documented.The changes maintain backward compatibility while introducing new issue-based terminology:
- Each deprecated constant has a clear deprecation notice
- New constants maintain the same values
- Type-specific comments are preserved
zhttp/zhttp_test.go (1)
204-204
: LGTM! Test assertion updated to use new issue code.The test assertion has been correctly updated to use
IssueCodeInvalidJSON
instead ofErrCodeInvalidJSON
, maintaining consistency with the new terminology.numbers_validate_test.go (1)
22-22
: LGTM! Formatter function updated to use new issue type.The formatter function has been correctly updated to use
ZogIssue
instead ofZogError
, maintaining consistency with the new terminology.numbers_test.go (1)
22-22
: LGTM! Formatter function updated to use new issue type.The formatter function has been correctly updated to use
ZogIssue
instead ofZogError
, maintaining consistency with the new terminology.string_validate_test.go (1)
7-7
: LGTM: Import added for issue message verification.The addition of the
tutils
import aligns with the broader refactoring to standardize error handling.slices.go (1)
52-52
: LGTM: Return type and context creation updated.The changes to use
ZogIssueMap
andIssueFormatter
align with the broader refactoring for consistent issue handling.Also applies to: 55-55
string.go (1)
55-56
: LGTM: Return types updated for consistency.The changes to use
ZogIssueList
align with the broader refactoring for consistent issue handling.Also applies to: 76-77
boolean_test.go (1)
8-8
: LGTM: Import added for issue message verification.The addition of the
tutils
import aligns with the broader refactoring to standardize error handling.docs/docs/core-concepts/5-validate.mdx (1)
41-43
: LGTM! Terminology changes are consistent.The changes from error-related terms to issue-related terms are consistent with the PR's objective and maintain clarity in the documentation.
Also applies to: 59-59
docs/docs/core-concepts/2-parsing-vs-validation.md (1)
9-10
: LGTM! Documentation updates are clear and consistent.The changes to use "issue" terminology in return types and error messages are consistent and improve clarity.
Also applies to: 41-41, 45-45, 47-47
docs/docs/packages/zconst.md (1)
15-16
: LGTM! Comprehensive and consistent renaming of constants.The renaming from error-related to issue-related terminology is thorough and consistent across all type definitions and constants.
Also applies to: 29-29, 32-74
docs/docs/core-concepts/3-parsing.mdx (1)
50-51
: LGTM! Terminology changes are consistent.The updates from error to issue terminology are consistent throughout the parsing execution structure documentation.
Also applies to: 55-56, 58-59, 61-62, 64-66
🧰 Tools
🪛 LanguageTool
[style] ~51-~51: To form a complete sentence, be sure to include a subject.
Context: ...ZogIssues are returned. - Can be caught by catch 2. Default Check -> ...(MISSING_IT_THERE)
struct.go (1)
48-50
: LGTM! Return type and formatter changes align with the PR objectives.The changes from
ZogErrMap
toZogIssueMap
andErrorFormatter
toIssueFormatter
are consistent with the broader refactoring effort.struct_test.go (1)
161-163
: LGTM! Test coverage enhanced with standardized issue message verification.The addition of
tutils.VerifyDefaultIssueMessagesMap
improves test robustness by ensuring consistent issue message validation.boolean_validate_test.go (1)
211-214
: LGTM! Consistent error handling improvements.The changes improve test coverage by adding standardized issue message verification, consistent with other test files.
string_test.go (1)
169-181
: LGTM! Test cases simplified while maintaining coverage.The removal of custom messages in favor of default issue messages aligns with the standardization effort.
// Deprecated: Use Ctx.AddIssue() instead | ||
// This is old interface. It will be removed soon | ||
func (c *ExecCtx) NewError(path PathBuilder, e ZogError) { | ||
func (c *ExecCtx) NewError(path PathBuilder, e ZogIssue) { | ||
c.Errors.Add(path.String(), e) | ||
} |
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.
🛠️ Refactor suggestion
Consider removing deprecated method in this major version.
Since this is a breaking change PR (indicated by refactor!
in the title), consider removing the deprecated NewError
method now rather than keeping it around.
@@ -21,6 +22,7 @@ func TestValidateStringOptionalByDefault(t *testing.T) { | |||
|
|||
errs = field.Validate(&dest) | |||
assert.NotEmpty(t, errs) | |||
tutils.VerifyDefaultIssueMessages(t, errs) |
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.
💡 Verification agent
🧩 Analysis chain
Verify test coverage for error cases.
The test cases now use tutils.VerifyDefaultIssueMessages
for error verification, but some error scenarios might need additional coverage:
- Error messages for invalid string transformations
- Error messages for concurrent validation
- Error messages for edge cases (empty strings, special characters)
Also applies to: 95-96, 189-189, 195-195
🏁 Script executed:
#!/bin/bash
# Check test coverage for error cases
ast-grep --pattern 'func Test.*\(\s*t\s*\*testing\.T\s*\)\s*{\s*$$$\s*tutils\.VerifyDefaultIssueMessages\s*\(\s*t\s*,\s*errs\s*\)\s*$$$\s*}'
Length of output: 142
Directive: Verify and Enhance Error Case Test Coverage
It appears that the tests in string_validate_test.go
rely solely on a generic call to
tutils.VerifyDefaultIssueMessages(t, errs)
at the noted lines. We couldn’t locate additional specific verifications for error cases such as invalid string transformations, concurrent validation, or edge cases like empty strings and special characters.
To further evaluate coverage, please run the following shell script to inspect the test file around the mentioned line numbers:
After reviewing these sections, if you find that error scenarios (invalid transformations, concurrent errors, or edge cases) are not explicitly tested, please add or extend tests to cover those cases.
🏁 Script executed:
#!/bin/bash
echo "Displaying specific lines from string_validate_test.go for review:"
echo "Line 25:"
sed -n '25p' string_validate_test.go
echo "Lines 95-96:"
sed -n '95,96p' string_validate_test.go
echo "Line 189:"
sed -n '189p' string_validate_test.go
echo "Line 195:"
sed -n '195p' string_validate_test.go
Length of output: 617
Action Required: Expand Test Coverage for Detailed Error Scenarios
The current test file only uses:
tutils.VerifyDefaultIssueMessages(t, errs)
(and one instance ofassert.NotEmpty(t, errs)
),
without explicit coverage for:- Invalid string transformations,
- Concurrent validations, and
- Specific edge cases (like empty strings or special characters).
Based on the reviewed lines, these scenarios are not explicitly tested. It is recommended to add dedicated tests addressing these error conditions.
if v.required == nil && code == zconst.IssueCodeCoerce { | ||
return |
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.
💡 Verification agent
🧩 Analysis chain
Verify all error code references are updated.
The changes from ErrCode
to IssueCode
constants are consistent. However, let's verify that all error code references in the codebase have been updated.
Also applies to: 113-114
🏁 Script executed:
#!/bin/bash
# Search for any remaining references to ErrCode
rg -i "errcode" --type go
Length of output: 2361
Error Code Constants Inconsistency Detected
The usage in application code (e.g., in struct.go
using zconst.IssueCodeCoerce
) has been updated. However, our search shows that the definitions in zconst/consts.go
still use the old ErrCode
prefix. If the intent is to fully replace ErrCode
references with IssueCode
, these definitions need to be updated as well.
- Found in
zconst/consts.go
: Multiple constants (e.g.,ErrCodeCoerce
,ErrCodeRequired
, etc.) still retain theErrCode
prefix. - Usage in other parts:
struct.go
and likely other files now referenceIssueCode
constants.
Please verify if the legacy ErrCode
constants are still required for backward compatibility or if they should be renamed to fully adopt the new naming convention.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
42-42
: Fix grammatical error in API stability note.There's a word repetition in the sentence.
-> However, I believe very little API changes will happen from the current implementation. The APIs are are most likely to change are the **data providers** (please don't make your own if possible use the helpers whose APIs will not change meaningfully) and the Ctx most other APIs should remain the same +> However, I believe very little API changes will happen from the current implementation. The APIs that are most likely to change are the **data providers** (please don't make your own if possible use the helpers whose APIs will not change meaningfully) and the Ctx most other APIs should remain the same🧰 Tools
🪛 LanguageTool
[duplication] ~42-~42: Possible typo: you repeated a word.
Context: ...om the current implementation. The APIs are are most likely to change are the **data pr...(ENGLISH_WORD_REPEAT_RULE)
43-43
: Fix hyphenation in API stability note.The term "non basic" should be hyphenated.
-> Zog will not respect semver until v1.0.0 is released. Expect breaking changes (mainly in non basic apis) until then. +> Zog will not respect semver until v1.0.0 is released. Expect breaking changes (mainly in non-basic APIs) until then.🧰 Tools
🪛 LanguageTool
[misspelling] ~43-~43: This expression is normally spelled as one or with a hyphen.
Context: ...sed. Expect breaking changes (mainly in non basic apis) until then. ## Introduction ##...(EN_COMPOUNDS_NON_BASIC)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[duplication] ~42-~42: Possible typo: you repeated a word.
Context: ...om the current implementation. The APIs are are most likely to change are the **data pr...
(ENGLISH_WORD_REPEAT_RULE)
[misspelling] ~43-~43: This expression is normally spelled as one or with a hyphen.
Context: ...sed. Expect breaking changes (mainly in non basic apis) until then. ## Introduction ##...
(EN_COMPOUNDS_NON_BASIC)
🔇 Additional comments (4)
README.md (4)
22-22
: LGTM!The description is clear, concise, and accurately describes Zog's functionality.
28-30
: LGTM!The feature list updates are well-aligned with the PR objectives and provide clearer guidance about extensibility and error handling.
33-37
: LGTM!The helper packages list is updated correctly to include the new zjson package.
45-45
: LGTM!The added empty line improves readability by separating sections.
…rmatter and conf.DefaultErrorFormatter. Use IssueFormatter and DefaultIssueFormatter instead.
2c261cf
to
9a49c38
Compare
Deploying zog with
|
Latest commit: |
9a49c38
|
Status: | ✅ Deploy successful! |
Preview URL: | https://d0e7f9c0.zog-3a0.pages.dev |
Branch Preview URL: | https://refactor-rename-zogerr-to-is.zog-3a0.pages.dev |
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.
Actionable comments posted: 1
🧹 Nitpick comments (19)
docs/docs/core-design-decisions.md (1)
13-13
: Fix grammatical error and update documentation link.The line contains a word repetition and potentially outdated link:
- "which is is text" should be "which is text"
- Consider updating the link from "/errors" to "/issues" for consistency with the new terminology.
Apply this diff to fix the issues:
-Errors returned by you (for example in a `PreTransform` or `PostTransform` function) can be the ZogIssue interface or an error. If you return an error, it will be wrapped in a ZogIssue. ZogIssue is just a struct that wraps around an error and adds a message field which is is text that can be shown to the user. For more on this see [Errors](/errors) +Errors returned by you (for example in a `PreTransform` or `PostTransform` function) can be the ZogIssue interface or an error. If you return an error, it will be wrapped in a ZogIssue. ZogIssue is just a struct that wraps around an error and adds a message field which is text that can be shown to the user. For more on this see [Issues](/issues)🧰 Tools
🪛 LanguageTool
[duplication] ~13-~13: Possible typo: you repeated a word.
Context: ...an error and adds a message field which is is text that can be shown to the user. For...(ENGLISH_WORD_REPEAT_RULE)
i18n/es/es.go (1)
7-7
: Consider adding documentation for the Map variable.Adding a comment explaining the purpose and usage of this map would improve code maintainability.
+// Map provides Spanish translations for validation issues. +// It maps each type's validation issues to their corresponding Spanish messages. var Map zconst.LangMap = map[zconst.ZogType]map[zconst.ZogIssueCode]string{utils.go (1)
134-140
: Consider simplifying error wrapping logic.The error wrapping logic could be simplified by using type assertion with comma-ok idiom in a more idiomatic way.
Consider this alternative implementation:
func (e *errHelpers) WrapUnknown(o any, destType zconst.ZogType, err error) ZogIssue { - zerr, ok := err.(ZogIssue) - if !ok { - return e.FromErr(o, destType, err) - } - return zerr + if zerr, ok := err.(ZogIssue); ok { + return zerr + } + return e.FromErr(o, destType, err) }docs/docs/core-concepts/1-anatomy-of-schema.md (3)
32-34
: Clarify error handling behavior in PreTransform documentation.The documentation should be more explicit about how errors and ZogIssues are handled. Consider clarifying:
- Whether the function accepts both error and ZogIssue types
- How errors are wrapped into ZogIssues
-// The function may return an error or a ZogIssue. In this case all validation will be skipped and the error will be wrapped into a ZogIssue and entire execution will return. +// The function may return either a standard error or a ZogIssue. If a standard error is returned, it will be automatically wrapped into a ZogIssue. In both cases, all validation will be skipped and the execution will return immediately.
51-51
: Improve readability with proper punctuation.Add commas to improve sentence structure and readability.
-`schema.Required()` is a boolean that indicates if the field is required. If it is required and the data is a zero value the schema will return a [ZogIssue](/errors). +`schema.Required()` is a boolean that indicates if the field is required. If it is required, and the data is a zero value, the schema will return a [ZogIssue](/errors).🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...f it is required and the data is a zero value the schema will return a [ZogIssue](/er...(AI_HYDRA_LEO_MISSING_COMMA)
51-51
: Update documentation for consistency and readability.
- Update the link from
/errors
to/issues
for consistency with the new terminology.- Add commas to improve sentence structure.
-If it is required and the data is a zero value the schema will return a [ZogIssue](/errors). +If it is required and the data is a zero value the schema will return a [ZogIssue](/issues). -`schema.Catch(value)` sets a catch value. If this is set it will "catch" any errors or ZogIssues with the catch value. Meaning it will set the destination value to the catch value and exit. When this is triggered, no matter what error triggers it code will automatically jump to the [PostTransforms](#posttransforms). +`schema.Catch(value)` sets a catch value. If this is set, it will "catch" any errors or ZogIssues with the catch value. Meaning, it will set the destination value to the catch value and exit. When this is triggered, no matter what error triggers it, code will automatically jump to the [PostTransforms](#posttransforms).Also applies to: 55-55
🧰 Tools
🪛 LanguageTool
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...f it is required and the data is a zero value the schema will return a [ZogIssue](/er...(AI_HYDRA_LEO_MISSING_COMMA)
conf/issueFormatConf_test.go (1)
16-17
: Consider renamingDefaultErrMsgMap
toDefaultIssueMsgMap
.For consistency with the PR's objective of renaming error-related entities to issue-related ones, consider renaming
DefaultErrMsgMap
toDefaultIssueMsgMap
.- {input: &p.ZogErr{C: zconst.IssueCodeRequired, Typ: zconst.TypeString}, want: DefaultErrMsgMap[zconst.TypeString][zconst.IssueCodeRequired]}, + {input: &p.ZogErr{C: zconst.IssueCodeRequired, Typ: zconst.TypeString}, want: DefaultIssueMsgMap[zconst.TypeString][zconst.IssueCodeRequired]},slices_test.go (1)
127-137
: Consider renamingTestSliceErrors
toTestSliceIssues
.For consistency with the PR's objective of renaming error-related entities to issue-related ones, consider renaming this test function.
-func TestSliceErrors(t *testing.T) { +func TestSliceIssues(t *testing.T) {string.go (1)
57-57
: Consider renaming 'errs' variable for consistency.For better alignment with the new terminology, consider renaming the 'errs' variable to 'issues'.
- errs := p.NewErrsList() + issues := p.NewIssuesList()Also applies to: 77-77
docs/docs/core-concepts/6-validation-results.mdx (1)
7-7
: Add a comma for better readability.Add a comma after "Under the hood" for improved readability.
-Under the hood Zog follows the [Parsing Execution Structure] +Under the hood, Zog follows the [Parsing Execution Structure]🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: A comma might be missing here.
Context: ... Validation Results Examples Under the hood Zog follows the [Parsing Execution Stru...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
docs/docs/configuration.md (1)
35-35
: Fix typo and add comma.The word "configurating" is incorrect, and a comma is missing after "globally".
-For information on configurating error formatting globally please refer to the [errors page](/errors#5-configure-issue-messages-globally). +For information on configuring error formatting globally, please refer to the [errors page](/errors#5-configure-issue-messages-globally).🧰 Tools
🪛 LanguageTool
[typographical] ~35-~35: Consider adding a comma here.
Context: ... configurating error formatting globally please refer to the [errors page](/errors#5-co...(PLEASE_COMMA)
docs/docs/index.md (1)
27-27
: Improve readability with proper punctuation.Add commas to improve readability and fix the compound sentence structure.
-> However, I believe very little API changes will happen from the current implementation. The APIs most likely to change are the **data providers** (please don't make your own if possible use the helpers whose APIs will not change meaningfully) and the z.Ctx most other APIs should remain the same. I could be wrong but I don't expect many breaking changes. +> However, I believe very little API changes will happen from the current implementation. The APIs most likely to change are the **data providers** (please don't make your own if possible, use the helpers whose APIs will not change meaningfully) and the z.Ctx most other APIs should remain the same. I could be wrong, but I don't expect many breaking changes.🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...viders** (please don't make your own if possible use the helpers whose APIs will not cha...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~27-~27: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...should remain the same. I could be wrong but I don't expect many breaking changes. >...(COMMA_COMPOUND_SENTENCE)
docs/docs/packages/zconst.md (1)
32-74
: Convert hard tabs to spaces for better markdown compatibility.The markdown file uses hard tabs which can cause rendering issues in some markdown processors.
Replace hard tabs with spaces (typically 2 or 4 spaces) for better compatibility and consistent rendering across different platforms.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
32-32: Hard tabs
Column: 1(MD010, no-hard-tabs)
33-33: Hard tabs
Column: 1(MD010, no-hard-tabs)
34-34: Hard tabs
Column: 1(MD010, no-hard-tabs)
35-35: Hard tabs
Column: 1(MD010, no-hard-tabs)
36-36: Hard tabs
Column: 1(MD010, no-hard-tabs)
37-37: Hard tabs
Column: 1(MD010, no-hard-tabs)
38-38: Hard tabs
Column: 1(MD010, no-hard-tabs)
39-39: Hard tabs
Column: 1(MD010, no-hard-tabs)
41-41: Hard tabs
Column: 1(MD010, no-hard-tabs)
42-42: Hard tabs
Column: 1(MD010, no-hard-tabs)
43-43: Hard tabs
Column: 1(MD010, no-hard-tabs)
44-44: Hard tabs
Column: 1(MD010, no-hard-tabs)
46-46: Hard tabs
Column: 1(MD010, no-hard-tabs)
47-47: Hard tabs
Column: 1(MD010, no-hard-tabs)
48-48: Hard tabs
Column: 1(MD010, no-hard-tabs)
49-49: Hard tabs
Column: 1(MD010, no-hard-tabs)
50-50: Hard tabs
Column: 1(MD010, no-hard-tabs)
52-52: Hard tabs
Column: 1(MD010, no-hard-tabs)
53-53: Hard tabs
Column: 1(MD010, no-hard-tabs)
54-54: Hard tabs
Column: 1(MD010, no-hard-tabs)
55-55: Hard tabs
Column: 1(MD010, no-hard-tabs)
56-56: Hard tabs
Column: 1(MD010, no-hard-tabs)
57-57: Hard tabs
Column: 1(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1(MD010, no-hard-tabs)
59-59: Hard tabs
Column: 1(MD010, no-hard-tabs)
60-60: Hard tabs
Column: 1(MD010, no-hard-tabs)
61-61: Hard tabs
Column: 1(MD010, no-hard-tabs)
62-62: Hard tabs
Column: 1(MD010, no-hard-tabs)
63-63: Hard tabs
Column: 1(MD010, no-hard-tabs)
64-64: Hard tabs
Column: 1(MD010, no-hard-tabs)
65-65: Hard tabs
Column: 1(MD010, no-hard-tabs)
66-66: Hard tabs
Column: 1(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1(MD010, no-hard-tabs)
70-70: Hard tabs
Column: 1(MD010, no-hard-tabs)
71-71: Hard tabs
Column: 1(MD010, no-hard-tabs)
72-72: Hard tabs
Column: 1(MD010, no-hard-tabs)
73-73: Hard tabs
Column: 1(MD010, no-hard-tabs)
docs/docs/core-concepts/3-parsing.mdx (2)
50-51
: Add missing comma for better readability.The sentence would be clearer with a comma after "error".
- - On error all parsing and validation stops and [ZogIssues](/errors) are returned. + - On error, all parsing and validation stops and [ZogIssues](/errors) are returned.🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: A comma might be missing here.
Context: ...workflow.png) 1. Pretransforms - On error all parsing and validation stops and [Z...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[style] ~51-~51: To form a complete sentence, be sure to include a subject.
Context: ...ZogIssues are returned. - Can be caught by catch 2. Default Check -> ...(MISSING_IT_THERE)
61-61
: Improve sentence structure with proper punctuation.The sentence would be clearer with better punctuation.
- - On error: validation [ZogIssues](/errors) are added to the [ZogIssueList](/errors#zogissuelist) or [ZogIssueMap](/errors#zogissuelist). All validation functions are run even if one of them fails. + - On error: validation [ZogIssues](/errors) are added to the [ZogIssueList](/errors#zogissuelist) or [ZogIssueMap](/errors#zogissuelist), and all validation functions are run even if one of them fails.🧰 Tools
🪛 LanguageTool
[uncategorized] ~61-~61: Possible missing comma found.
Context: ...ssuelist). All validation functions are run even if one of them fails. - Can be ...(AI_HYDRA_LEO_MISSING_COMMA)
README.md (1)
42-42
: Improve sentence structure with proper punctuation.The sentence would be clearer with better punctuation.
-> - However, I believe very little API changes will happen from the current implementation. The APIs most likely to change are the **data providers** (please don't make your own if possible use the helpers whose APIs will not change meaningfully) and the z.Ctx most other APIs should remain the same. I could be wrong but I don't expect many breaking changes. +> - However, I believe very little API changes will happen from the current implementation. The APIs most likely to change are the **data providers** (please don't make your own if possible, use the helpers whose APIs will not change meaningfully) and the z.Ctx. Most other APIs should remain the same. I could be wrong, but I don't expect many breaking changes.🧰 Tools
🪛 LanguageTool
[uncategorized] ~42-~42: Possible missing comma found.
Context: ...viders** (please don't make your own if possible use the helpers whose APIs will not cha...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~42-~42: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...should remain the same. I could be wrong but I don't expect many breaking changes. >...(COMMA_COMPOUND_SENTENCE)
docs/docs/errors.md (3)
15-15
: Fix heading level increment.The heading level jumps from h2 to h6, which violates markdown heading structure rules.
-###### ZogIssueList +### ZogIssueList🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
15-15: Heading levels should only increment by one level at a time
Expected: h3; Actual: h6(MD001, heading-increment)
56-75
: Convert hard tabs to spaces in code block.The code block uses hard tabs which can cause rendering issues in some markdown processors.
Replace hard tabs with spaces (typically 2 or 4 spaces) in the code block for better compatibility and consistent rendering across different platforms.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
56-56: Hard tabs
Column: 1(MD010, no-hard-tabs)
57-57: Hard tabs
Column: 1(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1(MD010, no-hard-tabs)
59-59: Hard tabs
Column: 1(MD010, no-hard-tabs)
60-60: Hard tabs
Column: 1(MD010, no-hard-tabs)
61-61: Hard tabs
Column: 1(MD010, no-hard-tabs)
62-62: Hard tabs
Column: 1(MD010, no-hard-tabs)
63-63: Hard tabs
Column: 1(MD010, no-hard-tabs)
64-64: Hard tabs
Column: 1(MD010, no-hard-tabs)
65-65: Hard tabs
Column: 1(MD010, no-hard-tabs)
66-66: Hard tabs
Column: 1(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1(MD010, no-hard-tabs)
69-69: Hard tabs
Column: 1(MD010, no-hard-tabs)
70-70: Hard tabs
Column: 1(MD010, no-hard-tabs)
71-71: Hard tabs
Column: 1(MD010, no-hard-tabs)
72-72: Hard tabs
Column: 1(MD010, no-hard-tabs)
73-73: Hard tabs
Column: 1(MD010, no-hard-tabs)
74-74: Hard tabs
Column: 1(MD010, no-hard-tabs)
75-75: Hard tabs
Column: 1(MD010, no-hard-tabs)
216-217
: Improve sentence structure and punctuation.The sentences would be clearer with better punctuation.
-Really this only makes sense if you are doing i18n. Please please check out the [i18n](/packages/i18n) section for more information. +Really, this only makes sense if you are doing i18n. Please, please check out the [i18n](/packages/i18n) section for more information.🧰 Tools
🪛 LanguageTool
[formatting] ~216-~216: Insert a comma after “Really,”.
Context: ...e the i18n package** Really this only makes sense if you are doing ...(INTRODUCTORY_PHRASE_MISSING_COMMA)
[uncategorized] ~217-~217: Consider adding a comma between these intensifiers.
Context: ...only makes sense if you are doing i18n. Please please check out the i18n se...(RB_RB_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
README.md
(1 hunks)boolean.go
(2 hunks)boolean_test.go
(11 hunks)boolean_validate_test.go
(9 hunks)conf/Errors.go
(0 hunks)conf/issueFormatConf.go
(1 hunks)conf/issueFormatConf_test.go
(1 hunks)docs/docs/configuration.md
(1 hunks)docs/docs/context.mdx
(1 hunks)docs/docs/core-concepts/1-anatomy-of-schema.md
(4 hunks)docs/docs/core-concepts/2-parsing-vs-validation.md
(2 hunks)docs/docs/core-concepts/3-parsing.mdx
(1 hunks)docs/docs/core-concepts/5-validate.mdx
(2 hunks)docs/docs/core-concepts/6-validation-results.mdx
(1 hunks)docs/docs/core-design-decisions.md
(1 hunks)docs/docs/errors.md
(1 hunks)docs/docs/index.md
(1 hunks)docs/docs/packages/zconst.md
(2 hunks)docs/docs/zog-schemas.md
(1 hunks)errors_test.go
(2 hunks)i18n/en/en.go
(1 hunks)i18n/es/es.go
(1 hunks)i18n/i18n.go
(1 hunks)i18n/i18n_test.go
(1 hunks)internals/DataProviders.go
(3 hunks)internals/Issues.go
(6 hunks)internals/contexts.go
(6 hunks)internals/tests.go
(10 hunks)internals/types.go
(1 hunks)numbers.go
(2 hunks)numbers_test.go
(1 hunks)numbers_validate_test.go
(1 hunks)parsers/zjson/parseJson.go
(2 hunks)pointers.go
(4 hunks)pointers_test.go
(4 hunks)pointers_validate_test.go
(4 hunks)slices.go
(8 hunks)slices_test.go
(5 hunks)slices_validate_test.go
(6 hunks)string.go
(16 hunks)string_test.go
(10 hunks)string_validate_test.go
(10 hunks)struct.go
(4 hunks)struct_test.go
(3 hunks)time.go
(8 hunks)tutils/testIssueMessages.go
(1 hunks)utils.go
(4 hunks)utilsOptions.go
(3 hunks)utilsOptions_test.go
(2 hunks)zconst/consts.go
(4 hunks)zhttp/zhttp.go
(1 hunks)zhttp/zhttp_test.go
(1 hunks)zogSchema.go
(1 hunks)
💤 Files with no reviewable changes (1)
- conf/Errors.go
🚧 Files skipped from review as they are similar to previous changes (29)
- docs/docs/context.mdx
- zhttp/zhttp_test.go
- i18n/en/en.go
- i18n/i18n_test.go
- internals/types.go
- boolean_test.go
- numbers_validate_test.go
- numbers_test.go
- boolean.go
- docs/docs/core-concepts/2-parsing-vs-validation.md
- numbers.go
- i18n/i18n.go
- docs/docs/core-concepts/5-validate.mdx
- errors_test.go
- string_test.go
- boolean_validate_test.go
- zhttp/zhttp.go
- zogSchema.go
- tutils/testIssueMessages.go
- parsers/zjson/parseJson.go
- docs/docs/zog-schemas.md
- struct.go
- utilsOptions_test.go
- conf/issueFormatConf.go
- pointers_validate_test.go
- time.go
- string_validate_test.go
- internals/tests.go
- internals/contexts.go
🧰 Additional context used
🪛 LanguageTool
docs/docs/index.md
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...viders** (please don't make your own if possible use the helpers whose APIs will not cha...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~27-~27: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...should remain the same. I could be wrong but I don't expect many breaking changes. >...
(COMMA_COMPOUND_SENTENCE)
docs/docs/errors.md
[grammar] ~83-~83: In the grammatical structure ‘pronoun + be/get + used + to + verb’, the verb ‘used’ is used with the gerund.
Context: ...f issue that can occur in Zog. They are used to generate issue messages and to identify the issu...
(ADMIT_ENJOY_VB)
[uncategorized] ~83-~83: Possible missing comma found.
Context: ...ify the issue in the issue formatter. A full updated list of issue codes can be foun...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~165-~165: Possible missing comma found.
Context: ...atter for the entire parsing operation. Beware you must handle all ZogIssue
codes & ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~165-~165: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...must handle all ZogIssue
codes & types or you may get unexpected messages. ```go...
(COMMA_COMPOUND_SENTENCE)
[formatting] ~216-~216: Insert a comma after “Really,”.
Context: ...e the i18n package** Really this only makes sense if you are doing ...
(INTRODUCTORY_PHRASE_MISSING_COMMA)
[uncategorized] ~217-~217: Consider adding a comma between these intensifiers.
Context: ...only makes sense if you are doing i18n. Please please check out the i18n se...
(RB_RB_COMMA)
[style] ~221-~221: In American English, abbreviations like “etc.” require a period.
Context: ...hat caused the issue, the path, params, etc). ```go errs := userSchema.Parse(data,...
(ETC_PERIOD)
README.md
[uncategorized] ~42-~42: Possible missing comma found.
Context: ...viders** (please don't make your own if possible use the helpers whose APIs will not cha...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~42-~42: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...should remain the same. I could be wrong but I don't expect many breaking changes. >...
(COMMA_COMPOUND_SENTENCE)
docs/docs/configuration.md
[typographical] ~35-~35: Consider adding a comma here.
Context: ... configurating error formatting globally please refer to the [errors page](/errors#5-co...
(PLEASE_COMMA)
docs/docs/core-concepts/1-anatomy-of-schema.md
[uncategorized] ~51-~51: Possible missing comma found.
Context: ...f it is required and the data is a zero value the schema will return a [ZogIssue](/er...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...(value)` sets a catch value. If this is set it will "catch" any errors or ZogIssues...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...rors or ZogIssues with the catch value. Meaning it will set the destination value to th...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...riggered, no matter what error triggers it code will automatically jump to the [Po...
(AI_HYDRA_LEO_MISSING_COMMA)
docs/docs/core-concepts/3-parsing.mdx
[uncategorized] ~50-~50: A comma might be missing here.
Context: ...workflow.png) 1. Pretransforms - On error all parsing and validation stops and [Z...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[style] ~51-~51: To form a complete sentence, be sure to include a subject.
Context: ...ZogIssues are returned. - Can be caught by catch 2. Default Check -> ...
(MISSING_IT_THERE)
[style] ~59-~59: To form a complete sentence, be sure to include a subject.
Context: ...ns a required ZogIssue. - Can be caught by catch 6. Tests -> Run all ...
(MISSING_IT_THERE)
[uncategorized] ~61-~61: Possible missing comma found.
Context: ...ssuelist). All validation functions are run even if one of them fails. - Can be ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~62-~62: To form a complete sentence, be sure to include a subject.
Context: ...are run even if one of them fails. - Can be caught by catch 7. PostTransforms ->...
(MISSING_IT_THERE)
[uncategorized] ~64-~64: A comma is probably missing here.
Context: ...ll postTransforms on the value. - On error you return: aborts and adds your error ...
(MISSING_COMMA_AFTER_INTRODUCTORY_PHRASE)
docs/docs/core-concepts/6-validation-results.mdx
[uncategorized] ~7-~7: A comma might be missing here.
Context: ... Validation Results Examples Under the hood Zog follows the [Parsing Execution Stru...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
docs/docs/core-design-decisions.md
[duplication] ~13-~13: Possible typo: you repeated a word.
Context: ...an error and adds a message field which is is text that can be shown to the user. For...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
docs/docs/errors.md
15-15: Heading levels should only increment by one level at a time
Expected: h3; Actual: h6
(MD001, heading-increment)
56-56: Hard tabs
Column: 1
(MD010, no-hard-tabs)
57-57: Hard tabs
Column: 1
(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1
(MD010, no-hard-tabs)
59-59: Hard tabs
Column: 1
(MD010, no-hard-tabs)
60-60: Hard tabs
Column: 1
(MD010, no-hard-tabs)
61-61: Hard tabs
Column: 1
(MD010, no-hard-tabs)
62-62: Hard tabs
Column: 1
(MD010, no-hard-tabs)
63-63: Hard tabs
Column: 1
(MD010, no-hard-tabs)
64-64: Hard tabs
Column: 1
(MD010, no-hard-tabs)
65-65: Hard tabs
Column: 1
(MD010, no-hard-tabs)
66-66: Hard tabs
Column: 1
(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1
(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1
(MD010, no-hard-tabs)
69-69: Hard tabs
Column: 1
(MD010, no-hard-tabs)
70-70: Hard tabs
Column: 1
(MD010, no-hard-tabs)
71-71: Hard tabs
Column: 1
(MD010, no-hard-tabs)
72-72: Hard tabs
Column: 1
(MD010, no-hard-tabs)
73-73: Hard tabs
Column: 1
(MD010, no-hard-tabs)
74-74: Hard tabs
Column: 1
(MD010, no-hard-tabs)
75-75: Hard tabs
Column: 1
(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1
(MD010, no-hard-tabs)
90-90: Hard tabs
Column: 1
(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1
(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
95-95: Hard tabs
Column: 1
(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1
(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1
(MD010, no-hard-tabs)
100-100: Hard tabs
Column: 1
(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1
(MD010, no-hard-tabs)
103-103: Hard tabs
Column: 1
(MD010, no-hard-tabs)
104-104: Hard tabs
Column: 1
(MD010, no-hard-tabs)
105-105: Hard tabs
Column: 1
(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1
(MD010, no-hard-tabs)
108-108: Hard tabs
Column: 1
(MD010, no-hard-tabs)
109-109: Hard tabs
Column: 1
(MD010, no-hard-tabs)
110-110: Hard tabs
Column: 1
(MD010, no-hard-tabs)
111-111: Hard tabs
Column: 1
(MD010, no-hard-tabs)
112-112: Hard tabs
Column: 1
(MD010, no-hard-tabs)
113-113: Hard tabs
Column: 1
(MD010, no-hard-tabs)
114-114: Hard tabs
Column: 1
(MD010, no-hard-tabs)
115-115: Hard tabs
Column: 1
(MD010, no-hard-tabs)
116-116: Hard tabs
Column: 1
(MD010, no-hard-tabs)
117-117: Hard tabs
Column: 1
(MD010, no-hard-tabs)
118-118: Hard tabs
Column: 1
(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1
(MD010, no-hard-tabs)
120-120: Hard tabs
Column: 1
(MD010, no-hard-tabs)
121-121: Hard tabs
Column: 1
(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1
(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1
(MD010, no-hard-tabs)
124-124: Hard tabs
Column: 1
(MD010, no-hard-tabs)
126-126: Hard tabs
Column: 1
(MD010, no-hard-tabs)
127-127: Hard tabs
Column: 1
(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1
(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1
(MD010, no-hard-tabs)
137-137: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
docs/docs/packages/zconst.md
32-32: Hard tabs
Column: 1
(MD010, no-hard-tabs)
33-33: Hard tabs
Column: 1
(MD010, no-hard-tabs)
34-34: Hard tabs
Column: 1
(MD010, no-hard-tabs)
35-35: Hard tabs
Column: 1
(MD010, no-hard-tabs)
36-36: Hard tabs
Column: 1
(MD010, no-hard-tabs)
37-37: Hard tabs
Column: 1
(MD010, no-hard-tabs)
38-38: Hard tabs
Column: 1
(MD010, no-hard-tabs)
39-39: Hard tabs
Column: 1
(MD010, no-hard-tabs)
41-41: Hard tabs
Column: 1
(MD010, no-hard-tabs)
42-42: Hard tabs
Column: 1
(MD010, no-hard-tabs)
43-43: Hard tabs
Column: 1
(MD010, no-hard-tabs)
44-44: Hard tabs
Column: 1
(MD010, no-hard-tabs)
46-46: Hard tabs
Column: 1
(MD010, no-hard-tabs)
47-47: Hard tabs
Column: 1
(MD010, no-hard-tabs)
48-48: Hard tabs
Column: 1
(MD010, no-hard-tabs)
49-49: Hard tabs
Column: 1
(MD010, no-hard-tabs)
50-50: Hard tabs
Column: 1
(MD010, no-hard-tabs)
52-52: Hard tabs
Column: 1
(MD010, no-hard-tabs)
53-53: Hard tabs
Column: 1
(MD010, no-hard-tabs)
54-54: Hard tabs
Column: 1
(MD010, no-hard-tabs)
55-55: Hard tabs
Column: 1
(MD010, no-hard-tabs)
56-56: Hard tabs
Column: 1
(MD010, no-hard-tabs)
57-57: Hard tabs
Column: 1
(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1
(MD010, no-hard-tabs)
59-59: Hard tabs
Column: 1
(MD010, no-hard-tabs)
60-60: Hard tabs
Column: 1
(MD010, no-hard-tabs)
61-61: Hard tabs
Column: 1
(MD010, no-hard-tabs)
62-62: Hard tabs
Column: 1
(MD010, no-hard-tabs)
63-63: Hard tabs
Column: 1
(MD010, no-hard-tabs)
64-64: Hard tabs
Column: 1
(MD010, no-hard-tabs)
65-65: Hard tabs
Column: 1
(MD010, no-hard-tabs)
66-66: Hard tabs
Column: 1
(MD010, no-hard-tabs)
67-67: Hard tabs
Column: 1
(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 1
(MD010, no-hard-tabs)
70-70: Hard tabs
Column: 1
(MD010, no-hard-tabs)
71-71: Hard tabs
Column: 1
(MD010, no-hard-tabs)
72-72: Hard tabs
Column: 1
(MD010, no-hard-tabs)
73-73: Hard tabs
Column: 1
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (43)
i18n/es/es.go (1)
7-72
: LGTM! Consistent renaming from Error to Issue.The changes align well with the PR objectives, maintaining consistent terminology across all error codes while preserving the correct Spanish translations.
utils.go (3)
10-67
: LGTM! Clear deprecation notices with migration paths.The type definitions and deprecations are well-documented with clear migration paths. The examples in the documentation are comprehensive and helpful.
86-105
: LGTM! Clean implementation of issue helpers.The new
issueHelpers
struct and its methods provide a clean and well-scoped implementation for handling issue sanitization.
166-172
: LGTM! Clear deprecation notice with helpful migration example.The deprecation notice effectively communicates the simpler approach of passing maps directly to
schema.Parse()
.utilsOptions.go (6)
6-6
: LGTM!The addition of the
zconst
import is necessary for the newIssueCode
functionality.
15-15
: LGTM!The type changes from
ErrFmtFunc
toIssueFmtFunc
andZogError
toZogIssue
align with the PR's objective of standardizing issue-related terminology.
22-26
: LGTM!The type changes from
ErrFmtFunc
toIssueFmtFunc
maintain consistent terminology throughout the codebase.
28-36
: LGTM! Well-documented new functionality.The new
IssueCode
function is a valuable addition that enhances test customization capabilities. The documentation is clear and includes a helpful example.
53-57
: LGTM! Clear deprecation notice.The deprecation is well-documented and the implementation correctly delegates to
WithIssueFormatter
, facilitating a smooth transition to the new terminology.
59-64
: LGTM! Clear documentation of formatter precedence.The documentation effectively explains the hierarchy of formatters, and the implementation correctly uses the new
SetIssueFormatter
method.docs/docs/core-concepts/1-anatomy-of-schema.md (2)
65-65
: LGTM! Consistent with the PR objectives.The renaming of
ErrCode
toIssueCode
aligns well with the PR's goal of standardizing terminology around "issues".
85-85
: LGTM! Example code updated correctly.The example correctly demonstrates the use of
IssueCode
in custom test creation.internals/DataProviders.go (5)
80-83
: LGTM!The error code has been correctly updated to use
IssueCodeCoerce
while maintaining the descriptive error message.
100-103
: LGTM!The error code has been correctly updated to use
IssueCodeCoerce
while maintaining the descriptive error message.
108-111
: LGTM!The error code has been correctly updated to use
IssueCodeCoerce
while maintaining the descriptive error message.
116-119
: LGTM!The error code has been correctly updated to use
IssueCodeCoerce
while maintaining the descriptive error message.
11-11
: Verify the impact of this breaking change.The change in the return type of
DpFactory
fromZogError
toZogIssue
is a breaking change that could affect downstream code.Run the following script to identify potential impacts:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Search for usages of DpFactory to identify potential impacts. # Test: Search for DpFactory usages rg -A 5 'DpFactory' # Test: Search for function declarations matching DpFactory's signature ast-grep --pattern 'func $name($$$) (DataProvider, ZogError)'Length of output: 2718
Impact of breaking change verified.
All occurrences ofDpFactory
in the codebase now consistently useZogIssue
in their return types. No active code remains that returns the oldZogError
. This confirms that the refactoring to rename error-related types to issue-related types has been applied uniformly.conf/issueFormatConf_test.go (1)
11-14
: LGTM! Consistent renaming of error to issue.The renaming of the test function and input type from error to issue is consistent with the PR objective.
pointers.go (2)
40-42
: LGTM! Consistent renaming of error to issue.The return type change from
p.ZogErrMap
top.ZogIssueMap
and the use ofconf.IssueFormatter
are consistent with the PR objective.
105-106
: LGTM! Consistent field renaming.The field renaming from
ErrCode
toIssueCode
maintains consistency with the PR's objective.pointers_test.go (2)
6-7
: LGTM! Good addition of test utilities.Adding the
tutils
package improves test maintainability by centralizing issue message verification.
20-21
: LGTM! Consistent use of issue message verification.The use of
tutils.VerifyDefaultIssueMessagesMap
andzconst.ISSUE_KEY_ROOT
maintains consistency with the PR's objective.Also applies to: 186-187
slices_test.go (2)
8-9
: LGTM! Good addition of test utilities.Adding the
tutils
package improves test maintainability by centralizing issue message verification.
179-200
: LGTM! Good simplification of test assertions.Removing custom messages in favor of default issue messages and using
tutils.VerifyDefaultIssueMessagesMap
improves test maintainability.internals/Issues.go (5)
9-68
: Well-structured interface definition with clear documentation!The interface is well-defined with comprehensive documentation for each method. The method signatures are consistent with the new "issue" terminology.
70-74
: Good backward compatibility handling!The type alias and formatter function type changes maintain backward compatibility while moving towards the new terminology.
76-85
: Clean struct implementation!The struct fields are well-organized and consistently use the new terminology.
87-164
: Consistent method implementations!All methods are properly implemented with consistent return types and error handling.
166-228
: Well-defined collection types and implementation!The list and map types are properly defined, and the implementation is consistent with the new terminology.
zconst/consts.go (3)
3-48
: Clean constant and type definitions!The constants and type definitions are consistently renamed and well-documented.
50-54
: Clear deprecation notice and type definition!The deprecation notice is well-documented, and the new type is properly defined as its replacement.
56-192
: Comprehensive set of issue code constants!Each constant is well-documented with clear deprecation notices and corresponding new issue codes.
struct_test.go (2)
158-164
: Well-structured test case update!The test case properly validates the issue handling with appropriate assertions.
225-229
: Good test coverage for issue handling!The additional test cases properly verify the behavior with missing fields using the new issue verification utilities.
slices.go (3)
52-62
: Clean method signature updates!The Validate and Parse methods are consistently updated to use the new issue-related types and formatters.
Also applies to: 132-142
322-349
: Consistent test struct updates!The Contains method's test struct properly uses the new issue codes and parameters.
351-395
: Well-updated helper functions!The slice validation helper functions consistently use the new issue codes and maintain clean implementations.
docs/docs/packages/zconst.md (1)
15-16
: LGTM! Documentation updated to reflect new terminology.The comment and type definition correctly use the new "issue" terminology, maintaining consistency with the PR's objectives.
slices_validate_test.go (5)
7-7
: LGTM: Import added for test utilities.The addition of the
tutils
package import aligns with the PR's objective to standardize issue message verification.
12-12
: LGTM: Improved test structure and standardized issue verification.The changes improve test readability by separating validator creation steps and align with the standardized issue message verification approach.
Also applies to: 20-20, 26-26
95-95
: LGTM: Standardized length validation issue messages.The changes align with the PR's objective to use default issue messages for length validation.
Also applies to: 107-107
111-111
: LGTM: Standardized minimum length validation issue messages.The changes align with the PR's objective to use default issue messages for minimum length validation.
Also applies to: 123-123
127-127
: LGTM: Standardized maximum length validation issue messages.The changes align with the PR's objective to use default issue messages for maximum length validation.
Also applies to: 139-139
} | ||
|
||
func TestValidateSliceContains(t *testing.T) { | ||
validator := Slice(String()).Contains("test", Message("custom")) | ||
validator := Slice(String()).Contains("test") |
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.
💡 Verification agent
🧩 Analysis chain
Verify consistency in message handling across tests.
While these changes align with using default issue messages, TestValidateSliceMultipleValidators
(lines 214-217) still uses custom messages. Consider whether this test should also be updated for consistency.
Also applies to: 155-155
🏁 Script executed:
#!/bin/bash
# Description: Check for remaining custom message usage in test files
# Look for patterns indicating custom message usage in validator chains
rg -l 'Message\(".*"\)' '**/*_test.go'
Length of output: 93
I'm re-running the search specifically on the file under test (slices_validate_test.go) to check for any occurrences of custom message usage (e.g. calls to Message("…")
) since the glob pattern used earlier didn’t match.
Below is a script that checks for any lines containing the custom message pattern in that file:
Once we get the output from this command, we can verify whether TestValidateSliceMultipleValidators (around lines 214–217) or the referenced lines near 155 still contain custom messages while other tests (such as in TestValidateSliceContains) use default messages.
If the output shows occurrences of Message("…")
in those sections, it confirms the inconsistency in message handling mentioned in the review comment. Otherwise, if no such calls are found, it may warrant a manual re-review of the changes.
Please run this script and review the results so we can decide whether the test(s) should be updated for consistency.
🏁 Script executed:
[run_scripts]
#!/bin/bash
# Check for custom message usage in slices_validate_test.go
rg -n 'Message\(".*"\)' slices_validate_test.go
Length of output: 322
Inconsistency in Message Handling Across Tests
The verification shows that TestValidateSliceMultipleValidators
still uses custom messages:
- Line 164: Uses
Message("custom")
- Lines 215-217: Use
Message("too short")
,Message("too long")
, andMessage("must contain test")
This is inconsistent with the intended switch to default issue messages (as seen in TestValidateSliceContains
). Please review whether the custom messages in these tests should be updated for consistency.
Summary by CodeRabbit