Allow Path or Expr evaluating to Into<String> as arg for import_tokens_proc and import_tokens_attr

[sam0x17]

Part of #3, specifically closes item 3 of it

Originally just Path was supported. This adds additional support for an expression that evaluates to something compatible to Into<String>. This is useful for the scenario outlined in #3 where we might have to determine at compile-time what the real path to macro_magic is from the perspective of the user of your proc macro.

[sam0x17]

Still needs docs but curious about your thoughts @thiolliere

[sam0x17]

The error messages are actually pretty nice.. if they provide an expr that evaluates to something else we get the helpful:

And in lieu of that (like if you provide something that is neither a path nor an expr), it will say "expected expression" which is also fine. This is because path gets speculatively parsed first, since paths are exprs anyway.

[gui1117]

personally I would prefer that the expression to return syn::Path, user will use path or provide a complex expression usually.
We rarely use string in proc_macro, that would also avoid the unwrap in the parsing of the string returned by the expression.
like if I do typo like this:

diff --git a/tests/test_macros/src/lib.rs b/tests/test_macros/src/lib.rs
index 02f2a89..15f9c13 100644
--- a/tests/test_macros/src/lib.rs
+++ b/tests/test_macros/src/lib.rs
@@ -72,7 +72,7 @@ pub fn some_other_macro(tokens: TokenStream) -> TokenStream {
 
 // as demonstrated here, `import_tokens_attr` can take a path or an expression that evaluates
 // to something compatible with `Into<String>`
-#[import_tokens_attr(format!("{}::export_mod::sub_mod::macro_magic", "middle_crate"))]
+#[import_tokens_attr(format!("{}:export_mod::sub_mod::macro_magic", "middle_crate"))]
 #[proc_macro_attribute]
 pub fn distant_re_export_attr(attr: TokenStream, tokens: TokenStream) -> TokenStream {
     let imported_item = parse_macro_input!(attr as Item);

then it gives this error:

error: custom attribute panicked
 --> tests/isolated_crate/src/lib.rs:7:1
  |
7 | #[distant_re_export_attr(middle_crate::ForeignItem)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: message: called `Result::unwrap()` on an `Err` value: Error("unexpected token")

So I was thinking something like this

diff --git a/core/src/lib.rs b/core/src/lib.rs
index e9dcaa1..c9aa031 100644
--- a/core/src/lib.rs
+++ b/core/src/lib.rs
@@ -723,6 +723,7 @@ impl syn::parse::Parse for OverridePath {
         if let Ok(path) = parse2::<Path>(remaining.clone()) {
             return Ok(OverridePath::Path(path));
         }
+        // Maybe we can do better by combining it with a custom error see syn::parse::Error::combine
         Ok(OverridePath::Expr(parse2::<Expr>(remaining)?))
     }
 }
@@ -730,10 +731,7 @@ impl syn::parse::Parse for OverridePath {
 impl ToTokens for OverridePath {
     fn to_tokens(&self, tokens: &mut TokenStream2) {
         match self {
-            OverridePath::Path(path) => {
-                let path = path.to_token_stream().to_string();
-                tokens.extend(quote!(#path))
-            }
+            OverridePath::Path(path) => tokens.extend(quote!(syn::parse_quote!(#path))),
             OverridePath::Expr(expr) => tokens.extend(quote!(#expr)),
         }
     }
@@ -832,7 +830,7 @@ pub fn import_tokens_attr_internal<T1: Into<TokenStream2>, T2: Into<TokenStream2
                 #path_resolver
                 let path = path.to_token_stream();
                 let custom_parsed = custom_parsed.to_token_stream();
-                let resolved_mm_override_path = syn::parse2::<syn::Path>(String::from(#mm_override_path).parse().unwrap()).unwrap();
+                let resolved_mm_override_path: syn::Path = #mm_override_path;
                 quote::quote! {
                     #pound resolved_mm_override_path::forward_tokens! {
                         #pound path,
@@ -914,7 +912,8 @@ pub fn import_tokens_proc_internal<T1: Into<TokenStream2>, T2: Into<TokenStream2
                     Ok(path) => path,
                     Err(e) => return e.to_compile_error().into(),
                 };
-                let resolved_mm_override_path = syn::parse2::<syn::Path>(String::from(#mm_override_path).parse().unwrap()).unwrap();
+
+                let resolved_mm_override_path: syn::Path = #mm_override_path;
                 quote::quote! {
                     #pound resolved_mm_override_path::forward_tokens! {
                         #pound source_path,
diff --git a/tests/test_macros/src/lib.rs b/tests/test_macros/src/lib.rs
index 02f2a89..937ded0 100644
--- a/tests/test_macros/src/lib.rs
+++ b/tests/test_macros/src/lib.rs
@@ -70,9 +70,14 @@ pub fn some_other_macro(tokens: TokenStream) -> TokenStream {
     .into()
 }
 
+fn expression_to_generate_access_to_macro_magic() -> syn::Path {
+       syn::parse2(quote!(middle_crate::export_mod::sub_mod::macro_magic)).unwrap()
+}
+
+
 // as demonstrated here, `import_tokens_attr` can take a path or an expression that evaluates
 // to something compatible with `Into<String>`
-#[import_tokens_attr(format!("{}::export_mod::sub_mod::macro_magic", "middle_crate"))]
+#[import_tokens_attr(expression_to_generate_access_to_macro_magic())]
 #[proc_macro_attribute]
 pub fn distant_re_export_attr(attr: TokenStream, tokens: TokenStream) -> TokenStream {
     let imported_item = parse_macro_input!(attr as Item);
@@ -87,7 +92,7 @@ pub fn distant_re_export_attr(attr: TokenStream, tokens: TokenStream) -> TokenSt
     .into()
 }
 
-#[import_tokens_proc(format!("middle_crate::export_mod::{}::macro_magic", "sub_mod"))]
+#[import_tokens_proc(expression_to_generate_access_to_macro_magic())]
 #[proc_macro]
 pub fn distant_re_export_proc(tokens: TokenStream) -> TokenStream {
     let imported_item = parse_macro_input!(tokens as Item);

but it is good anyway.

also error when failing to parse expression can be improved with https://docs.rs/syn/latest/syn/parse/struct.Error.html#method.combine
so we could combine with some custom message: "expect expression or path"

[gui1117]

thinking again best would be expression to return a syn::Result<syn::Path> so we can even return a good error message without macro panic.

[sam0x17]

thinking again best would be expression to return a syn::Result<syn::Path> so we can even return a good error message without macro panic.

Well there really is no panic for that particular part if for example a non-string is provided, it's a compile-error in expansion code and rust-analyzer and rustc are highlighting it properly, so I would leave that alone

As far as returning a syn::Path instead of a String goes, let me see how well that works. I'm worried about assuming the user has syn but maybe that is fine

let resolved_mm_override_path = syn::parse2::<syn::Path>(String::from(#mm_override_path).parse().unwrap()).unwrap();

now the first unwrap() there would only fail if a string was provided that is an invalid TokenStream, which would result in an earlier compile failure anyway, so this one will never throw. Now the second unwrap, this one I should handle better so I'll do that

edit: done! now looks like this:

                let resolved_mm_override_path = match syn::parse2::<syn::Path>(String::from(#mm_override_path).parse().unwrap()) {
                    Ok(res) => res,
                    Err(err) => return err.to_compile_error().into()
                };

edit: ah just noticed your previous comment, taking a look

[sam0x17]

also added the error.combine() stuff 👍🏻

[gui1117]

sorry I think you may be right, using syn here can be an issue if user use another version of syn or not syn at all.
String is a good standard type, I think your solution is probably better

EDIT: or maybe TokenStream as it is standard too