Consider using compile_error! instead of panicking

[msrd0]

I just tried out askama for the first time and my IDE (using rust-analyzer) reported that my code failed to compile because proc-macro derive panicked. This error message is not very helpful as I had to go to the command line and ask cargo what the real problem was. If the derive macro issued a compile_error!(msg) instead of panicking, the error message would've been correctly reported by rust-analyzer. This is also already built into syn to make life easier.

[djc]

The simple approach using what syn provides is not enough because many of the potential panics originate from the template parser, which doesn't process Rust code as such. Still, it would surely be nice to use something better than panic!(), it just hasn't been much of a priority for me. In the same space, I've also seen https://lib.rs/crates/proc-macro-error which looks interesting.

[msrd0]

I agree that the spans cannot point to the template code (which I assume is what you mean with not good enough) as that's not processed Rust code, but the error message reported by tools like rust-analyzer would be much more useful if compile_error! would be used instead of panicking.

[djc]

Can you explain that with an example?

[msrd0]

Sure. Assuming you confuse programming languages and write #[template(path = "login.rs")] when you ment to write login.html, you get the following output in cargo check:

error: proc-macro derive panicked
 --> src/routing/mod.rs:4:10
  |
4 | #[derive(Template)]
  |          ^^^^^^^^
  |
  = help: message: template "login.rs" not found in directories ["/somewhere/on/my/computer"]

However, rust-analyzer (invoked via lsp-mode) does not print the help message, so I'm left with this in my IDE:

If compile_error!(template "login.rs" not found) would've been used instead (which proc-macro-error seems to be doing too), the helpful part of the error message would've been shown in the IDE and I wouldn't have had to run cargo check manually to see what's wrong.

---

However, if there was a syntax error in the template, I'd still get an error message pointing to the macro invocation:

error: proc-macro derive panicked
 --> src/routing/mod.rs:4:10
  |
4 | #[derive(Template)]
  |          ^^^^^^^^
  |
  = help: message: unable to parse template:
          
          "{% hello world %}"

If spans outside of the derive input were supported (which I don't think is even planned, but I might be wrong) the output could look like this:

error: unable to parse template
 --> templates/login.html:1:1
  |
1 | {% hello world %}
  | ^^^^^^^^^^^^^^^^^
  |
note: proc-macro invoked here
 --> src/routing/mod.rs:4:10
  |
4 | #[derive(Template)]
  |          ^^^^^^^^
  |

which would be ideal but I believe is currently unachievable.

[djc]

Ok, I believe this is substantially improved by #373. Let me know if I got anything wrong!

[msrd0]

No, unfortunately it did not, I now get

error: macros that expand to items must be delimited with braces or followed by a semicolon

followed by another error

error: proc-macro derive produced unparseable tokens

[djc]

Okay, would you mind making a PR? It should be straightforward to fix from here:

https://github.com/djc/askama/blob/main/askama_derive/src/lib.rs#L17

[djc]

I don't know how compile errors are tested, but it would be ideal to have a test for this, too.

[msrd0]

Sure, I've used the trybuild crate for testing error messages, I can setup some test cases if you have a list of possible error sources maybe?

[djc]

That's a long list... See the changes in #373. But really I think the integration test is the most important part, so maybe just add one test that tests the case that you started with -- trying to parse a Rust file instead of a template?