Dramatically simplify gix_macros::momo

NobodyXu · NobodyXu · commit c72eaa05697a · 2023-08-29T21:38:33.000+10:00
And make applying support of `gix_macros::momo` truly painless by making
it work out-of-the-box.

Signed-off-by: Jiahao XU &lt;Jiahao_XU@outlook.com&gt;
diff --git a/gix-macros/src/lib.rs b/gix-macros/src/lib.rs
@@ -1,8 +1,8 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 
 use proc_macro::TokenStream;
 use quote::quote;
-use syn::{fold::Fold, punctuated::Punctuated, spanned::Spanned, *};
+use syn::{punctuated::Punctuated, spanned::Spanned, *};
 
 #[derive(Copy, Clone)]
 // All conversions we support.  Check references to this type for an idea how to add more.
@@ -13,14 +13,6 @@ enum Conversion<'a> {
 }
 
 impl<'a> Conversion<'a> {
-    fn target_type(&self) -> Type {
-        match *self {
-            Conversion::Into(ty) => ty.clone(),
-            Conversion::AsRef(ty) => parse_quote!(&#ty),
-            Conversion::AsMut(ty) => parse_quote!(&mut #ty),
-        }
-    }
-
     fn conversion_expr(&self, i: Ident) -> Expr {
         match *self {
             Conversion::Into(_) => parse_quote!(#i.into()),
@@ -62,58 +54,29 @@ fn parse_bounds(bounds: &Punctuated<TypeParamBound, Token![+]>) -> Option<Conver
 }
 
 // create a map from generic type to Conversion
-fn parse_generics(decl: &Signature) -> (HashMap<Ident, Conversion<'_>>, Generics) {
+fn parse_generics(decl: &Signature) -> HashMap<Ident, Conversion<'_>> {
     let mut ty_conversions = HashMap::new();
-    let mut params = Punctuated::new();
     for gp in decl.generics.params.iter() {
         if let GenericParam::Type(ref tp) = gp {
             if let Some(conversion) = parse_bounds(&tp.bounds) {
                 ty_conversions.insert(tp.ident.clone(), conversion);
                 continue;
             }
         }
-        params.push(gp.clone());
     }
-    let where_clause = if let Some(ref wc) = decl.generics.where_clause {
-        let mut idents_to_remove = HashSet::new();
-        let mut predicates = Punctuated::new();
+    if let Some(ref wc) = decl.generics.where_clause {
         for wp in wc.predicates.iter() {
             if let WherePredicate::Type(ref pt) = wp {
                 if let Some(ident) = parse_bounded_type(&pt.bounded_ty) {
                     if let Some(conversion) = parse_bounds(&pt.bounds) {
-                        idents_to_remove.insert(ident.clone());
                         ty_conversions.insert(ident, conversion);
                         continue;
                     }
                 }
             }
-            predicates.push(wp.clone());
         }
-        params = params
-            .into_iter()
-            .filter(|param| {
-                if let GenericParam::Type(type_param) = param {
-                    !idents_to_remove.contains(&type_param.ident)
-                } else {
-                    true
-                }
-            })
-            .collect();
-        Some(WhereClause {
-            predicates,
-            ..wc.clone()
-        })
-    } else {
-        None
-    };
-    (
-        ty_conversions,
-        Generics {
-            params,
-            where_clause,
-            ..decl.generics.clone()
-        },
-    )
+    }
+    ty_conversions
 }
 
 fn pat_to_ident(pat: &Pat) -> Ident {
@@ -131,140 +94,36 @@ fn pat_to_expr(pat: &Pat) -> Expr {
 fn convert<'a>(
     inputs: &'a Punctuated<FnArg, Token![,]>,
     ty_conversions: &HashMap<Ident, Conversion<'a>>,
-) -> (
-    Punctuated<FnArg, Token![,]>,
-    Conversions,
-    Punctuated<Expr, Token![,]>,
-    bool,
-) {
-    let mut argtypes = Punctuated::new();
-    let mut conversions = Conversions {
-        intos: Vec::new(),
-        as_refs: Vec::new(),
-        as_muts: Vec::new(),
-    };
+) -> (Punctuated<Expr, Token![,]>, bool) {
     let mut argexprs = Punctuated::new();
     let mut has_self = false;
     inputs.iter().for_each(|input| match input {
         FnArg::Receiver(..) => {
             has_self = true;
-            argtypes.push(input.clone());
         }
-        FnArg::Typed(PatType {
-            ref pat,
-            ref ty,
-            ref colon_token,
-            ..
-        }) => match **ty {
+        FnArg::Typed(PatType { ref pat, ref ty, .. }) => match **ty {
             Type::ImplTrait(TypeImplTrait { ref bounds, .. }) => {
                 if let Some(conv) = parse_bounds(bounds) {
-                    argtypes.push(FnArg::Typed(PatType {
-                        attrs: Vec::new(),
-                        pat: pat.clone(),
-                        colon_token: *colon_token,
-                        ty: Box::new(conv.target_type()),
-                    }));
                     let ident = pat_to_ident(pat);
-                    conversions.add(ident.clone(), conv);
                     argexprs.push(conv.conversion_expr(ident));
                 } else {
-                    argtypes.push(input.clone());
                     argexprs.push(pat_to_expr(pat));
                 }
             }
             Type::Path(..) => {
                 if let Some(conv) = parse_bounded_type(ty).and_then(|ident| ty_conversions.get(&ident)) {
-                    argtypes.push(FnArg::Typed(PatType {
-                        attrs: Vec::new(),
-                        pat: pat.clone(),
-                        colon_token: *colon_token,
-                        ty: Box::new(conv.target_type()),
-                    }));
                     let ident = pat_to_ident(pat);
-                    conversions.add(ident.clone(), *conv);
                     argexprs.push(conv.conversion_expr(ident));
                 } else {
-                    argtypes.push(input.clone());
                     argexprs.push(pat_to_expr(pat));
                 }
             }
             _ => {
-                argtypes.push(input.clone());
                 argexprs.push(pat_to_expr(pat));
             }
         },
     });
-    (argtypes, conversions, argexprs, has_self)
-}
-
-struct Conversions {
-    intos: Vec<Ident>,
-    as_refs: Vec<Ident>,
-    as_muts: Vec<Ident>,
-}
-
-impl Conversions {
-    fn add(&mut self, ident: Ident, conv: Conversion) {
-        match conv {
-            Conversion::Into(_) => self.intos.push(ident),
-            Conversion::AsRef(_) => self.as_refs.push(ident),
-            Conversion::AsMut(_) => self.as_muts.push(ident),
-        }
-    }
-}
-
-fn has_conversion(idents: &[Ident], expr: &Expr) -> bool {
-    if let Expr::Path(ExprPath { ref path, .. }) = *expr {
-        if path.segments.len() == 1 {
-            let seg = path.segments.iter().last().unwrap();
-            return idents.iter().any(|i| i == &seg.ident);
-        }
-    }
-    false
-}
-
-#[allow(clippy::collapsible_if)]
-impl Fold for Conversions {
-    fn fold_expr(&mut self, expr: Expr) -> Expr {
-        //TODO: Also catch `Expr::Call` with suitable paths & args
-        match expr {
-            Expr::MethodCall(mc) if mc.args.is_empty() => match &*mc.method.to_string() {
-                "into" if has_conversion(&self.intos, &mc.receiver) => *mc.receiver,
-
-                "as_ref" if has_conversion(&self.as_refs, &mc.receiver) => *mc.receiver,
-                "as_mut" if has_conversion(&self.as_muts, &mc.receiver) => *mc.receiver,
-
-                _ => syn::fold::fold_expr(self, Expr::MethodCall(mc)),
-            },
-            Expr::Call(call) if call.args.len() == 1 => match &*call.func {
-                Expr::Path(ExprPath {
-                    path: Path { segments, .. },
-                    ..
-                }) if segments.len() == 2 => match (&*segments[0].ident.to_string(), &*segments[1].ident.to_string()) {
-                    ("Into", "into") if has_conversion(&self.intos, &call.args[0]) => call.args[0].clone(),
-
-                    ("AsRef", "as_ref") if matches!(&call.args[0], Expr::Reference(ExprReference { expr, mutability: None, .. }) if has_conversion(&self.as_refs, expr)) => {
-                        if let Expr::Reference(ExprReference { expr, .. }) = &call.args[0] {
-                            (**expr).clone()
-                        } else {
-                            panic!("expr must be Reference")
-                        }
-                    }
-                    ("AsMut", "as_mut") if matches!(&call.args[0], Expr::Reference(ExprReference { expr, mutability: Some(_), .. }) if has_conversion(&self.as_muts, expr)) => {
-                        if let Expr::Reference(ExprReference { expr, .. }) = &call.args[0] {
-                            (**expr).clone()
-                        } else {
-                            panic!("expr must be Reference")
-                        }
-                    }
-
-                    _ => syn::fold::fold_expr(self, Expr::Call(call)),
-                },
-                _ => syn::fold::fold_expr(self, Expr::Call(call)),
-            },
-            _ => syn::fold::fold_expr(self, expr),
-        }
-    }
+    (argexprs, has_self)
 }
 
 fn contains_self_type_path(path: &Path) -> bool {
@@ -339,8 +198,8 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream {
     };
 
     if let Item::Fn(item_fn) = fn_item {
-        let (ty_conversions, generics) = parse_generics(&item_fn.sig);
-        let (argtypes, mut conversions, argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions);
+        let ty_conversions = parse_generics(&item_fn.sig);
+        let (argexprs, has_self) = convert(&item_fn.sig.inputs, &ty_conversions);
 
         let uses_self = has_self
             || item_fn.sig.inputs.iter().any(has_self_type)
@@ -367,11 +226,9 @@ fn momo_inner(code: proc_macro2::TokenStream) -> proc_macro2::TokenStream {
             vis: Visibility::Inherited,
             sig: Signature {
                 ident: inner_ident.clone(),
-                generics,
-                inputs: argtypes,
                 ..item_fn.sig.clone()
             },
-            block: Box::new(conversions.fold_block(*item_fn.block)),
+            block: item_fn.block,
         });
 
         if uses_self {
diff --git a/gix/src/repository/config/transport.rs b/gix/src/repository/config/transport.rs
@@ -23,14 +23,13 @@ impl crate::Repository {
         )),
         allow(unused_variables)
     )]
-    #[allow(clippy::needless_lifetimes)]
     #[momo]
     pub fn transport_options<'a>(
         &self,
         url: impl Into<&'a BStr>,
         remote_name: Option<&BStr>,
     ) -> Result<Option<Box<dyn Any>>, crate::config::transport::Error> {
-        let url = gix_url::parse(url)?;
+        let url = gix_url::parse(url.into())?;
         use gix_url::Scheme::*;
 
         match &url.scheme {
diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs
@@ -24,6 +24,7 @@ impl crate::Repository {
     /// Loose object could be partially decoded, even though that's not implemented.
     #[momo]
     pub fn find_object(&self, id: impl Into<ObjectId>) -> Result<Object<'_>, object::find::existing::Error> {
+        let id = id.into();
         if id == gix_hash::ObjectId::empty_tree(self.object_hash()) {
             return Ok(Object {
                 id,
@@ -43,6 +44,7 @@ impl crate::Repository {
     #[doc(alias = "read_header", alias = "git2")]
     #[momo]
     pub fn find_header(&self, id: impl Into<ObjectId>) -> Result<gix_odb::find::Header, object::find::existing::Error> {
+        let id = id.into();
         if id == gix_hash::ObjectId::empty_tree(self.object_hash()) {
             return Ok(gix_odb::find::Header::Loose {
                 kind: gix_object::Kind::Tree,
@@ -60,6 +62,7 @@ impl crate::Repository {
         &self,
         id: impl Into<ObjectId>,
     ) -> Result<Option<gix_odb::find::Header>, object::find::Error> {
+        let id = id.into();
         if id == gix_hash::ObjectId::empty_tree(self.object_hash()) {
             return Ok(Some(gix_odb::find::Header::Loose {
                 kind: gix_object::Kind::Tree,
@@ -72,6 +75,7 @@ impl crate::Repository {
     /// Try to find the object with `id` or return `None` if it wasn't found.
     #[momo]
     pub fn try_find_object(&self, id: impl Into<ObjectId>) -> Result<Option<Object<'_>>, object::find::Error> {
+        let id = id.into();
         if id == gix_hash::ObjectId::empty_tree(self.object_hash()) {
             return Ok(Some(Object {
                 id,
@@ -175,11 +179,11 @@ impl crate::Repository {
         constraint: PreviousValue,
     ) -> Result<Reference<'_>, tag::Error> {
         let tag = gix_object::Tag {
-            target: target.into(),
+            target: target.as_ref().into(),
             target_kind,
-            name: name.into(),
+            name: name.as_ref().into(),
             tagger: tagger.map(|t| t.to_owned()),
-            message: message.into(),
+            message: message.as_ref().into(),
             pgp_signature: None,
         };
         let tag_id = self.write_object(&tag)?;
diff --git a/gix/src/repository/reference.rs b/gix/src/repository/reference.rs
@@ -22,14 +22,14 @@ impl crate::Repository {
         target: impl Into<ObjectId>,
         constraint: PreviousValue,
     ) -> Result<Reference<'_>, reference::edit::Error> {
-        let id = target;
+        let id = target.into();
         let mut edits = self.edit_reference(RefEdit {
             change: Change::Update {
                 log: Default::default(),
                 expected: constraint,
                 new: Target::Peeled(id),
             },
-            name: format!("refs/tags/{name}").try_into()?,
+            name: format!("refs/tags/{}", name.as_ref()).try_into()?,
             deref: false,
         })?;
         assert_eq!(edits.len(), 1, "reference splits should ever happen");
diff --git a/gix/src/repository/worktree.rs b/gix/src/repository/worktree.rs
@@ -63,10 +63,11 @@ impl crate::Repository {
         id: impl Into<gix_hash::ObjectId>,
     ) -> Result<(gix_worktree_stream::Stream, gix_index::File), crate::repository::worktree_stream::Error> {
         use gix_odb::{FindExt, HeaderExt};
+        let id = id.into();
         let header = self.objects.header(id)?;
         if !header.kind().is_tree() {
             return Err(crate::repository::worktree_stream::Error::NotATree {
-                id: id.to_owned(),
+                id,
                 actual: header.kind(),
             });
         }
