migration guide for proc-macro-error users #7
Comments
|
Sure! Would happily accept a contribution to that effect! |
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 |
|
@SergioBenitez I opened dtolnay/syn#1520, which I hope will lead to enlightenment. The rest of the usage of 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 |
|
I think that's the opposite of what's needed? The function wants to return a |
|
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 |
|
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: Note that unlike in dtolnay/syn#1520 I've used |
|
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 Here's the start to doing so. Note that it doesn't use 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 |
|
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The text was updated successfully, but these errors were encountered: