search

amazon-ion / ion-schema-rust

Rust implementation of Ion Schema
https://amazon-ion.github.io/ion-schema/sandbox
Apache License 2.0
13 stars 6 forks source link

Add more fields to `IonSchemaError` to be used for `PartialEq` #141

Open desaikd opened 1 year ago

desaikd commented 1 year ago

I'm a little uneasy about having the equality function test only for description string equality, which can be pretty fragile. Developers are likely to write unit tests against a given error string, which means we can't change that error string down the road without also breaking their tests.

We do description string comparisons like this in ion-rust's IonError type, but the description messages are intentionally terse and do not include lots of contextual information like position. Even there I don't love it.

I suppose ideally we'd have fields besides the description that characterized the nature of the failure. For example, a schema_name field in each UnresolvableSchemaError, or an enum of reasons that an InvalidSchemaError occurred. Well-defined, tightly-scoped fields that we're comfortable committing to using in our PartialEq impl.

This doesn't need to block this PR, but we should think it through before 1.0.

_Originally posted by @zslayton in https://github.com/amazon-ion/ion-schema-rust/pull/139#discussion_r1084512034_

dlurton commented 1 year ago

You can probably just make an impl of PartialEq that ignores the description, and possibly other fields in that case. (Although I would definitely include the position of the error within the document being validated (not within the ISL).