Return type oriented refactor of the "error store" concept

[schneems]

Note that this PR is to my own branch and not upstream. Generally maintainers don't like refactoring PRs, I did this work for my own exploration. If it's useful to the maintainers, great! But it's more of a proof of concept.

What daft does today

Proc-macros should be friends with rust-analyzer source: Oxide and Friends episode about daft.

The way this "niceness" is implemented in daft is that an error accumulator is passed into every function that could error. Then at the end, if there's stuff in that accumulator emit code (if any could be generated) and also emit the errors.

The return types in this situation are either T if errors would not prevent code generation or Option<T> if they could.

Edit (Mar 27th): There are also cases where None is a valid return option in the case of something being ignored/hidden. Because the pattern is to check the presence of errors.has_error() it's difficult to distinguish between "None" was returned because it's marked as ignored, but there's a minor error (like duplicate attributes) versus "We could not generate T so return None, and there are major errors, we should not continue."

What this PR does

This PR relies on syn's built-in ability to accumulate errors syn::Error::combine() and moves to clarify possible return state by updating the return types. The unit of accumulation is now local to functions and the return types tell us about the effect of that function on compilation:

• Errors that block code generation return Result<T, syn::Error>
• Errors that do not block code generation return (T, Option<syn::Error>)
• Errors that may or may not block code generation should return Result<T, Option<syn::Error>, syn::Error>.
  Ok(T, None) indicates no errors
  Ok(T, Some()) indicates an error that did not block code generation.
  Err() indicates code could not generate due to error

To put it another way: A return of Option<syn::Error> indicates a "warning" error while Result<T, syn::Error> indicates an unrecoverable error.

There's still a convenience error accumulator, but it's a very thin wrapper around VecDeque<syn::Error>.

Thoughs

A big upside of this approach (in general) is that it allows error accumulation when the signature is not under direct programmer control. One example is someone implementing syn::parse::Parse which has a signature of fn parse(input: ParseStream<'_>) -> Result<Self>;. I feel it would be easier to apply this pattern to an existing proc-macro codebase.

The big downside of my approach is that error accumulation when the return type is Result<T, syn::Error> becomes social. Someone could use a ? out of habit when they should be collecting and emitting.

What I want

I want to talk about patterns and best practices for error accumulation in proc-macros. Daft presented one paradigm, and this PR presented another. I'm curious what people think (pros and cons of each) and if there are other patterns that might be even better! Probably the best way to have those conversations is on mastodon https://ruby.social/@schneems. Comments below a PR make for horrible conversation/exploration threads.

There's also probably a better pattern for the inside of DiffFields::new that I refactored, LMK if you've got something with the same behavior but more concise.

Terms and conditions

Commits are mostly checkpoints where code compiled, if I were to do this "properly" I would probably go back and re-do the work in a more systematic fashion. This was "change some code, get it to compile, commit" with little thought for consistency.