refactor!: Zog issue interface is converted into a struct#119
refactor!: Zog issue interface is converted into a struct#119
Conversation
… ZogErrList and ZogErrMap types.
WalkthroughThis pull request refactors error handling across the codebase by replacing method calls (e.g., Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Case
participant Formatter as Issue Formatter
participant Issue as ZogIssue
Test->>Formatter: Invoke formatter with *ZogIssue
Formatter->>Issue: Access properties (Message, Code)
Issue-->>Formatter: Return field values
Formatter-->>Test: Return formatted error message
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
Deploying zog with
|
| Latest commit: |
8861cd4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d4bcc845.zog-3a0.pages.dev |
| Branch Preview URL: | https://refactor-zogissue-as-struct.zog-3a0.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (11)
conf/issueFormatConf.go (1)
53-53: Update example in documentation commentThe usage example in the comment still shows the old signature
func(e p.ZogIssue, c z.Ctx)which should be updated to use a pointer parameterfunc(e *p.ZogIssue, c z.Ctx)to match the actual implementation.-conf.IssueFormatter = func(e p.ZogIssue, c z.Ctx) { +conf.IssueFormatter = func(e *p.ZogIssue, c z.Ctx) {docs/docs/core-concepts/1-anatomy-of-schema.md (1)
45-46: Grammar and formatting fixes needed in FOOTGUNS section.The formatting of the FOOTGUNS section has been changed to use blockquotes, but there are some grammatical issues to address.
-> **FOOTGUNS** > _Type Coercion_: Please note that pretransforms are executed before type coercion (if using `schema.Parse()`). This means that if you are responsible for checking that the data matches your expected type. If you blinding typecast the data to, for example, an int and your user provides a string as input you will cause a panic. -> _Pure Functions_: Since pretransforms are pure functions. A copy of the data is passed in. So if you place a preTransform on a large struct it will copy the entire struct which is not efficient. Be careful! +> **FOOTGUNS** +> +> _Type Coercion_: Please note that pretransforms are executed before type coercion (if using `schema.Parse()`). This means that you are responsible for checking that the data matches your expected type. If you're blindly typecasting the data to, for example, an int and your user provides a string as input, you will cause a panic. +> +> _Pure Functions_: Since pretransforms are pure functions, a copy of the data is passed in. So if you place a preTransform on a large struct, it will copy the entire struct which is not efficient. Be careful!🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: “you” seems less likely than “you’re” (you are).
Context: ...the data matches your expected type. If you blinding typecast the data to, for exam...(AI_HYDRA_LEO_CP_YOU_YOUARE)
[uncategorized] ~45-~45: Possible missing comma found.
Context: ... int and your user provides a string as input you will cause a panic. > _Pure Functio...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~46-~46: Possible missing comma found.
Context: ... a large struct it will copy the entire struct which is not efficient. Be careful! ##...(AI_HYDRA_LEO_MISSING_COMMA)
struct.go (2)
95-103: Improved error handling pattern with clearer variable assignment.The code has been refactored to use a temporary variable
newDpfor the factory result, allowing separate error checking before assigning todataProv. This improves readability by separating the concerns of function calling, error checking, and variable assignment.
104-110: Improved error handling pattern with clearer variable assignment.Similar to the previous change, this section has been refactored to use a temporary variable
newDpinstead of directly assigning todataProv. This makes the code more maintainable and easier to reason about.internals/DataProviders.go (1)
74-76: Improved nil value handling.Added a new check for
val == nilwhich now returns anEmptyDataProviderwith the underlying value set tovalinstead of returning an error. This improves the handling of nil values.Consider adding a unit test for this new code path to ensure nil values are handled correctly.
utils.go (2)
67-67: Minor spelling fix for comment.Consider changing "Suger" to "Sugar" in your comment for clarity and consistency with standard nomenclature.
-// Suger function that does both sanitize and collect. +// Sugar function that does both sanitize and collect.
92-97: Minor spelling fix for comment.As with the earlier comment, "Suger" should be "Sugar" for clarity and consistency.
-// Suger function that does both sanitize and collect. +// Sugar function that does both sanitize and collect.docs/docs/errors.md (3)
9-9: Add missing comma after “For example”.A comma is typically used after “For example” to follow grammar conventions.
-For example the error generated trying to unmarshal... +For example, the error generated trying to unmarshal...🧰 Tools
🪛 LanguageTool
[typographical] ~9-~9: After the expression ‘for example’ a comma is usually used.
Context: ...rlying error that caused the issue. For example the error generated trying to unmarshal...(COMMA_FOR_EXAMPLE)
50-76: Replace hard tabs with spaces.The markdown lint indicates multiple hard tabs in this section. Converting to spaces can avoid potential alignment issues across editors.
-<TAB>ISSUE_KEY_FIRST = "$first" + ISSUE_KEY_FIRST = "$first"(And similarly for other lines flagged by the linter.)
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
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)
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)
80-80: Add period after “etc.”In American English, abbreviations like “etc.” customarily include a period.
-... the path, params, etc) +... the path, params, etc.)🧰 Tools
🪛 LanguageTool
[style] ~80-~80: 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)
internals/Issues.go (1)
93-95: Reset fields before pooling if necessary.
FreeIssue(i *ZogIssue)callsZogIssuePool.Put(i), but the code does not appear to reset individual fields. Consider clearing or zeroing outi’s fields (e.g.,i.Code = 0,i.Value = nil) to prevent data contamination in future reuses.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
boolean_test.go(1 hunks)conf/issueFormatConf.go(1 hunks)conf/issueFormatConf_test.go(1 hunks)docs/docs/core-concepts/1-anatomy-of-schema.md(3 hunks)docs/docs/errors.md(5 hunks)docs/docs/examples-of-use/html-templates.md(2 hunks)docs/docs/examples-of-use/rest-apis.md(1 hunks)errors_test.go(1 hunks)i18n/i18n.go(1 hunks)i18n/i18n_test.go(2 hunks)internals/DataProviders.go(3 hunks)internals/Issues.go(3 hunks)internals/contexts.go(5 hunks)internals/pools.go(2 hunks)numbers_int64_test.go(8 hunks)numbers_test.go(8 hunks)numbers_validate_test.go(8 hunks)parsers/zjson/parseJson.go(2 hunks)pointers.go(2 hunks)pointers_test.go(3 hunks)pointers_validate_test.go(1 hunks)slices_test.go(4 hunks)slices_validate_test.go(2 hunks)string_test.go(9 hunks)string_validate_test.go(10 hunks)struct.go(1 hunks)struct_helper_test.go(10 hunks)struct_test.go(4 hunks)struct_validate_test.go(3 hunks)time_test.go(5 hunks)time_validate_test.go(5 hunks)tutils/testIssueMessages.go(1 hunks)utils.go(4 hunks)utilsOptions.go(1 hunks)utilsOptions_test.go(3 hunks)zhttp/zhttp.go(1 hunks)zhttp/zhttp_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- struct_helper_test.go
🧰 Additional context used
🪛 LanguageTool
docs/docs/core-concepts/1-anatomy-of-schema.md
[uncategorized] ~45-~45: “you” seems less likely than “you’re” (you are).
Context: ...the data matches your expected type. If you blinding typecast the data to, for exam...
(AI_HYDRA_LEO_CP_YOU_YOUARE)
[uncategorized] ~45-~45: Possible missing comma found.
Context: ... int and your user provides a string as input you will cause a panic. > _Pure Functio...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~46-~46: Possible missing comma found.
Context: ... a large struct it will copy the entire struct which is not efficient. Be careful! ##...
(AI_HYDRA_LEO_MISSING_COMMA)
docs/docs/errors.md
[typographical] ~9-~9: After the expression ‘for example’ a comma is usually used.
Context: ...rlying error that caused the issue. For example the error generated trying to unmarshal...
(COMMA_FOR_EXAMPLE)
[style] ~80-~80: 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)
🪛 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)
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)
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)
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)
101-101: 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)
107-107: 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)
120-120: Hard tabs
Column: 1
(MD010, no-hard-tabs)
121-121: 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)
132-132: Hard tabs
Column: 1
(MD010, no-hard-tabs)
133-133: Hard tabs
Column: 1
(MD010, no-hard-tabs)
138-138: Hard tabs
Column: 1
(MD010, no-hard-tabs)
139-139: Hard tabs
Column: 1
(MD010, no-hard-tabs)
144-144: Hard tabs
Column: 1
(MD010, no-hard-tabs)
145-145: Hard tabs
Column: 1
(MD010, no-hard-tabs)
150-150: Hard tabs
Column: 1
(MD010, no-hard-tabs)
151-151: Hard tabs
Column: 1
(MD010, no-hard-tabs)
156-156: Hard tabs
Column: 1
(MD010, no-hard-tabs)
157-157: Hard tabs
Column: 1
(MD010, no-hard-tabs)
162-162: Hard tabs
Column: 1
(MD010, no-hard-tabs)
167-167: Hard tabs
Column: 1
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (91)
docs/docs/examples-of-use/rest-apis.md (1)
10-15: Documentation enhancement - clear examples for both Parse and Validate approaches.The additions improve the documentation by clearly showing two different approaches to handling form data:
- Using
Parseto unmarshal and validate in one step- Using a separate unmarshalling step followed by explicit validation with
ValidateThis gives users more flexibility in how they integrate Zog into their existing code.
tutils/testIssueMessages.go (2)
16-22: Updated error property access pattern.The changes correctly replace method calls with direct field access to ZogIssue struct fields, which aligns with the PR objective of converting the Zog issue interface into a struct.
29-30: Updated error message assertion.Consistent with the other changes, error message assertions now use direct field access instead of method calls.
docs/docs/examples-of-use/html-templates.md (2)
25-25: API simplification.Changed from
zhttp.NewRequestDataProvider(r)to the more concisezhttp.Request(r), which makes the API more intuitive and reduces verbosity.
37-37: Updated error property access pattern.Changed from using method calls (
e[0].Message()) to direct field access (e[0].Message), consistent with the PR objective of converting the Zog issue interface into a struct.Also applies to: 42-42
pointers.go (2)
62-67: Improved error handling.The error handling logic has been refined to ensure that the context value (
ctx.Val) is only updated after confirming thatfn()executes without error. This prevents the context from being updated with potentially invalid data.
24-25:Details
✅ Verification successful
Type determination delegation.
Changed from returning a fixed pointer type to delegating the type determination to the underlying schema. This is a significant behavioral change that affects type-related functionality throughout the system.
Let's check how this change impacts other parts of the codebase:
🏁 Script executed:
#!/bin/bash # Search for places where the getType method is called/used rg -A 3 "getType\(\)" --type goLength of output: 5884
Type determination delegation verified
The change in
pointers.go—switching from returning a fixedzconst.TypePtrto delegating type determination viav.schema.getType()—has been verified. The search results confirm that:
- The new delegation pattern is consistent, with
getType()calls inpointers.goand elsewhere now correctly routing through the underlying schema.- Other schema types (e.g., in
boolean.go,numbers.go,slices.go, etc.) continue to use their respective fixed type constants, while the pointer schema now aligns its type with its wrapped schema.- The tests (e.g., in
boolean_test.go,numbers_test.go, and others) validate the expected type outputs, ensuring no breaking changes in downstream functionality.Overall, this delegation is intentional and systematically applied across the codebase. Please ensure that downstream integrations are thoroughly tested, as this change significantly alters the way pointer types are determined.
zhttp/zhttp_test.go (1)
314-314: Field access updated to match ZogIssue struct refactoringThis change reflects the refactoring from accessing error code via a method call (
err.Code()) to directly accessing the field property (err.Code), aligning with the conversion of ZogIssue from an interface to a struct.i18n/i18n.go (1)
26-26: Parameter type updated to use pointer receiverThe parameter type for
IssueFormatterhas been changed from a value type to a pointer type (*internals.ZogIssue). This change is consistent with the refactoring across the codebase, allowing direct modification of the ZogIssue instance.internals/pools.go (2)
34-34: Type renamed from ZogErr to ZogIssueThe object pool now returns
&ZogIssue{}instead of&ZogErr{}, establishing consistency in naming across the codebase as part of the refactoring.
85-85: Type renamed from ZogErr to ZogIssueSimilarly updated in the
ClearPoolsfunction to maintain consistency with the renaming fromZogErrtoZogIssuethroughout the codebase.utilsOptions.go (1)
15-15: Updated to use pointer receiver for ZogIssueChanged the parameter type from
ZogIssueto*ZogIssueto align with the codebase-wide refactoring of treating ZogIssue as a struct with direct field access rather than an interface with methods.boolean_test.go (1)
130-131: Good refactoring: Updated to use Message field directly.The test logic now correctly accesses the error message as a direct property instead of using a method call, consistent with the ZogIssue interface to struct conversion mentioned in the PR objectives.
time_validate_test.go (1)
20-20: Consistent updates to error message access.All assertions now correctly access the
Messagefield directly rather than calling a method. This consistent pattern reflects the ZogIssue interface to struct conversion.Also applies to: 89-89, 101-101, 113-113, 123-123
string_test.go (1)
119-119: Thorough refactoring of error assertions.All error message assertions throughout the test file have been systematically updated to use the direct property access pattern (
errs[0].Messageinstead of method calls), keeping the test functionality identical while aligning with the new struct-based approach.Also applies to: 129-129, 272-272, 286-286, 300-300, 314-314, 324-324, 329-329, 334-334, 343-343, 361-361
slices_test.go (2)
45-45: API simplification: Direct data passing to Parse method.The Parse API has been simplified by allowing direct data passing instead of wrapping it with a provider. This is a good usability improvement that makes the API more intuitive.
Also applies to: 57-57
86-86: Updated error message access pattern.Consistent with other files, error message assertions now use direct property access instead of method calls, maintaining the same behavior while following the new struct-based approach.
Also applies to: 232-232
time_test.go (1)
19-19: Consistent refactoring of error message accessThe changes from
errs[0].Message()toerrs[0].Messagealign with the PR's objective of converting ZogIssue from an interface to a struct, enabling direct field access instead of method calls.Also applies to: 97-97, 107-107, 118-118, 128-128
zhttp/zhttp.go (1)
31-31: Appropriate use of pointers for ZogIssue return typesThe changes from returning
ZogIssueby value to returning*ZogIssuepointers are a good practice that:
- Avoids unnecessary copying of error data
- Allows for mutation of error objects after creation if needed
- Aligns with idiomatic Go error handling patterns
The struct initialization at line 34 also shows the clean syntax benefit of using a struct versus an interface.
Also applies to: 34-34, 40-40
numbers_test.go (2)
22-22: Parameter type correctly updated to pointer receiverChanging the WithIssueFormatter parameter from
ZogIssueto*ZogIssueis appropriate since the function needs to modify the error by callinge.SetMessage(). Using a pointer parameter provides both better performance and necessary mutability.
27-27: Consistent conversion from method calls to field accessAll assertions have been systematically updated from
errs[0].Message()toerrs[0].Message, maintaining consistency with the ZogIssue interface-to-struct conversion throughout the codebase.Also applies to: 30-30, 68-68, 74-74, 80-80, 187-187, 202-202, 217-217, 222-222, 241-241, 256-256, 261-261, 280-280
conf/issueFormatConf.go (1)
26-27: Appropriate parameter type change and field access conversionThe formatter function now correctly:
- Uses
*p.ZogIssuepointer parameter, which is necessary since it modifies the error object- Accesses ZogIssue fields directly instead of through method calls, consistent with the struct conversion
These changes maintain the same functionality while improving the code structure.
Also applies to: 31-32, 37-37, 40-40
numbers_int64_test.go (2)
22-22: Parameter type change is consistent with ZogIssue refactoring.The function parameter has been changed from a value receiver to a pointer receiver, which is consistent with the conversion of ZogIssue from an interface to a struct.
27-27: Accessing Message as a property instead of method call.The change from method calls (
errs[0].Message()) to property access (errs[0].Message) is consistent with the ZogIssue interface being converted to a struct. This pattern is applied consistently throughout all error message checks in this file.Also applies to: 30-30, 57-57, 63-63, 69-69, 175-175, 190-190, 205-205, 210-210, 229-229, 244-244, 249-249, 268-268
string_validate_test.go (1)
125-125: Consistent property access pattern for error messages.All error message assertions have been updated to access
Messageas a property rather than a method call, which aligns with the broader refactoring of the ZogIssue interface into a struct.Also applies to: 134-134, 273-273, 288-288, 303-303, 318-318, 333-333, 344-344, 353-353, 373-373
pointers_validate_test.go (2)
29-29: Parameter type change to pointer receiver for ZogIssue.The function parameter was changed from
ZogIssueto*ZogIssue, consistent with the conversion of ZogIssue from an interface to a struct and enabling in-place modifications.
34-34: Updated error message access pattern.The assertions have been updated to access
Messageas a property rather than a method call, maintaining consistency with the ZogIssue refactoring throughout the codebase.Also applies to: 37-37
docs/docs/core-concepts/1-anatomy-of-schema.md (1)
22-22: Simplified field declaration in documentation.The Markdown link formatting was removed from the field declaration (
tests: []Testinstead of[tests](#tests): []Test), making the example struct definition cleaner and more straightforward.numbers_validate_test.go (2)
22-22: Function parameter type updated to reflect ZogIssue refactoring.The parameter type has been changed from
ZogIssueto*ZogIssuewhich aligns with the refactoring of ZogIssue from an interface to a struct. Using pointers is more efficient when working with structs.
27-27: Direct field access replaces method calls on ZogIssue.Changed from using
errs[0].Message()toerrs[0].Message, which is consistent with the refactoring of ZogIssue from an interface with methods to a struct with fields. This provides more direct and potentially more efficient access to the error message.Also applies to: 30-30, 47-47, 157-157, 173-173, 189-189, 195-195, 216-216, 232-232, 238-238, 259-259
slices_validate_test.go (2)
177-177: Direct field access replaces method calls on ZogIssue.Changed from using
errs["$root"][0].Message()toerrs["$root"][0].Message, which is consistent with the refactoring of ZogIssue from an interface with methods to a struct with fields.
228-229: Renamed from Errors to Issues for more appropriate semantics.Changed references from
Errors.SanitizeListtoIssues.SanitizeList, providing more accurate terminology. This is a good change that improves code readability and reflects the actual purpose of the functionality more clearly.Also applies to: 235-236
conf/issueFormatConf_test.go (3)
13-13: Updated test input type to use pointer to ZogIssue.Changed the test input type from
p.ZogIssueto*p.ZogIssue, aligning with the overall refactoring of ZogIssue from an interface to a struct that's passed by reference.
16-18: Updated test data to use new ZogIssue struct fields.Test cases have been updated to use the new ZogIssue struct with properly named fields (
Code,Dtype,Message) instead of the previous abbreviated names (C,Typ,Msg). This improves code readability and makes the field purposes more obvious.
23-23: Direct field access replaces method calls for ZogIssue.Access pattern changed from
test.input.Message()totest.input.Message, which is consistent with the refactoring of ZogIssue from an interface with methods to a struct with fields.struct_test.go (4)
54-54: API simplification looks good.The change from using
NewMapDataProvider(data)to passingdatadirectly toParsesimplifies the API usage. This is a nice improvement that reduces unnecessary wrapping.
70-70: Consistent API simplification.Similar to the change at line 54, this also simplifies the API by passing
datadirectly toParse.
103-103: Consistent API simplification.Direct parsing of map data without wrapping in a data provider, consistent with previous changes.
188-188: Changed from method call to field access.The change from
errs["$root"][0].Message()toerrs["$root"][0].Messagerepresents the conversion from interface method to struct field access, aligning with the PR objective.i18n/i18n_test.go (3)
96-96: Changed from method call to field access.The change from
nameErrs[0].Message()tonameErrs[0].Messagealigns with the new struct-based approach rather than interface methods.
101-101: Consistent field access pattern.Similar to line 96, this changes from method call
Message()to direct field accessMessage.
123-123: Consistent field access pattern.Changed from
errs[0].Message()toerrs[0].Message, maintaining consistency with the struct-based approach.parsers/zjson/parseJson.go (3)
36-36: Return type changed to pointer for ZogIssue.The return signature has been updated from
(p.DataProvider, p.ZogIssue)to(p.DataProvider, *p.ZogIssue), which aligns with the change from interface to struct. Using a pointer is more efficient for structs.
45-45: Using ZogIssue struct instead of ZogErr.The change from
&p.ZogErr{C: zconst.IssueCodeInvalidJSON, Err: err}to&p.ZogIssue{Code: zconst.IssueCodeInvalidJSON, Err: err}standardizes on a single error type and uses more descriptive field names.
48-48: Consistent use of ZogIssue struct.Similar to line 45, this standardizes on using the ZogIssue struct with descriptive field names.
struct_validate_test.go (3)
139-139: Changed from method call to field access.The change from
errs["num"][0].Code()toerrs["num"][0].Codealigns with the new struct-based approach rather than interface methods.
160-161: Consistent field access pattern.Both error property accesses have been updated from method calls to direct field access (
Code()toCodeandMessage()toMessage), which is consistent with the struct-based approach.
235-235: Consistent field access pattern.Changed from
errs["somefield"][0].Code()toerrs["somefield"][0].Code, maintaining consistency with the struct-based approach.errors_test.go (2)
12-18: Field renaming and type refactoring looks good.The refactoring from
ZogErrtoZogIssuewith more descriptive field names (likeCodeinstead ofC,Messageinstead ofMsg) improves code readability. The change to pointer type (*p.ZogIssue) is consistent with the PR objective of converting the issue interface into a struct.
24-26: Type change is consistent.The change to use
*p.ZogIssueis consistent with the earlier test case and aligns with the overall PR objective.utilsOptions_test.go (5)
20-22: Function signature changed appropriately.The parameter type for the
WithIssueFormatterfunction has been updated to accept a pointer (*p.ZogIssue) instead of a value, which is consistent with the refactoring approach.
24-26: Struct type and field name refactoring.The change from
ZogErrtoZogIssueand fromEPathtoPathfollows better naming conventions and improves readability.
28-28: Direct field access instead of method call.Changed from method call (
err.Message()) to direct field access (err.Message), which is consistent with the conversion from interface to struct.
33-35: Function signature updated consistently.The
MessageFuncparameter type has been updated to use a pointer, which is consistent with the other refactoring changes.
37-38: Direct field access pattern applied consistently.All assertions have been updated to use direct field access (
err[0].Message,err[0].Code) instead of method calls, which is consistent with the interface-to-struct conversion.Also applies to: 46-47, 52-53, 70-72, 77-79
internals/DataProviders.go (2)
8-8: Return type changed to pointer.The
DpFactorytype now returns a pointer toZogIssueinstead of a value. This is a breaking change but is consistent with the PR objective of refactoring the issue interface into a struct.
105-105: Simplified error handling for nil pointers.The error handling for nil pointers has been simplified to just return an
EmptyDataProviderwithout an error message. This is consistent with the improved nil handling approach in this refactoring.pointers_test.go (6)
36-38: Function signature updated consistently.The
WithIssueFormatterfunction has been updated to accept a pointer toZogIssueinstead of a value, maintaining consistency with the refactoring approach.
41-42: Direct field access pattern applied consistently.All assertions have been updated to use direct field access (
.Message,.Code) instead of method calls, which is consistent with the interface-to-struct conversion.Also applies to: 44-45, 92-93, 192-193
80-82: Added new field to test structure.A new field
Value2has been added to theTestStructtype, which enhances test coverage.
85-87: Schema validation enhanced.The schema has been updated to include a
NotNil()constraint for thevaluefield and added validation for the newvalue2field. This improves test coverage.
95-96: Added assertion for new field.An assertion has been added to verify that the
Value2field is correctly initialized to nil. This is consistent with the added field in the test structure.
97-100: Updated test input data.The input map has been updated to include a value for the new
value2field, which properly tests the enhanced schema validation.utils.go (6)
20-21: Refactor awareness: ZogIssue aliasing looks good.Defining the
ZogIssuealias in place of the old types is consistent with the refactor objective. Good job keeping it succinct and clearly documented.
76-81: Good use of Collect after Sanitize.This helper method nicely combines two operations. The approach is straightforward, and there do not appear to be any issues.
83-90: Switching to direct field access looks appropriate.Using
err.Messagedirectly instead of a method call is consistent with the new struct-based approach to issues.
99-104: Smart memory reuse approach.The use of
CollectMapto "free" Zog issues in the map is a good approach to optimize memory usage. Ensure that these freed objects aren't used concurrently to avoid data races.
106-111: Potential concurrency caution.Similar to
CollectMap, ensure thatCollectListusage only occurs under safe conditions to prevent reusing issues still in use elsewhere.
113-115: Confirm concurrency safety in pooled usage.Freeing or pooling
ZogIssueinstances can yield performance benefits, but confirm that there’s no risk of multiple goroutines referencing the same freed struct. If concurrency is possible, a synchronization approach is recommended.docs/docs/errors.md (2)
246-247: Pointer-based signature matches the updated struct approach.Changing
IssueFmtFuncto accept*ZogIssuealigns with your pointer-based refactor. This looks consistent throughout the codebase.
300-301: Custom issue formatter caution.Overriding
conf.IssueFormattercan yield uniform or surprising results. If not done carefully, all issues may receive the same message. Looks correct for advanced usage, but proceed with caution.internals/contexts.go (11)
13-15: Pointer-based methods for error addition.Switching from value to pointer for
NewErrorandAddIssueis consistent with your new struct-based pattern. No issues here.
53-54: Ensure the fallback formatter is always present.In cases where
e.Messageis empty, callingc.Fmter(e, c)is crucial. Double-check thatc.Fmteris never nil to avoid panics.
62-63: Deprecated method usage is well-handled.Using
NewErrorto invoke an internal addition of issues maintains backward compatibility. Good approach.
67-68: No concerns with FmtErr usage.Formatting remains consistent, and the short-circuit check for
e.Message != ""is sensible.
116-125: Pooling approach can be efficient but verify concurrency usage.Grabbing an issue from a pool, resetting its fields, and returning the pointer is efficient. Just ensure all references are local to one goroutine at a time.
130-138: Consistent IssueFromTest logic.Setting the code, path, params, and message to defaults or test-based values is intuitive. This aligns well with pointer semantics.
149-155: Coercion error usage.Creating a brand-new
ZogIssuefor a coercion error is clear. The approach fully resets message, code, and path as intended.
162-163: Reusing the existingZogIssueif possible.Wrapping unknown errors in a new issue while reusing
errif it’s already a *ZogIssue is a neat approach. No concerns here.
175-185: TestCtx integration looks correct.Extracting another
ZogIssuefrom the pool and applying the test’s code and params is consistent with the rest of the refactor.
188-189: Formatter fallback logic.Checks for an existing message, then calls
c.Test.IssueFmtFunc(e, c), and ultimately falls back to the schema’s FmtErr. Good layered design.
201-201: Maintains consistent pointer-based approach.
AddIssue(e *ZogIssue)inTestCtxensures the message is properly formatted before adding to errors. This matches the updated interface.internals/Issues.go (9)
10-10: No concerns with function type alias.
The introduction of thisIssueFmtFunctype alias referencingZogIssueappears straightforward and consistent with the updated struct-based design.
12-15: Good, clear documentation.
The doc comment clearly outlines the purpose and usage ofZogIssue. This helps maintain clarity for future maintainers.
16-34: Ensure sensitive data is handled properly.
While this struct is labeled “safe to expose to the user,” theValue anyfield could inadvertently hold sensitive or PII data. Verify that there are no edge cases where internal or sensitive details leak to user-facing logs or messages.
36-76: Setters and method chaining are implemented consistently.
All setter methods follow a uniform pattern, returning*ZogIssuefor easy chaining. The logic is clear and correct, with no immediate pitfalls or concurrency concerns.
78-92: Error and String methods align with Go’s error interface.
Unwrap(),Error(), andString()methods implement error unwrapping and string formatting as expected, facilitating straightforward error handling.
98-101: Straightforward alias for list of issues.
type ZogIssueList = []*ZogIssueis consistent with the move to pointer-based usage ofZogIssue. No immediate problems identified.
105-105: Interface signature update is coherent.
Accepting*ZogIssuein theAddmethod aligns with the new struct-based approach, preserving consistency across the codebase.
122-122: Appending to the list remains simple and clear.
Storing incoming issues inErrsListis correctly handled. No concurrency or logical issues observed here.
149-164: Logic for marking the first error looks intentional.
Storing the first issue underzconst.ISSUE_KEY_FIRSTcan be helpful for quick access to the lead error. Ensure you have sufficient test coverage confirming that the correct “root” or “first” entries are retained inErrsMap.
1 participant
Summary by CodeRabbit
Refactor
Tests
Documentation
Chores
These updates deliver a more uniform error reporting approach and ensure that both runtime behavior and documentation remain clear and reliable for end-users.