Use trybuild and move to proc-macro-error

[CreepySkeleton]

This PR is on top of #59

Deleted files

• diag.rs: no need for it anymore

  I also got rid of emit_with_attr_note, in my opinion it's just useless noise. If people have made use of #[auto_impl] attribute they already know where it was requested, user wants to know where the error is.

• spanned.rs: no need either. To be honest, your implementation was no better than the one from proc_macro2 (if not worse). Anyway, you don't need proc_macro::Span anymore - proc-macro-error works with proc-macro2::Span.

I also added helpful notes where applicable, made some refactoring here and there (not much), fixed some typos.

I didn't touch some panic! invocations - they look like bug catching and this is just the job of panic. Some other panics were replaced with abort!.

Btw, nightly feature from proc_macro2 doesn't really do anything, I suggest to remove it.

Also, I would like to propose not to use Trybuid::compile_pass - it literally executes a separate cargo build run for every each file which is slow.

[LukasKalbertodt]

Thanks for your PR!

I will review this in detail later. I just quickly looked over it now. A request up front: could you make multiple commits out of that? For example, three:

• Refactoring/small fixes unrelated to the main goal of this PR
• The main implementation of this PR (use proc-macro-error)
• Adjusting the UI tests

But you can also create more commits if you think it makes sense.

And in general, please only include style changes that are justified by the official Rust styleguide (i.e. what rustfmt does).

[LukasKalbertodt]

Please use the old extra-braces syntax: match _ { Ident(ident) => { match .... I doubt this change was made by rustfmt?

[CreepySkeleton]

rustfmt is the one to blame here. I would argue that the current approach is somewhat cleaner though, what do you think?

[CreepySkeleton]

And in general, please only include style changes that are justified by the official Rust styleguide (i.e. what rustfmt does).

I didn't do any style adjustments except that one match () => if, the blame is on rustfmt entirely :)

[CreepySkeleton]

Here's an attempt to integrate rustfmt and make it match your code style. Unfortunately, those options are nightly, but who cares?

[LukasKalbertodt]

Awesome work, thank you! And sorry for being so picky, especially regarding style :P

I'm pretty busy ATM, so I only managed to review ae0ce6c, but this is the "main" one.

As a general thing: in many places (especially gen and proxy) you write emit_error! where we immediately return Err(()) afterwards. In these cases it would make sense to just abort! instead, right? I just quickly checked and it seems like the Err value is never really used. So I think it might be possible to replace all emit_error!s in gen and proxy with abort and also make all of those functions return T instead of Result<T,()> (for whatever T...). I think that should simplify things even further.

Thanks again for your work. In this commit it sure looks worth it to switch to your crate!

[CreepySkeleton]

Of course it would be fine to use abort! only but in most cases it's quite useful to keep working, looking for other errors to report. I tweaked the code in a way that functions return as late as possible, take a look.

[CreepySkeleton]

knock knock @LukasKalbertodt

[LukasKalbertodt]

@CreepySkeleton Please excuse the delay, I have plenty of other stuff to do :(
But I have not forgotten this and planned on completely reviewing this PR this week still!

[LukasKalbertodt]

Okay, I finally managed to get this reviewed. Sorry again for the delay.

Overall I'm pretty happy with this PR. I think it does indeed improve the code base and makes many parts of the code more readable. Not to mention that this PR removes 100 lines more than it adds, which is always a plus. I left a few inline comments still, that I'd like to see fixed.

One additional thing: could you please remove the two last commits (the ones about rustfmt) from this PR? I don't mind adding rustfmt, but I think it's pretty separate from the main concern of this PR, so we should keep PRs kind of atomic.

Oh, and thanks a bunch for separating all changes into multiple commits. That really made it way easier to review!

With my comments fixed, I think this can be merged.

[CreepySkeleton]

Ugh, I must admit I completely forgot about this PR, sorry about it @LukasKalbertodt :(
In my defense, nobody tried to ping me anyway.

I fixed all of the suggestions above If you sill have an appetite for this changes.

[LukasKalbertodt]

No problem for the delay ;-)
Thanks for fixing everything! Great work with this PR.