Skip to content

Added optional deref implementation for Getters #19

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.

Sign up for GitHub

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

Closed
Razican wants to merge 4 commits into from
Closed

Added optional deref implementation for Getters #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6aabad
Added deref implementation
Razican Dec 28, 2018
cf91053
Improved syntax
Razican Dec 28, 2018
730d060
Added tests and documentation for the #[deref] attribute
Razican Jan 2, 2019
f237bdc
Merge branch 'master' into deref
Hoverbear Jan 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ proc-macro = true

[dependencies]
quote = "0.6.10"
syn = "0.15.21"
syn = "0.15.23"
proc-macro2 = { version = "0.4.24", default-features = false }
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ extern crate getset;
pub struct Foo<T> where T: Copy + Clone + Default {
/// Doc comments are supported!
/// Multiline, even.
#[get] #[set] #[get_mut]
#[get] #[deref] #[set] #[get_mut]
Copy link
Copy Markdown
Collaborator

@Hoverbear Hoverbear Jan 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I ask why you chose this option over something like #[get ="deref"] or #[get = "pub deref"]?

Copy link
Copy Markdown
Contributor Author

@Razican Razican Jan 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was mostly to be coherent with the current implementation. The string associated with the attribute was the "visibility" of the function, so it was a bit strange to add the deref part there. I can change it, of course, but it was strange for me.

Also, if we used the deref in that place, would we allow #[get="deref pub"]? or only #[get="pub deref"]. What about complex visibility modifiers such as #[get="pub(crate)"]?

It can be done, but i think it's a bit more cumbersome to define.

Copy link
Copy Markdown
Collaborator

@Hoverbear Hoverbear Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I'm not entirely sure I favor either way. One bother for me is that #[get_mut] #[deref] seems odd. It makes #[deref] into a no-op. Also #[deref] alone would be a no-op.

That said, I could see a legitimate use case under #21 .

Copy link
Copy Markdown
Collaborator

@Hoverbear Hoverbear May 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I thought about this more. I think we should take a conservative approach.

The goal of this library is to reduce the boilerplate associated with trivial getters and setters in a minimal fashion to support library evolution in a practical manner.

Notably: Field Access is not ABI stable across struct versions. Having Getters and Setters helps with stability.

Thankfully visibility modifiers are fairly easily tokenizable. We can create a simple set from the tokens present, and if deref (which is not a valid visibility modifer) is present we can do this deref implementation.

How does that sound to you?

I think ideal would be something like #[get(vis = "pub", deref)] but at least when I tried a when I first wrote this it didn't seem possible.

private: T,

/// Doc comments are supported!
Expand All @@ -34,6 +34,6 @@ fn main() {
let mut foo = Foo::default();
foo.set_private(1);
(*foo.private_mut()) += 1;
assert_eq!(*foo.private(), 2);
assert_eq!(foo.private(), 2);
}
```
193 changes: 123 additions & 70 deletions src/generate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use proc_macro2::TokenStream as TokenStream2;
use proc_macro2::{Ident, Span};
use proc_macro2::{Ident, Span, TokenStream as TokenStream2};
use syn::{Attribute, Field, Lit, Meta, MetaNameValue};

pub struct GenParams {
pub attribute_name: &'static str,
pub optional_attrs: &'static [&'static str],
pub fn_name_prefix: &'static str,
pub fn_name_suffix: &'static str,
}
Expand All @@ -22,7 +22,7 @@ pub fn implement(field: &Field, mode: GenMode, params: GenParams) -> TokenStream
let field_name = field
.clone()
.ident
.expect("Expected the field to have a name");
.expect("expected the field to have a name");
let fn_name = Ident::new(
&format!(
"{}{}{}",
Expand All @@ -32,107 +32,160 @@ pub fn implement(field: &Field, mode: GenMode, params: GenParams) -> TokenStream
);
let ty = field.ty.clone();

let attr = field
let attrs = field
.attrs
.iter()
.filter(|v| attr_name(v).expect("attribute") == params.attribute_name)
.last();
.filter(|v| {
let attr_name = attr_name(v).expect("expected attribute");
attr_name == params.attribute_name
|| params.optional_attrs.iter().any(|n| attr_name == *n)
})
.collect::<Vec<_>>();

let doc = field
.attrs
.iter()
.filter(|v| attr_name(v).expect("attribute") == "doc")
.filter(|v| attr_name(v).expect("expected attribute") == "doc")
.collect::<Vec<_>>();

match attr {
Some(attr) => {
match attr.interpret_meta() {
// `#[get]` or `#[set]`
Some(Meta::Word(_)) => match mode {
GenMode::Get => {
if attrs.is_empty() {
// Don't need to do anything.
quote! {}
} else {
match mode {
GenMode::Get => {
let deref = attrs
.iter()
.any(|v| attr_name(v).expect("expected attribute") == "deref");
let vis_attr = attrs
.iter()
.find(|v| attr_name(v).expect("expected attribute") == params.attribute_name)
.expect("no #[get] attribute found");
match vis_attr.interpret_meta() {
// `#[get]`
Some(Meta::Word(_)) => {
if deref {
quote! {
#(#doc)*
#[inline(always)]
fn #fn_name(&self) -> #ty {
self.#field_name
}
}
} else {
quote! {
#(#doc)*
#[inline(always)]
fn #fn_name(&self) -> &#ty {
&self.#field_name
}
}
}
}
// `#[get = "pub"]`
Some(Meta::NameValue(MetaNameValue {
lit: Lit::Str(ref s),
..
})) => {
let visibility = Ident::new(&s.value(), s.span());
if deref {
quote! {
#(#doc)*
#[inline(always)]
#visibility fn #fn_name(&self) -> #ty {
self.#field_name
}
}
} else {
quote! {
#(#doc)*
#[inline(always)]
#visibility fn #fn_name(&self) -> &#ty {
&self.#field_name
}
}
}
}
_ => panic!("unexpected attribute parameters"),
}
}
GenMode::Set => {
let attr = attrs[0];
match attr.interpret_meta() {
// `#[set]`
Some(Meta::Word(_)) => {
quote! {
#(#doc)*
#[inline(always)]
fn #fn_name(&self) -> &#ty {
&self.#field_name
fn #fn_name(&mut self, val: #ty) -> &mut Self {
self.#field_name = val;
self
}
}
}
GenMode::Set => {
// `#[set = "pub"]`
Some(Meta::NameValue(MetaNameValue {
lit: Lit::Str(ref s),
..
})) => {
let visibility = Ident::new(&s.value(), s.span());
quote! {
#(#doc)*
#[inline(always)]
fn #fn_name(&mut self, val: #ty) -> &mut Self {
#visibility fn #fn_name(&mut self, val: #ty) -> &mut Self {
self.#field_name = val;
self
}
}
}
GenMode::GetMut => {
_ => panic!("unexpected attribute parameters"),
}
}
GenMode::GetMut => {
let attr = attrs[0];
match attr.interpret_meta() {
// `#[get_mut]`
Some(Meta::Word(_)) => {
quote! {
#(#doc)*
fn #fn_name(&mut self) -> &mut #ty {
&mut self.#field_name
}
}
}
},
// `#[get = "pub"]` or `#[set = "pub"]`
Some(Meta::NameValue(MetaNameValue {
lit: Lit::Str(ref s),
..
})) => {
let visibility = Ident::new(&s.value(), s.span());
match mode {
GenMode::Get => {
quote! {
#(#doc)*
#[inline(always)]
#visibility fn #fn_name(&self) -> &#ty {
&self.#field_name
}
}
}
GenMode::Set => {
quote! {
#(#doc)*
#[inline(always)]
#visibility fn #fn_name(&mut self, val: #ty) -> &mut Self {
self.#field_name = val;
self
}
}
}
GenMode::GetMut => {
quote! {
#(#doc)*
#visibility fn #fn_name(&mut self) -> &mut #ty {
&mut self.#field_name
}
// `#[get_mut = "pub"]`
Some(Meta::NameValue(MetaNameValue {
lit: Lit::Str(ref s),
..
})) => {
let visibility = Ident::new(&s.value(), s.span());
quote! {
#(#doc)*
#visibility fn #fn_name(&mut self) -> &mut #ty {
&mut self.#field_name
}
}
}
_ => panic!("unexpected attribute parameters"),
// This currently doesn't work, but it might in the future.
// `#[get(pub)]`
// MetaItem::List(_, ref vec) => {
// let s = vec.iter().last().expect("No item found in attribute list.");
// let visibility = match s {
// &NestedMetaItem::MetaItem(MetaItem::Word(ref i)) => {
// Ident::new(format!("{}", i))
// }
// &NestedMetaItem::Literal(Lit::Str(ref l, _)) => Ident::from(l.clone()),
// _ => panic!("Unexpected attribute parameters."),
// };
// quote! {
// #visibility fn #fn_name(&self) -> &#ty {
// &self.#field_name
// }
// }
// }
}
// This currently doesn't work, but it might in the future.
//
// // `#[get(pub)]`
// MetaItem::List(_, ref vec) => {
// let s = vec.iter().last().expect("No item found in attribute list.");
// let visibility = match s {
// &NestedMetaItem::MetaItem(MetaItem::Word(ref i)) => Ident::new(format!("{}", i)),
// &NestedMetaItem::Literal(Lit::Str(ref l, _)) => Ident::from(l.clone()),
// _ => panic!("Unexpected attribute parameters."),
// };
// quote! {
// #visibility fn #fn_name(&self) -> &#ty {
// &self.#field_name
// }
// }
// },
_ => panic!("Unexpected attribute parameters."),
}
}
// Don't need to do anything.
None => quote! {},
}
}
11 changes: 7 additions & 4 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ extern crate getset;
pub struct Foo<T> where T: Copy + Clone + Default {
/// Doc comments are supported!
/// Multiline, even.
#[get] #[set] #[get_mut]
#[get] #[deref] #[set] #[get_mut]
private: T,

/// Doc comments are supported!
Expand All @@ -28,7 +28,7 @@ fn main() {
let mut foo = Foo::default();
foo.set_private(1);
(*foo.private_mut()) += 1;
assert_eq!(*foo.private(), 2);
assert_eq!(foo.private(), 2);
}
```
*/
Expand All @@ -46,7 +46,7 @@ use syn::{DataStruct, DeriveInput, Field};
mod generate;
use generate::{GenMode, GenParams};

#[proc_macro_derive(Getters, attributes(get))]
#[proc_macro_derive(Getters, attributes(get, deref))]
pub fn getters(input: TokenStream) -> TokenStream {
// Parse the string representation
let ast = syn::parse(input).expect("Couldn't parse for getters");
Expand All @@ -58,6 +58,7 @@ pub fn getters(input: TokenStream) -> TokenStream {
GenMode::Get,
GenParams {
attribute_name: "get",
optional_attrs: &["deref"],
fn_name_prefix: "",
fn_name_suffix: "",
},
Expand All @@ -71,14 +72,15 @@ pub fn getters(input: TokenStream) -> TokenStream {
#[proc_macro_derive(MutGetters, attributes(get_mut))]
pub fn mut_getters(input: TokenStream) -> TokenStream {
// Parse the string representation
let ast = syn::parse(input).expect("Couldn't parse for getters");
let ast = syn::parse(input).expect("Couldn't parse for mutable getters");
// Build the impl
let gen = produce(&ast, |f| {
generate::implement(
f,
GenMode::GetMut,
GenParams {
attribute_name: "get_mut",
optional_attrs: &[],
fn_name_prefix: "",
fn_name_suffix: "_mut",
},
Expand All @@ -100,6 +102,7 @@ pub fn setters(input: TokenStream) -> TokenStream {
GenMode::Set,
GenParams {
attribute_name: "set",
optional_attrs: &[],
fn_name_prefix: "set_",
fn_name_suffix: "",
},
Expand Down
Loading