migration guide for proc-macro-error users
Opened this issue · 13 comments
proc-macro-error has some extremely popular dependents but appears to be abandoned with no activity in the past year and 3 separate merge requests updating its syn dependency. It would be neat to help users of that crate migrate to this one.
Sure! Would happily accept a contribution to that effect!
I tried to use this in test-case but could not get nice errors working there.
Hmm, what do you mean by that, specifically?
They have compile tests that I could not get passing when I replaced proc-macro-error with proc-macro2-diagnostics. Actually the immediately problem was:
AFAICT there's nothing in proc-macro2-diagnostics that can emit a nice error there. I tried using syn::ParseBuffer::error but that seemed to end up discarding the custom error - I just saw a generic parsing error.
@SergioBenitez I opened dtolnay/syn#1520, which I hope will lead to enlightenment. The rest of the usage of proc-macro-error was trivial and had no test coverage:
diff --git a/crates/test-case-core/Cargo.toml b/crates/test-case-core/Cargo.toml
index ba15c27..e314fe4 100644
--- a/crates/test-case-core/Cargo.toml
+++ b/crates/test-case-core/Cargo.toml
@@ -24,6 +24,6 @@ path = "src/lib.rs"
[dependencies]
cfg-if = "1.0"
proc-macro2 = { version = "1.0", features = [] }
-proc-macro-error = "1.0"
+proc-macro2-diagnostics = "0.10"
quote = "1.0"
syn = { version = "2.0", features = ["full", "extra-traits"] }
diff --git a/crates/test-case-core/src/complex_expr.rs b/crates/test-case-core/src/complex_expr.rs
index 2a4334e..08a7637 100644
--- a/crates/test-case-core/src/complex_expr.rs
+++ b/crates/test-case-core/src/complex_expr.rs
@@ -1,5 +1,6 @@
use crate::utils::fmt_syn;
use proc_macro2::Group;
+use proc_macro2::Span;
use proc_macro2::TokenStream;
use quote::{quote, TokenStreamExt};
use std::fmt::{Display, Formatter};
@@ -450,7 +451,11 @@ fn regex_assertion(expected_regex: &Expr) -> TokenStream {
fn not_assertion(not: &ComplexTestCase) -> TokenStream {
match not {
ComplexTestCase::Not(_) => {
- proc_macro_error::abort_call_site!("multiple negations on single item are forbidden")
+ use proc_macro2_diagnostics::SpanDiagnosticExt as _;
+
+ Span::call_site()
+ .error("multiple negations on single item are forbidden")
+ .emit_as_expr_tokens()
}
ComplexTestCase::And(cases) => negate(and_assertion(cases)),
ComplexTestCase::Or(cases) => negate(or_assertion(cases)),
diff --git a/crates/test-case-macros/Cargo.toml b/crates/test-case-macros/Cargo.toml
index 79a2513..8301521 100644
--- a/crates/test-case-macros/Cargo.toml
+++ b/crates/test-case-macros/Cargo.toml
@@ -24,7 +24,7 @@ path = "src/lib.rs"
[dependencies]
proc-macro2 = { version = "1.0", features = [] }
-proc-macro-error = "1.0"
+proc-macro2-diagnostics = "0.10"
quote = "1.0"
syn = { version = "2.0", features = ["full", "extra-traits", "parsing"] }
test-case-core = { version = "3.2.1", path = "../test-case-core", default-features = false }
diff --git a/crates/test-case-macros/src/lib.rs b/crates/test-case-macros/src/lib.rs
index b598095..7d95fea 100644
--- a/crates/test-case-macros/src/lib.rs
+++ b/crates/test-case-macros/src/lib.rs
@@ -22,7 +22,6 @@ use test_case_core::{TestCase, TestMatrix};
/// When _expected result_ is provided, it is compared against the actual value generated with _test body_ using `assert_eq!`.
/// _Test cases_ that don't provide _expected result_ should contain custom assertions within _test body_ or return `Result` similar to `#[test]` macro.
#[proc_macro_attribute]
-#[proc_macro_error::proc_macro_error]
pub fn test_case(args: TokenStream, input: TokenStream) -> TokenStream {
let test_case = parse_macro_input!(args as TestCase);
let mut item = parse_macro_input!(input as ItemFn);
@@ -51,7 +50,6 @@ pub fn test_case(args: TokenStream, input: TokenStream) -> TokenStream {
/// is not allowed for `test_matrix`, because test case names are auto-generated from the test body
/// function name and matrix values.
#[proc_macro_attribute]
-#[proc_macro_error::proc_macro_error]
pub fn test_matrix(args: TokenStream, input: TokenStream) -> TokenStream {
let matrix = parse_macro_input!(args as TestMatrix);
let mut item = parse_macro_input!(input as ItemFn);
Does that look right? Still, it's not clear how to use proc-macro2-diagnostics with syn to emit custom parsing errors (and it's not clear that syn has an answer to this either).
Does that look right? Still, it's not clear how to use
proc-macro2-diagnosticswithsynto emit custom parsing errors (and it's not clear thatsynhas an answer to this either).
proc-macro2-diagnostics::Diagnostic implements From<syn::Error>.
Does that look right? Still, it's not clear how to use
proc-macro2-diagnosticswithsynto emit custom parsing errors (and it's not clear thatsynhas an answer to this either).
proc-macro2-diagnostics::DiagnosticimplementsFrom<syn::Error>.
I think that's the opposite of what's needed? The function wants to return a syn::Error.
Yeah, I understood what you're saying, but it seems like we want the inverse - the context in which proc-macro-error is currently used wants to emit a syn Error. But I see there's also the inverse implementation which I'll try now.
Perhaps unsurprisingly, I get the same result as I described in dtolnay/syn#1520.
diff --git a/crates/test-case-core/src/complex_expr.rs b/crates/test-case-core/src/complex_expr.rs
index e708f2e..06f1151 100644
--- a/crates/test-case-core/src/complex_expr.rs
+++ b/crates/test-case-core/src/complex_expr.rs
@@ -1,5 +1,6 @@
use crate::utils::fmt_syn;
use proc_macro2::Group;
+use proc_macro2::Span;
use proc_macro2::TokenStream;
use quote::{quote, TokenStreamExt};
use std::fmt::{Display, Formatter};
@@ -241,6 +242,8 @@ impl ComplexTestCase {
}
fn parse_single_item(input: ParseStream) -> syn::Result<ComplexTestCase> {
+ use proc_macro2_diagnostics::SpanDiagnosticExt as _;
+
Ok(if let Ok(group) = Group::parse(input) {
syn::parse2(group.stream())?
} else if input.parse::<kw::eq>().is_ok() || input.parse::<kw::equal_to>().is_ok() {
@@ -323,11 +326,11 @@ impl ComplexTestCase {
expected_regex: input.parse()?,
})
} else {
- proc_macro_error::abort!(input.span(), "'with-regex' feature is required to use 'matches-regex' keyword");
+ return Err(input.span().error("'with-regex' feature is required to use 'matches-regex' keyword").into());
}
}
} else {
- proc_macro_error::abort!(input.span(), "cannot parse complex expression")
+ return Err(input.span().error("cannot parse complex expression").into());
})
}
}
diff --git a/crates/test-case-macros/src/lib.rs b/crates/test-case-macros/src/lib.rs
index b598095..7d95fea 100644
--- a/crates/test-case-macros/src/lib.rs
+++ b/crates/test-case-macros/src/lib.rs
@@ -22,7 +22,6 @@ use test_case_core::{TestCase, TestMatrix};
/// When _expected result_ is provided, it is compared against the actual value generated with _test body_ using `assert_eq!`.
/// _Test cases_ that don't provide _expected result_ should contain custom assertions within _test body_ or return `Result` similar to `#[test]` macro.
#[proc_macro_attribute]
-#[proc_macro_error::proc_macro_error]
pub fn test_case(args: TokenStream, input: TokenStream) -> TokenStream {
let test_case = parse_macro_input!(args as TestCase);
let mut item = parse_macro_input!(input as ItemFn);
@@ -51,7 +50,6 @@ pub fn test_case(args: TokenStream, input: TokenStream) -> TokenStream {
/// is not allowed for `test_matrix`, because test case names are auto-generated from the test body
/// function name and matrix values.
#[proc_macro_attribute]
-#[proc_macro_error::proc_macro_error]
pub fn test_matrix(args: TokenStream, input: TokenStream) -> TokenStream {
let matrix = parse_macro_input!(args as TestMatrix);
let mut item = parse_macro_input!(input as ItemFn);
produces a test failure:
---- features_produce_human_readable_errors stdout ----
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot file: tests/snapshots/rust-stable/acceptance__features_produce_human_readable_errors.snap
Snapshot: features_produce_human_readable_errors
Source: tests/acceptance_tests.rs:140
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Expression: output
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
0 │-error: 'with-regex' feature is required to use 'matches-regex' keyword
1 │-error: could not compile `features_produce_human_readable_errors` (lib test) due to previous error
0 │+error: could not compile `features_produce_human_readable_errors` (lib test) due to previous error
1 │+error: unexpected token
────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
To update snapshots run `cargo insta review`
Stopped on the first failure. Run `cargo insta test` to run all snapshots.
Note that unlike in dtolnay/syn#1520 I've used input.span().error(....).into(), yet the result is the same.
I took a close look at your crate. The issues stem from parsers that don't properly propagate errors. It should suffice to fix your parsers, and doing so properly would likely mean you don't need to use proc-macro2-diagnostics (or any other such crate) at all.
Here's the start to doing so. Note that it doesn't use proc-macro2-diagnostics in the ComplexTestCase parser at all:
diff --git a/crates/test-case-core/Cargo.toml b/crates/test-case-core/Cargo.toml
index a6e1166..58fc2e3 100644
--- a/crates/test-case-core/Cargo.toml
+++ b/crates/test-case-core/Cargo.toml
@@ -24,6 +24,6 @@ path = "src/lib.rs"
[dependencies]
cfg-if = "1.0"
proc-macro2 = { version = "1.0", features = [] }
-proc-macro-error = { version = "1.0", default-features = false }
quote = "1.0"
syn = { version = "2.0", features = ["full", "extra-traits"] }
+proc-macro2-diagnostics = "0.10"
diff --git a/crates/test-case-core/src/complex_expr.rs b/crates/test-case-core/src/complex_expr.rs
index e708f2e..28d382a 100644
--- a/crates/test-case-core/src/complex_expr.rs
+++ b/crates/test-case-core/src/complex_expr.rs
@@ -1,6 +1,6 @@
use crate::utils::fmt_syn;
-use proc_macro2::Group;
-use proc_macro2::TokenStream;
+use proc_macro2::{Group, TokenStream, Span};
+use proc_macro2_diagnostics::SpanDiagnosticExt;
use quote::{quote, TokenStreamExt};
use std::fmt::{Display, Formatter};
use syn::parse::{Parse, ParseStream};
@@ -316,18 +316,19 @@ impl ComplexTestCase {
ComplexTestCase::Empty
} else if input.parse::<kw::matching_regex>().is_ok()
|| input.parse::<kw::matches_regex>().is_ok()
{
cfg_if::cfg_if! {
if #[cfg(feature = "with-regex")] {
ComplexTestCase::Regex(Regex {
expected_regex: input.parse()?,
})
} else {
- proc_macro_error::abort!(input.span(), "'with-regex' feature is required to use 'matches-regex' keyword");
+ let msg = "'with-regex' feature is required to use 'matches-regex' keyword";
+ return Err(input.error(msg));
}
}
} else {
- proc_macro_error::abort!(input.span(), "cannot parse complex expression")
+ return Err(input.error("cannot parse complex expression"));
})
}
}
@@ -450,7 +451,9 @@ fn regex_assertion(expected_regex: &Expr) -> TokenStream {
fn not_assertion(not: &ComplexTestCase) -> TokenStream {
match not {
ComplexTestCase::Not(_) => {
- proc_macro_error::abort_call_site!("multiple negations on single item are forbidden")
+ Span::call_site()
+ .error("multiple negations on single item are forbidden")
+ .emit_as_item_tokens()
}
ComplexTestCase::And(cases) => negate(and_assertion(cases)),
ComplexTestCase::Or(cases) => negate(or_assertion(cases)),
diff --git a/crates/test-case-core/src/test_case.rs b/crates/test-case-core/src/test_case.rs
index 105d421..1c5e022 100644
--- a/crates/test-case-core/src/test_case.rs
+++ b/crates/test-case-core/src/test_case.rs
@@ -18,7 +18,7 @@ impl Parse for TestCase {
fn parse(input: ParseStream) -> Result<Self, Error> {
Ok(Self {
args: Punctuated::parse_separated_nonempty_with(input, Expr::parse)?,
- expression: input.parse().ok(),
+ expression: Some(input.parse()?),
comment: input.parse().ok(),
})
}
diff --git a/crates/test-case-macros/src/lib.rs b/crates/test-case-macros/src/lib.rs
index b598095..7d95fea 100644
--- a/crates/test-case-macros/src/lib.rs
+++ b/crates/test-case-macros/src/lib.rs
@@ -22,7 +22,6 @@ use test_case_core::{TestCase, TestMatrix};
/// When _expected result_ is provided, it is compared against the actual value generated with _test body_ using `assert_eq!`.
/// _Test cases_ that don't provide _expected result_ should contain custom assertions within _test body_ or return `Result` similar to `#[test]` macro.
#[proc_macro_attribute]
-#[proc_macro_error::proc_macro_error]
pub fn test_case(args: TokenStream, input: TokenStream) -> TokenStream {
let test_case = parse_macro_input!(args as TestCase);
let mut item = parse_macro_input!(input as ItemFn);
@@ -51,7 +50,6 @@ pub fn test_case(args: TokenStream, input: TokenStream) -> TokenStream {
/// is not allowed for `test_matrix`, because test case names are auto-generated from the test body
/// function name and matrix values.
#[proc_macro_attribute]
-#[proc_macro_error::proc_macro_error]
pub fn test_matrix(args: TokenStream, input: TokenStream) -> TokenStream {
let matrix = parse_macro_input!(args as TestMatrix);
let mut item = parse_macro_input!(input as ItemFn);In particular, this change results in properly propagating the error you were failing to see:
fn parse(input: ParseStream) -> Result<Self, Error> {
Ok(Self {
args: Punctuated::parse_separated_nonempty_with(input, Expr::parse)?,
- expression: input.parse().ok(),
+ expression: Some(input.parse()?),
comment: input.parse().ok(),
})
}It also breaks the expected parse in many cases, of course. This is the "fix your parsers" part of things. As it stands, the crate abuses the "abort" behavior of proc-macro-error to get away with sloppy parsing.
Thanks, dtolnay pointed the same thing out. I filed frondeus/test-case#135 since it's not my crate and the fix isn't obvious.
Update: frondeus/test-case#136 (which moves from proc-macro-error to this crate) has been merged.