cedar-policy / cedar

Implementation of the Cedar Policy Language
https://www.cedarpolicy.com
Apache License 2.0
897 stars 80 forks source link

`api::ValidationResult` implements error but `diagnostics::ValidationResult` doesn't #1158

Open oflatt opened 3 months ago

oflatt commented 3 months ago

Describe the improvement you'd like to request

api::ValidationResult is a wrapper for diagnostics::ValidationResult that implements Diagnostic and Error. However, this makes things awkward internally since diagnostics::ValidationResult doesn't have an error implementation.

I propose we move the implementation of error and diagnostic to the inner diagnostics::ValidationResult.

Describe alternatives you've considered

No response

Additional context

No response

Is this something that you'd be interested in working on?

john-h-kastner-aws commented 3 months ago

Make sense. IIRC, there were some issue with the wrapper types for the public API that made this the convenient way to do things. You can add it to the internal with a bit of duplicate code. Feel free to refactor if you see a nicer way to do it.