Feat/serializer

[lfbrehm]

Introduces missing macros mentioned in #32 and implements Serializer to address issue #15.

[lfbrehm]

Thanks for the feedback!

I should probably mention that the implementation of Serializer is more or less copied from serde_json as mentioned in the serde documentation. They use a pretty similar Value enum, so this was straightforward. I also added a disclaimer to interpreter/src/ser.rs.

1. I implemented the TryToValue trait in the new commit. It's probably just a preference, but I'd rather call to_value and get a separate SerializationError than serialize everything implicitly in add_variable, also because this error is specific to serialization and not adding variables.
2. I forgot to add the serde-specific variant to this error, should be fixed now.
3. I also added a new variant to ExecutionError to forward the error, but I am not happy with how I have approached error handling in general here.

I am open to any improvements and suggestions you may have from your side!

[clarkmcc]

@lfbrehm Thanks! First of all, I don't consider myself a Rust expert, so if there's an idiom that I'm not considering, I'm definitely happy to consider it. I found several cases[1] of functions accepting T: Serialize and returning errors so there's at least some precedence for this sort of thing, though our use-case here may not be entirely comparable.

My personal thought process was: if you still have to serialize and error handle, is it simpler on the user to do that all in one step rather than requiring them to import a conversion function.

let value = to_value(MyStruct{})?;
ctx.add_variable("value", value);

// vs

ctx.add_variable("value", MyStruct{})?;

In summary, if it's only a personal preference thing, then I would prefer the approach that requires less cognitive overhead for users that aren't familiar with the library (one of the big reasons I implemented #11). If there's an idiom that we'd be ignoring by implementing it this then I think I would prefer the idiom over my preference.

[1] https://sourcegraph.com/search?q=context:global+/T:+Serialize%3E%5C(/&patternType=keyword&sm=0

[clarkmcc]

I'll poke around a bit on the error handling and see if I can come up with something that makes sense. I agree that ctx.add_variable(...) should not return an ExecutionError, but I don't have a problem with it returning a SerializationError since that is the only way that function can fail.

[St4NNi]

Hey, first of all, thanks @lfbrehm for this PR. Since I was a bit involved in the creation of this PR behind the scenes, I figured I might be able to add some of my thoughts here as well:

I think there should definitely be an infallible function to add an existing Value to a context, otherwise it introduces unnecessary verbosity via execution errors that will most likely never occur (e.g. in all, exists and friends).

IMHO the best solution would be to add an additional add_variable_from_serialize function to the context, which would take any T: Serialize and convert it to a value, and then call the infallible add_variable. This would avoid the problem that users have to guess which function to import, and would only add slightly more verbosity. It would also preserve the current function signatures and keep the add_variable function backwards compatible. Additionally, it would make the function signatures easier to understand, for Into<Value> and T: Serialize most people have a clear understanding of what to expect.

[clarkmcc]

Yeah I'm comfortable with that.

I'd also be fine with doing the inverse of what you suggested, and introduce a new function name add_variable_from_value or something that's more geared towards internal usage. The idea here is the shorter/cleaner function (add_variable) is the one that should be used my users in most cases. It wouldn't be backwards compatible as you say, but we're pre-1.0 so backwards compatibility isn't a guarantee yet. If one API is better than the other, now would be the time for a breaking change in favor of a better API.

But as I said, if you guys would prefer to leave add_variable unchanged and introduce a new function, I would be good with that as well.

[lfbrehm]

I think your suggestion to add add_variable_from_value is the best step forward. This introduces an internal function to add already converted variables to the context without any error handling and could serve as a public function for users dealing only with values. Exposing an add_variable function, that takes serializable values is then the 'default' that introduces convenience for users. Error handling then is also intuitive, since ExecutionErrors are not mixed with SerializationErrors.

Feel free to add any suggestions you have for this new commit!