feat(remap): error-return-value error handling pattern

[JeanMertz]

Another step towards solving our error-handling story as described in #5479 and this gist.

You can now make the rhs expression in an assignment infallible by capturing the error message:

// fallible, will be rejected at compile-time in the future
json = parse_json(...)

// infallible
json, err = parse_json(...)

// infallible, rejected at compile-time since the `err` case will always be null
five, err = 5

err is null if the rhs expression succeeded, otherwise it contains the error message as a string.

This joins the existing error-handling strategies introduced in #5876 and #5830.

note: this does not yet implement the "ignore-target" (e.g. json, _ = parse_json(...)), I'll create a follow-up issue to add that, as it's not strictly needed for this to be useful.

closes #5865.

[JeanMertz]

Ah wait, I just realised I forgot to add one more feature to this PR.

I want to make this pattern result in a compile-time error:

five, err = 5

That is to say, if a rhs expression is known to be infallible, we want to reject error assignments, since it only confuses readers and will only ever result in err being set to null.

[JeanMertz]

Alright, this one is good to review at this point. cc @jszwedko @FungusHumungus.

[JeanMertz]

I left a comment in #4803 (comment) to further improve this error message sometime next week when we're going to tackle compiler error messages.

[JeanMertz]

I added this to make it easier to write some of our tests, that don't care about the return value.

Eventually, if/when VRL is its own project (it is, but it's still in the Vector repo right now), we'll change this so that Vector either handle regex return values, or we raise an error in Vector itself instead of here.

But the TL;DR is: please ignore this for now 😄

[StephenWakely]

I think we could take fuller advantage of Rusts type system here to make illegal states unrepresentable.

pub enum Target {
  Path(Path),
  Variable(Variable)
}

pub enum AssignmentTarget {
  Fallible { ok: Box<Target> },
  Infallible { ok: Box<Target>, err: Box<Target> }
}

This makes it logically impossible to specify nested Infallible targets. It is much clearer what this code is expecting. Any bugs there will just be caught quickly by the compiler and we won't need all the unimplemented! calls below.

[JeanMertz]

I thought about doing that, but I didn't see the value in this specific case (I'm not averse to using unimplemented, since it's covered by the initializer and the parser, so if you ever change this, many tests would fail).

Would you be okay with me filling this as an issue for a future improvement, so that I can keep the pace up for 0.12 (this also requires some re-jiggling in the parser and module exporting, etc)?

[StephenWakely]

Sure!

[JeanMertz]

Thanks, tracking in #5940.

[StephenWakely]

I think it would be much clearer to reorganise those Structs a bit.

[JeanMertz]

Moving this into master, but still happy to get your feedback @jszwedko, I'll process them in a follow-up PR if needed.