From 0265af415d5b61a4e912fd8481839213f2a16074 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 20 Oct 2021 13:36:40 +0800 Subject: [PATCH] General code cleanup. --- codegen/src/attrs.rs | 29 +++++------ codegen/src/function.rs | 62 +++++++++++------------ codegen/src/module.rs | 51 ++++++++----------- codegen/src/register.rs | 7 +-- codegen/src/rhai_module.rs | 36 ++++++------- codegen/src/test/function.rs | 4 +- codegen/ui_tests/second_shared_ref.stderr | 2 +- 7 files changed, 89 insertions(+), 102 deletions(-) diff --git a/codegen/src/attrs.rs b/codegen/src/attrs.rs index 7ffdae87..57d18c96 100644 --- a/codegen/src/attrs.rs +++ b/codegen/src/attrs.rs @@ -1,3 +1,4 @@ +use proc_macro2::{Ident, Span, TokenStream}; use syn::{ parse::{ParseStream, Parser}, spanned::Spanned, @@ -24,14 +25,14 @@ pub trait ExportedParams: Sized { #[derive(Debug, Clone)] pub struct AttrItem { - pub key: proc_macro2::Ident, + pub key: Ident, pub value: Option, - pub span: proc_macro2::Span, + pub span: Span, } #[derive(Debug, Clone)] pub struct ExportInfo { - pub item_span: proc_macro2::Span, + pub item_span: Span, pub items: Vec, } @@ -42,8 +43,7 @@ pub fn parse_attr_items(args: ParseStream) -> syn::Result { items: Vec::new(), }); } - let arg_list = args - .call(syn::punctuated::Punctuated::::parse_separated_nonempty)?; + let arg_list = args.call(syn::punctuated::Punctuated::parse_separated_nonempty)?; parse_punctuated_items(arg_list) } @@ -53,7 +53,8 @@ pub fn parse_punctuated_items( ) -> syn::Result { let list_span = arg_list.span(); - let mut attrs: Vec = Vec::new(); + let mut attrs = Vec::new(); + for arg in arg_list { let arg_span = arg.span(); let (key, value) = match arg { @@ -62,7 +63,7 @@ pub fn parse_punctuated_items( ref right, .. }) => { - let attr_name: syn::Ident = match left.as_ref() { + let attr_name = match left.as_ref() { syn::Expr::Path(syn::ExprPath { path: attr_path, .. }) => attr_path.get_ident().cloned().ok_or_else(|| { @@ -79,13 +80,11 @@ pub fn parse_punctuated_items( }; (attr_name, Some(attr_value)) } - syn::Expr::Path(syn::ExprPath { - path: attr_path, .. - }) => attr_path + syn::Expr::Path(syn::ExprPath { path, .. }) => path .get_ident() .cloned() .map(|a| (a, None)) - .ok_or_else(|| syn::Error::new(attr_path.span(), "expecting attribute name"))?, + .ok_or_else(|| syn::Error::new(path.span(), "expecting attribute name"))?, x => return Err(syn::Error::new(x.span(), "expecting identifier")), }; attrs.push(AttrItem { @@ -102,18 +101,16 @@ pub fn parse_punctuated_items( } pub fn outer_item_attributes( - args: proc_macro2::TokenStream, + args: TokenStream, _attr_name: &str, ) -> syn::Result { if args.is_empty() { return Ok(T::no_attrs()); } - let parser = syn::punctuated::Punctuated::::parse_separated_nonempty; - let arg_list = parser.parse2(args)?; + let arg_list = syn::punctuated::Punctuated::parse_separated_nonempty.parse2(args)?; - let export_info = parse_punctuated_items(arg_list)?; - T::from_info(export_info) + T::from_info(parse_punctuated_items(arg_list)?) } pub fn inner_item_attributes( diff --git a/codegen/src/function.rs b/codegen/src/function.rs index 903d9fd6..73bda67a 100644 --- a/codegen/src/function.rs +++ b/codegen/src/function.rs @@ -1,3 +1,10 @@ +use proc_macro2::{Span, TokenStream}; +use quote::{quote, quote_spanned, ToTokens}; +use syn::{ + parse::{Parse, ParseStream}, + spanned::Spanned, +}; + #[cfg(no_std)] use alloc::format; #[cfg(not(no_std))] @@ -5,12 +12,6 @@ use std::format; use std::borrow::Cow; -use quote::{quote, quote_spanned, ToTokens}; -use syn::{ - parse::{Parse, ParseStream}, - spanned::Spanned, -}; - use crate::attrs::{ExportInfo, ExportScope, ExportedParams}; #[derive(Clone, Debug, Eq, PartialEq, Copy, Hash)] @@ -52,7 +53,7 @@ impl Default for FnSpecialAccess { } impl FnSpecialAccess { - pub fn get_fn_name(&self) -> Option<(String, String, proc_macro2::Span)> { + pub fn get_fn_name(&self) -> Option<(String, String, Span)> { match self { FnSpecialAccess::None => None, FnSpecialAccess::Property(Property::Get(ref g)) => { @@ -64,12 +65,12 @@ impl FnSpecialAccess { FnSpecialAccess::Index(Index::Get) => Some(( FN_IDX_GET.to_string(), "index_get".to_string(), - proc_macro2::Span::call_site(), + Span::call_site(), )), FnSpecialAccess::Index(Index::Set) => Some(( FN_IDX_SET.to_string(), "index_set".to_string(), - proc_macro2::Span::call_site(), + Span::call_site(), )), } } @@ -98,12 +99,12 @@ pub fn print_type(ty: &syn::Type) -> String { #[derive(Debug, Default)] pub struct ExportedFnParams { pub name: Vec, - pub return_raw: Option, - pub pure: Option, + pub return_raw: Option, + pub pure: Option, pub skip: bool, pub special: FnSpecialAccess, pub namespace: FnNamespaceAccess, - pub span: Option, + pub span: Option, } pub const FN_GET: &str = "get$"; @@ -274,7 +275,7 @@ impl ExportedParams for ExportedFnParams { #[derive(Debug)] pub struct ExportedFn { - entire_span: proc_macro2::Span, + entire_span: Span, signature: syn::Signature, visibility: syn::Visibility, pass_context: bool, @@ -369,7 +370,7 @@ impl Parse for ExportedFn { if !is_ok { return Err(syn::Error::new( ty.span(), - "this type in this position passes from Rhai by value", + "function parameters other than the first one cannot be passed by reference", )); } } @@ -443,7 +444,7 @@ impl ExportedFn { !matches!(self.visibility, syn::Visibility::Inherited) } - pub fn span(&self) -> &proc_macro2::Span { + pub fn span(&self) -> &Span { &self.entire_span } @@ -456,7 +457,7 @@ impl ExportedFn { .params .name .iter() - .map(|s| syn::LitStr::new(s, proc_macro2::Span::call_site())) + .map(|s| syn::LitStr::new(s, Span::call_site())) .collect(); if let Some((s, _, span)) = self.params.special.get_fn_name() { @@ -586,7 +587,7 @@ impl ExportedFn { Ok(()) } - pub fn generate(self) -> proc_macro2::TokenStream { + pub fn generate(self) -> TokenStream { let name: syn::Ident = syn::Ident::new(&format!("rhai_fn_{}", self.name()), self.name().span()); let impl_block = self.generate_impl("Token"); @@ -603,17 +604,16 @@ impl ExportedFn { } } - pub fn generate_dynamic_fn(&self) -> proc_macro2::TokenStream { + pub fn generate_dynamic_fn(&self) -> TokenStream { let name = self.name().clone(); let mut dynamic_signature = self.signature.clone(); - dynamic_signature.ident = - syn::Ident::new("dynamic_result_fn", proc_macro2::Span::call_site()); + dynamic_signature.ident = syn::Ident::new("dynamic_result_fn", Span::call_site()); dynamic_signature.output = syn::parse2::(quote! { -> RhaiResult }) .unwrap(); - let arguments: Vec = dynamic_signature + let arguments: Vec<_> = dynamic_signature .inputs .iter() .filter_map(|fn_arg| match fn_arg { @@ -628,7 +628,7 @@ impl ExportedFn { let return_span = self .return_type() .map(|r| r.span()) - .unwrap_or_else(proc_macro2::Span::call_site); + .unwrap_or_else(Span::call_site); if self.params.return_raw.is_some() { quote_spanned! { return_span => pub #dynamic_signature { @@ -646,16 +646,16 @@ impl ExportedFn { } } - pub fn generate_impl(&self, on_type_name: &str) -> proc_macro2::TokenStream { + pub fn generate_impl(&self, on_type_name: &str) -> TokenStream { let sig_name = self.name().clone(); let arg_count = self.arg_count(); let is_method_call = self.mutable_receiver(); - let mut unpack_statements: Vec = Vec::new(); - let mut unpack_exprs: Vec = Vec::new(); + let mut unpack_statements = Vec::new(); + let mut unpack_exprs = Vec::new(); #[cfg(feature = "metadata")] - let mut input_type_names: Vec = Vec::new(); - let mut input_type_exprs: Vec = Vec::new(); + let mut input_type_names = Vec::new(); + let mut input_type_exprs = Vec::new(); let return_type = self .return_type() @@ -672,7 +672,7 @@ impl ExportedFn { if is_method_call { skip_first_arg = true; let first_arg = self.arg_list().next().unwrap(); - let var = syn::Ident::new("arg0", proc_macro2::Span::call_site()); + let var = syn::Ident::new("arg0", Span::call_site()); match first_arg { syn::FnArg::Typed(syn::PatType { pat, ty, .. }) => { #[cfg(feature = "metadata")] @@ -725,7 +725,7 @@ impl ExportedFn { let str_type_path = syn::parse2::(quote! { str }).unwrap(); let string_type_path = syn::parse2::(quote! { String }).unwrap(); for (i, arg) in self.arg_list().enumerate().skip(skip_first_arg as usize) { - let var = syn::Ident::new(&format!("arg{}", i), proc_macro2::Span::call_site()); + let var = syn::Ident::new(&format!("arg{}", i), Span::call_site()); let is_string; let is_ref; match arg { @@ -811,7 +811,7 @@ impl ExportedFn { let return_span = self .return_type() .map(|r| r.span()) - .unwrap_or_else(proc_macro2::Span::call_site); + .unwrap_or_else(Span::call_site); let return_expr = if self.params.return_raw.is_none() { quote_spanned! { return_span => Ok(Dynamic::from(#sig_name(#(#unpack_exprs),*))) @@ -822,7 +822,7 @@ impl ExportedFn { } }; - let type_name = syn::Ident::new(on_type_name, proc_macro2::Span::call_site()); + let type_name = syn::Ident::new(on_type_name, Span::call_site()); #[cfg(feature = "metadata")] let param_names = quote! { diff --git a/codegen/src/module.rs b/codegen/src/module.rs index c5d20b51..bb791ef6 100644 --- a/codegen/src/module.rs +++ b/codegen/src/module.rs @@ -1,14 +1,6 @@ use quote::{quote, ToTokens}; use syn::{parse::Parse, parse::ParseStream}; -use crate::function::ExportedFn; -use crate::rhai_module::ExportedConst; - -#[cfg(no_std)] -use alloc::vec as new_vec; -#[cfg(not(no_std))] -use std::vec as new_vec; - #[cfg(no_std)] use core::mem; #[cfg(not(no_std))] @@ -17,7 +9,8 @@ use std::mem; use std::borrow::Cow; use crate::attrs::{AttrItem, ExportInfo, ExportScope, ExportedParams}; -use crate::function::ExportedFnParams; +use crate::function::ExportedFn; +use crate::rhai_module::ExportedConst; #[derive(Debug, Clone, Eq, PartialEq, Hash, Default)] pub struct ExportedModParams { @@ -32,9 +25,7 @@ impl Parse for ExportedModParams { return Ok(ExportedModParams::default()); } - let info = crate::attrs::parse_attr_items(args)?; - - Self::from_info(info) + Self::from_info(crate::attrs::parse_attr_items(args)?) } } @@ -115,8 +106,8 @@ impl Parse for Module { fn parse(input: ParseStream) -> syn::Result { let mut mod_all: syn::ItemMod = input.parse()?; let fns: Vec<_>; - let mut consts: Vec<_> = new_vec![]; - let mut sub_modules: Vec<_> = Vec::new(); + let mut consts = Vec::new(); + let mut sub_modules = Vec::new(); if let Some((_, ref mut content)) = mod_all.content { // Gather and parse functions. fns = content @@ -129,11 +120,9 @@ impl Parse for Module { // #[cfg] attributes are not allowed on functions crate::attrs::deny_cfg_attr(&item_fn.attrs)?; - let params: ExportedFnParams = - match crate::attrs::inner_item_attributes(&mut item_fn.attrs, "rhai_fn") { - Ok(p) => p, - Err(e) => return Err(e), - }; + let params = + crate::attrs::inner_item_attributes(&mut item_fn.attrs, "rhai_fn")?; + syn::parse2::(item_fn.to_token_stream()) .and_then(|mut f| { f.set_params(params)?; @@ -155,11 +144,9 @@ impl Parse for Module { }) => { // #[cfg] attributes are not allowed on const declarations crate::attrs::deny_cfg_attr(&attrs)?; - match vis { - syn::Visibility::Public(_) => { - consts.push((ident.to_string(), ty.clone(), expr.as_ref().clone())) - } - _ => {} + + if matches!(vis, syn::Visibility::Public(_)) { + consts.push((ident.to_string(), ty.clone(), expr.as_ref().clone())) } } _ => {} @@ -192,7 +179,7 @@ impl Parse for Module { } } } else { - fns = new_vec![]; + fns = Vec::new(); } Ok(Module { mod_all, @@ -205,7 +192,7 @@ impl Parse for Module { } impl Module { - pub fn attrs(&self) -> &Vec { + pub fn attrs(&self) -> &[syn::Attribute] { &self.mod_all.attrs } @@ -273,10 +260,12 @@ impl Module { // NB: sub-modules must have their new items for exporting generated in depth-first order // to avoid issues caused by re-parsing them - let inner_modules: Vec = sub_modules.drain(..) - .try_fold::, _, - Result, syn::Error>>( - Vec::new(), |mut acc, m| { acc.push(m.generate_inner()?); Ok(acc) })?; + let inner_modules = sub_modules + .drain(..) + .try_fold::<_, _, Result<_, syn::Error>>(Vec::new(), |mut acc, m| { + acc.push(m.generate_inner()?); + Ok(acc) + })?; // Regenerate the module with the new content added. Ok(quote! { @@ -319,7 +308,7 @@ impl Module { } #[allow(dead_code)] - pub fn content(&self) -> Option<&Vec> { + pub fn content(&self) -> Option<&[syn::Item]> { match self.mod_all { syn::ItemMod { content: Some((_, ref vec)), diff --git a/codegen/src/register.rs b/codegen/src/register.rs index 5813e2c1..773711c9 100644 --- a/codegen/src/register.rs +++ b/codegen/src/register.rs @@ -22,10 +22,11 @@ type RegisterMacroInput = (syn::Expr, proc_macro2::TokenStream, syn::Path); pub fn parse_register_macro( args: proc_macro::TokenStream, ) -> Result { - let parser = syn::punctuated::Punctuated::::parse_separated_nonempty; - let args = parser.parse(args).unwrap(); + let args = syn::punctuated::Punctuated::<_, syn::Token![,]>::parse_separated_nonempty + .parse(args) + .unwrap(); let arg_span = args.span(); - let mut items: Vec = args.into_iter().collect(); + let mut items: Vec<_> = args.into_iter().collect(); if items.len() != 3 { return Err(syn::Error::new( arg_span, diff --git a/codegen/src/rhai_module.rs b/codegen/src/rhai_module.rs index f3137f78..29366301 100644 --- a/codegen/src/rhai_module.rs +++ b/codegen/src/rhai_module.rs @@ -1,7 +1,8 @@ -use std::collections::BTreeMap; - +use proc_macro2::{Span, TokenStream}; use quote::quote; +use std::collections::BTreeMap; + use crate::attrs::ExportScope; use crate::function::{ flatten_type_groups, print_type, ExportedFn, FnNamespaceAccess, FnSpecialAccess, FN_GET, @@ -16,17 +17,17 @@ pub fn generate_body( consts: &[ExportedConst], sub_modules: &mut [Module], parent_scope: &ExportScope, -) -> proc_macro2::TokenStream { - let mut set_fn_statements: Vec = Vec::new(); - let mut set_const_statements: Vec = Vec::new(); - let mut add_mod_blocks: Vec = Vec::new(); - let mut set_flattened_mod_blocks: Vec = Vec::new(); +) -> TokenStream { + let mut set_fn_statements = Vec::new(); + let mut set_const_statements = Vec::new(); + let mut add_mod_blocks = Vec::new(); + let mut set_flattened_mod_blocks = Vec::new(); let str_type_path = syn::parse2::(quote! { str }).unwrap(); let string_type_path = syn::parse2::(quote! { String }).unwrap(); for (const_name, _, _) in consts { - let const_literal = syn::LitStr::new(&const_name, proc_macro2::Span::call_site()); - let const_ref = syn::Ident::new(&const_name, proc_macro2::Span::call_site()); + let const_literal = syn::LitStr::new(&const_name, Span::call_site()); + let const_ref = syn::Ident::new(&const_name, Span::call_site()); set_const_statements.push( syn::parse2::(quote! { m.set_var(#const_literal, #const_ref); @@ -41,11 +42,9 @@ pub fn generate_body( continue; } let module_name = item_mod.module_name(); - let exported_name: syn::LitStr = syn::LitStr::new( - item_mod.exported_name().as_ref(), - proc_macro2::Span::call_site(), - ); - let cfg_attrs: Vec<&syn::Attribute> = item_mod + let exported_name: syn::LitStr = + syn::LitStr::new(item_mod.exported_name().as_ref(), Span::call_site()); + let cfg_attrs: Vec<_> = item_mod .attrs() .iter() .filter(|&a| a.path.get_ident().map(|i| *i == "cfg").unwrap_or(false)) @@ -71,7 +70,8 @@ pub fn generate_body( } // NB: these are token streams, because re-parsing messes up "> >" vs ">>" - let mut gen_fn_tokens: Vec = Vec::new(); + let mut gen_fn_tokens = Vec::new(); + for function in fns { function.update_scope(&parent_scope); if function.skipped() { @@ -83,7 +83,7 @@ pub fn generate_body( ); let reg_names = function.exported_names(); - let fn_input_types: Vec = function + let fn_input_types: Vec<_> = function .arg_list() .map(|fn_arg| match fn_arg { syn::FnArg::Receiver(_) => panic!("internal error: receiver fn outside impl!?"), @@ -229,8 +229,8 @@ pub fn check_rename_collisions(fns: &[ExportedFn]) -> Result<(), syn::Error> { }) } - let mut renames = BTreeMap::::new(); - let mut fn_defs = BTreeMap::::new(); + let mut renames = BTreeMap::new(); + let mut fn_defs = BTreeMap::new(); for item_fn in fns.iter() { if !item_fn.params().name.is_empty() || item_fn.params().special != FnSpecialAccess::None { diff --git a/codegen/src/test/function.rs b/codegen/src/test/function.rs index 02a010b8..04d693b2 100644 --- a/codegen/src/test/function.rs +++ b/codegen/src/test/function.rs @@ -126,7 +126,7 @@ mod function_tests { let err = syn::parse2::(input_tokens).unwrap_err(); assert_eq!( format!("{}", err), - "this type in this position passes from Rhai by value" + "function parameters other than the first one cannot be passed by reference" ); } @@ -139,7 +139,7 @@ mod function_tests { let err = syn::parse2::(input_tokens).unwrap_err(); assert_eq!( format!("{}", err), - "this type in this position passes from Rhai by value" + "function parameters other than the first one cannot be passed by reference" ); } diff --git a/codegen/ui_tests/second_shared_ref.stderr b/codegen/ui_tests/second_shared_ref.stderr index 2b53f098..814cd250 100644 --- a/codegen/ui_tests/second_shared_ref.stderr +++ b/codegen/ui_tests/second_shared_ref.stderr @@ -1,4 +1,4 @@ -error: this type in this position passes from Rhai by value +error: function parameters other than the first one cannot be passed by reference --> ui_tests/second_shared_ref.rs:12:41 | 12 | pub fn test_fn(input: Clonable, factor: &bool) -> bool {