-
Notifications
You must be signed in to change notification settings - Fork 75
Add -typederrors flag for typed enum conversion errors #112
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
Conversation
This comment was marked as spam.
This comment was marked as spam.
|
Thank you for your review @bincyber I plan on rebasing and adding you as a contributor for your efforts here unless you object. I think in addition to the change to a single shared definition of this error it should be named more idiomatically, if I'm remembering correctly that means |
Sounds great |
02356a8 to
a8b6e3c
Compare
|
I went to add this error globally and found it can't be at the top of the repo because that's where the tool code lives and we would have to make enumer itself a depdendency of the generated code. I was able to sort an implementation of the change using One alternative for having the error in scope without adding a dependency is to de-duplicate: we could write just a separate file with the error type (typeS in the future) and check if it's there before writing again from another type being generated. This has downsides like the errors could be duplicated across a codebase in different paths and even just different levels of hierarchical data structures. It would also be a more significant refactor. I think the path of a new package space inside enumer, github.com/dmarkham/enumer/errs is the least-friction path ahead and because this addition of a generated non-standard-library dependency is behind a flag that's newly added I don't think it represents the concern it felt like when I uncovered that requirement. I also need to remember to update the README with this new flag |
2296974 to
6678004
Compare
6678004 to
0bba577
Compare
0bba577 to
d8a6155
Compare
bincyber
left a comment
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.
🚢
|
One last thing: like #111 I think this deserves a "golden" test for the generated return error |
701fa38 to
2d32fa8
Compare
477c248 to
95f9918
Compare
|
because this is an addition but is backwards compatible, I think the new tag should be version 1.6.0 |
Co-authored-by: @bincyber <bincyber@users.noreply.github.com>
95f9918 to
599be04
Compare
|
I keep thinking about how to test this further and because the referenced error is in this change, I can't do too much. I know it's working because of the testing, and I would have noticed the other .golden test files getting broken if it broke backwards compatibility. I'm just going to merge it. |
Summary
Implements the typed errors feature discussed in #111 using the
errors.Join()approach. This provides type-safe error checking while maintaining full backwards compatibility.Key Features
-typederrorsflag: Optional flag that enables typed error generation (defaults to false)enumerrs.ErrInvalidValuesentinel error for type-safe checkingerrors.Join()approach: Combines typed error with descriptive messagesImplementation Details
-typederrors: Returns joined typed errorerrors.Join(enumerrs.ErrInvalidValue, fmt.Errorf("..."))fmt.Errorf(...)(unchanged behavior)Test Plan
errors.Is()🤖 Generated with help from Claude Code