Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syn::Result::ok silently discards errors #135

Open
tamird opened this issue Oct 16, 2023 · 0 comments
Open

syn::Result::ok silently discards errors #135

tamird opened this issue Oct 16, 2023 · 0 comments

Comments

@tamird
Copy link
Contributor

tamird commented Oct 16, 2023

In my attempt to migrate this crate away from the abandoned proc-macro-error crate I discovered these usages of syn::Result::ok were causing test failures with this diff applied:

diff --git a/crates/test-case-core/src/complex_expr.rs b/crates/test-case-core/src/complex_expr.rs
index e708f2e..2a4334e 100644
--- a/crates/test-case-core/src/complex_expr.rs
+++ b/crates/test-case-core/src/complex_expr.rs
@@ -323,11 +323,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.error("'with-regex' feature is required to use 'matches-regex' keyword"));
                 }
             }
         } else {
-            proc_macro_error::abort!(input.span(), "cannot parse complex expression")
+            return Err(input.error("cannot parse complex expression"));
         })
     }
 }

I realize that these .ok() calls are used in cases where expressions and comments are optional, but it would be better to peek and avoid errors so that these .ok()s can be removed to allow the change above.

tamird added a commit to tamird/test-case that referenced this issue Nov 7, 2023
This prevents syn::Error from propagating.

Updates frondeus#135.
tamird added a commit to tamird/test-case that referenced this issue Nov 7, 2023
luke-biel pushed a commit that referenced this issue Nov 17, 2023
* Don't blindly discard errors

This prevents syn::Error from propagating.

Updates #135.

* Use syn::ParseStream::error

This fails tests.

#135

* Use proc-macro2-diagnostics

Bump MSRV to 1.63 for the new yansi dependency (through
proc-macro2-diagnostics). See
https://github.com/SergioBenitez/yansi/blob/e8247a492e1b/Cargo.toml#L12.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant