From 60fddd15537ef9651f31e05e5c6463d4b42f4e1e Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 5 Sep 2021 17:02:28 +0900 Subject: [PATCH] little bit better error message for signature error --- Cargo.lock | 8 +++--- derive/Cargo.toml | 2 +- derive/src/pyclass.rs | 60 ++++++++++++++++++++---------------------- derive/src/pymodule.rs | 16 ++++++----- derive/src/util.rs | 6 +---- 5 files changed, 44 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e70094cff..c15eed1a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1315,9 +1315,9 @@ dependencies = [ [[package]] name = "openssl-probe" -version = "0.1.4" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28988d872ab76095a6e6ac88d99b54fd267702734fd7ffe610ca27f533ddb95a" +checksum = "77af24da69f9d9341038eba93a073b1fdaaa1b788221b00a69bce9e762cb32de" [[package]] name = "openssl-src" @@ -2287,9 +2287,9 @@ dependencies = [ [[package]] name = "syn-ext" -version = "0.2.3" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9210542fa1f4811e2024f7ef22668ef8e035a54f6c9d4826faae8ae2885bd455" +checksum = "ccfaff88a7edb37fd3a16f24b986500e4a6d4b87218d9b38255e208ca4a08848" dependencies = [ "syn", ] diff --git a/derive/Cargo.toml b/derive/Cargo.toml index 9b5f48abd..9954df1c4 100644 --- a/derive/Cargo.toml +++ b/derive/Cargo.toml @@ -12,7 +12,7 @@ proc-macro = true [dependencies] syn = { version = "1.0", features = ["full", "extra-traits"] } -syn-ext = { version = "0.2.3", features = ["full"] } +syn-ext = { version = "0.3.0", features = ["full"] } quote = "1.0" proc-macro2 = "1.0" rustpython-compiler = { path = "../compiler/porcelain", version = "0.1.1" } diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index 9f0c51ce6..2fc214d67 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -1,6 +1,6 @@ use super::Diagnostic; use crate::util::{ - get_sig, path_eq, pyclass_ident_and_attrs, ClassItemMeta, ContentItem, ContentItemInner, + path_eq, pyclass_ident_and_attrs, text_signature, ClassItemMeta, ContentItem, ContentItemInner, ErrorVec, ItemMeta, ItemMetaInner, ItemNursery, SimpleItemMeta, ALL_ALLOWED_NAMES, }; use proc_macro2::TokenStream; @@ -426,22 +426,22 @@ where Item: ItemLike + ToTokens + GetIdent, { fn gen_impl_item(&self, args: ImplItemArgs<'_, Item>) -> Result<()> { - let ident = if args.item.is_function_or_method() { - Ok(args.item.get_ident().unwrap()) - } else { - Err(self.new_syn_error(args.item.span(), "can only be on a method")) - }?; + let func = args + .item + .function_or_method() + .map_err(|_| self.new_syn_error(args.item.span(), "can only be on a method"))?; + let ident = &func.sig().ident; let item_attr = args.attrs.remove(self.index()); let item_meta = MethodItemMeta::from_attr(ident.clone(), &item_attr)?; let py_name = item_meta.method_name()?; - let sig_doc = get_sig(args.item, &py_name); + let sig_doc = text_signature(func.sig(), &py_name); let tokens = { let doc = args.attrs.doc().map_or_else(TokenStream::new, |mut doc| { doc = format!("{}\n--\n\n{}", sig_doc, doc); - quote!(.with_doc(#doc.to_owned(), &ctx)) + quote!(.with_doc(#doc.to_owned(), ctx)) }); let build_func = match self.method_type.as_str() { "method" => quote!(.build_method(ctx, class.clone())), @@ -473,11 +473,11 @@ where Item: ItemLike + ToTokens + GetIdent, { fn gen_impl_item(&self, args: ImplItemArgs<'_, Item>) -> Result<()> { - let ident = if args.item.is_function_or_method() { - Ok(args.item.get_ident().unwrap()) - } else { - Err(self.new_syn_error(args.item.span(), "can only be on a method")) - }?; + let func = args + .item + .function_or_method() + .map_err(|_| self.new_syn_error(args.item.span(), "can only be on a method"))?; + let ident = &func.sig().ident; let item_attr = args.attrs.remove(self.index()); let item_meta = PropertyItemMeta::from_attr(ident.clone(), &item_attr)?; @@ -495,11 +495,11 @@ where Item: ItemLike + ToTokens + GetIdent, { fn gen_impl_item(&self, args: ImplItemArgs<'_, Item>) -> Result<()> { - let ident = if args.item.is_function_or_method() { - Ok(args.item.get_ident().unwrap()) - } else { - Err(self.new_syn_error(args.item.span(), "can only be on a method")) - }?; + let func = args + .item + .function_or_method() + .map_err(|_| self.new_syn_error(args.item.span(), "can only be on a method"))?; + let ident = &func.sig().ident; let item_attr = args.attrs.remove(self.index()); let item_meta = SlotItemMeta::from_attr(ident.clone(), &item_attr)?; @@ -507,17 +507,14 @@ where let slot_ident = item_meta.slot_name()?; let slot_name = slot_ident.to_string(); let tokens = { - let into_func = quote_spanned! {ident.span() => - Self::#ident as _ - }; const NON_ATOMIC_SLOTS: &[&str] = &["as_buffer"]; if NON_ATOMIC_SLOTS.contains(&slot_name.as_str()) { - quote! { - slots.#slot_ident = Some(#into_func); + quote_spanned! { func.span() => + slots.#slot_ident = Some(Self::#ident as _); } } else { - quote! { - slots.#slot_ident.store(Some(#into_func)) + quote_spanned! { func.span() => + slots.#slot_ident.store(Some(Self::#ident as _)); } } }; @@ -545,7 +542,7 @@ where let py_name = item_meta.simple_name()?; Ok(py_name) }; - let (py_name, tokens) = if args.item.is_function_or_method() || args.item.is_const() { + let (py_name, tokens) = if args.item.function_or_method().is_ok() || args.item.is_const() { let ident = args.item.get_ident().unwrap(); let py_name = get_py_name(&attr, ident)?; @@ -583,11 +580,12 @@ where fn gen_impl_item(&self, args: ImplItemArgs<'_, Item>) -> Result<()> { args.attrs.remove(self.index()); - let ident = if args.item.is_function_or_method() { - Ok(args.item.get_ident().unwrap()) - } else { - Err(self.new_syn_error(args.item.span(), "can only be on a method")) - }?; + let ident = &args + .item + .function_or_method() + .map_err(|_| self.new_syn_error(args.item.span(), "can only be on a method"))? + .sig() + .ident; args.context.class_extensions.push(quote! { Self::#ident(ctx, class); diff --git a/derive/src/pymodule.rs b/derive/src/pymodule.rs index dbb8c6146..c048729c0 100644 --- a/derive/src/pymodule.rs +++ b/derive/src/pymodule.rs @@ -1,7 +1,8 @@ use crate::error::Diagnostic; use crate::util::{ - get_sig, iter_use_idents, pyclass_ident_and_attrs, AttributeExt, ClassItemMeta, ContentItem, - ContentItemInner, ErrorVec, ItemMeta, ItemNursery, SimpleItemMeta, ALL_ALLOWED_NAMES, + iter_use_idents, pyclass_ident_and_attrs, text_signature, AttributeExt, ClassItemMeta, + ContentItem, ContentItemInner, ErrorVec, ItemMeta, ItemNursery, SimpleItemMeta, + ALL_ALLOWED_NAMES, }; use proc_macro2::TokenStream; use quote::{quote, quote_spanned, ToTokens}; @@ -247,16 +248,17 @@ trait ModuleItem: ContentItem { impl ModuleItem for FunctionItem { fn gen_module_item(&self, args: ModuleItemArgs<'_>) -> Result<()> { - let ident = match args.item { - Item::Fn(syn::ItemFn { sig, .. }) => sig.ident.clone(), - other => return Err(self.new_syn_error(other.span(), "can only be on a function")), - }; + let func = args + .item + .function_or_method() + .map_err(|_| self.new_syn_error(args.item.span(), "can only be on a function"))?; + let ident = &func.sig().ident; let item_attr = args.attrs.remove(self.index()); let item_meta = SimpleItemMeta::from_attr(ident.clone(), &item_attr)?; let py_name = item_meta.simple_name()?; - let sig_doc = get_sig(args.item, &py_name); + let sig_doc = text_signature(func.sig(), &py_name); let item = { let doc = args.attrs.doc().map_or_else(TokenStream::new, |mut doc| { diff --git a/derive/src/util.rs b/derive/src/util.rs index 3f0445dbe..248b6af78 100644 --- a/derive/src/util.rs +++ b/derive/src/util.rs @@ -507,11 +507,7 @@ where // Best effort attempt to generate a template from which a // __text_signature__ can be created. -pub(crate) fn get_sig(item: &Item, name: &str) -> String { - let sig = match item.function_or_method_impl().map(|f| f.sig()) { - Ok(sig) => sig, - _ => return "".to_owned(), - }; +pub(crate) fn text_signature(sig: &Signature, name: &str) -> String { let signature = func_sig(sig); if signature.starts_with("$self") { format!("{}({})", name, signature)