Improve error positioning of SemanticError #70
Conversation
dsnet
force-pushed
the
semantic-error
branch
5 times, most recently
from
fc4de8d to
a37b36b
Compare
| // The randomization is tied to the Hyrum-proofing already applied | ||
| // on map iteration in Go. |
There was a problem hiding this comment.
Is relying on this hyrum-proofing actually hyrum-busting 🤔😂? Seriously though, did we mention this hyrum proofing attempt as a design goal of v2 in the design discussion? I know some protojson users who grumble about this behavior. Not to say that I don't think it's worth doing but, just wondering if there's been any discussion already.
There was a problem hiding this comment.
It's always easier to remove hyrum proofing than it is to add it.
Personally, I'd be okay with the protobuf module now removing the hyrum proofing 4 years in, but it was necessary during the initial years trying to migrate to the new module and avoid backsliding into bad testing practices.
In a similar way, I think we should still have the hyrum proofing here while the the error output is still in flux. This PR makes significant improvements in finalizing what it might look like, but I'm certain we'll keep making tweaks over the upcoming year or two after initial release.
dsnet
force-pushed
the
semantic-error
branch
from
a37b36b to
637bd95
Compare
Changes made: * Always populate SemanticError.JSONPointer. * Export ErrUnknownName to programmatically identify an unknown name. * Reduce verbosity of SemanticError.Error method. * Apply Hyrum-proofing on a per-process basis, rather than on a per-error.Error method call. * Cleanup error wrapping for user-provided method and function calls. Some heuristics were used to provided reasonable position injection into user-provided errors.
dsnet
force-pushed
the
semantic-error
branch
from
637bd95 to
a491af2
Compare
|
Thank you so much for this |
Changes made: