Skip to content
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.

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

Add #[data(eq)] shorthand attribute for Data derive macro #1884

Merged
merged 5 commits into from
Aug 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ You can find its changes [documented below](#070---2021-01-01).
- Expose `RawWindowHandle` for `WindowHandle` under the `raw-win-handle` feature ([#1828] by [@djeedai])
- `Slider` widget now warns if max < min and swaps the values ([#1882] by [@Maan2003])
- Widget/Slider: Add stepping functionality ([#1875] by [@raymanfx])
- Add #[data(eq)] shorthand attribute for Data derive macro ([#1884] by [@Maan2003])

### Changed

Expand Down Expand Up @@ -771,6 +772,7 @@ Last release without a changelog :(
[#1873]: https://github.com/linebender/druid/pull/1873
[#1876]: https://github.com/linebender/druid/pull/1876
[#1882]: https://github.com/linebender/druid/pull/1882
[#1884]: https://github.com/linebender/druid/pull/1884
[#1885]: https://github.com/linebender/druid/pull/1885
[#1886]: https://github.com/linebender/druid/pull/1886

Expand Down
58 changes: 30 additions & 28 deletions druid-derive/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const BASE_DATA_ATTR_PATH: &str = "data";
const BASE_LENS_ATTR_PATH: &str = "lens";
const IGNORE_ATTR_PATH: &str = "ignore";
const DATA_SAME_FN_ATTR_PATH: &str = "same_fn";
const DATA_EQ_ATTR_PATH: &str = "eq";
const LENS_NAME_OVERRIDE_ATTR_PATH: &str = "name";

/// The fields for a struct or an enum variant.
Expand Down Expand Up @@ -67,11 +68,12 @@ pub struct Field<Attrs> {
pub attrs: Attrs,
}

#[derive(Debug)]
pub struct DataAttrs {
/// `true` if this field should be ignored.
pub ignore: bool,
pub same_fn: Option<ExprPath>,
#[derive(Debug, PartialEq)]
pub enum DataAttr {
Empty,
Ignore,
SameFn(ExprPath),
Eq,
}

#[derive(Debug)]
Expand All @@ -81,7 +83,7 @@ pub struct LensAttrs {
pub lens_name_override: Option<Ident>,
}

impl Fields<DataAttrs> {
impl Fields<DataAttr> {
pub fn parse_ast(fields: &syn::Fields) -> Result<Self, Error> {
let kind = match fields {
syn::Fields::Named(_) => FieldKind::Named,
Expand All @@ -91,7 +93,7 @@ impl Fields<DataAttrs> {
let fields = fields
.iter()
.enumerate()
.map(|(i, field)| Field::<DataAttrs>::parse_ast(field, i))
.map(|(i, field)| Field::<DataAttr>::parse_ast(field, i))
.collect::<Result<Vec<_>, _>>()?;

Ok(Fields { kind, fields })
Expand Down Expand Up @@ -125,7 +127,7 @@ impl<Attrs> Fields<Attrs> {
}
}

impl Field<DataAttrs> {
impl Field<DataAttr> {
pub fn parse_ast(field: &syn::Field, index: usize) -> Result<Self, Error> {
let ident = match field.ident.as_ref() {
Some(ident) => FieldIdent::Named(ident.to_string().trim_start_matches("r#").to_owned()),
Expand All @@ -134,9 +136,7 @@ impl Field<DataAttrs> {

let ty = field.ty.clone();

let mut ignore = false;
let mut same_fn = None;

let mut data_attr = DataAttr::Empty;
for attr in field.attrs.iter() {
if attr.path.is_ident(BASE_DRUID_DEPRECATED_ATTR_PATH) {
panic!(
Expand All @@ -146,28 +146,27 @@ impl Field<DataAttrs> {
} else if attr.path.is_ident(BASE_DATA_ATTR_PATH) {
match attr.parse_meta()? {
Meta::List(meta) => {
for nested in meta.nested.iter() {
assert!(
meta.nested.len() <= 1,
"only single data attribute is allowed"
);
if let Some(nested) = meta.nested.first() {
match nested {
NestedMeta::Meta(Meta::Path(path))
if path.is_ident(IGNORE_ATTR_PATH) =>
{
if ignore {
return Err(Error::new(
nested.span(),
"Duplicate attribute",
));
}
ignore = true;
data_attr = DataAttr::Ignore;
}
NestedMeta::Meta(Meta::NameValue(meta))
if meta.path.is_ident(DATA_SAME_FN_ATTR_PATH) =>
{
if same_fn.is_some() {
return Err(Error::new(meta.span(), "Duplicate attribute"));
}

let path = parse_lit_into_expr_path(&meta.lit)?;
same_fn = Some(path);
data_attr = DataAttr::SameFn(path);
}
NestedMeta::Meta(Meta::Path(path))
if path.is_ident(DATA_EQ_ATTR_PATH) =>
{
data_attr = DataAttr::Eq;
}
other => return Err(Error::new(other.span(), "Unknown attribute")),
}
Expand All @@ -185,15 +184,18 @@ impl Field<DataAttrs> {
Ok(Field {
ident,
ty,
attrs: DataAttrs { ignore, same_fn },
attrs: data_attr,
})
}

/// The tokens to be used as the function for 'same'.
pub fn same_fn_path_tokens(&self) -> TokenStream {
match self.attrs.same_fn {
Some(ref f) => quote!(#f),
None => {
match &self.attrs {
DataAttr::SameFn(f) => quote!(#f),
DataAttr::Eq => quote!(::core::cmp::PartialEq::eq),
// this should not be called for DataAttr::Ignore
DataAttr::Ignore => quote!(compiler_error!),
DataAttr::Empty => {
let span = Span::call_site();
quote_spanned!(span=> druid::Data::same)
}
Expand Down
12 changes: 6 additions & 6 deletions druid-derive/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

//! The implementation for #[derive(Data)]

use crate::attr::{DataAttrs, Field, FieldKind, Fields};
use crate::attr::{DataAttr, Field, FieldKind, Fields};

use quote::{quote, quote_spanned};
use syn::{spanned::Spanned, Data, DataEnum, DataStruct};
Expand All @@ -40,16 +40,16 @@ fn derive_struct(
let impl_generics = generics_bounds(&input.generics);
let (_, ty_generics, where_clause) = &input.generics.split_for_impl();

let fields = Fields::<DataAttrs>::parse_ast(&s.fields)?;
let fields = Fields::<DataAttr>::parse_ast(&s.fields)?;

let diff = if fields.len() > 0 {
let same_fns = fields
.iter()
.filter(|f| !f.attrs.ignore)
.filter(|f| f.attrs != DataAttr::Ignore)
.map(Field::same_fn_path_tokens);
let fields = fields
.iter()
.filter(|f| !f.attrs.ignore)
.filter(|f| f.attrs != DataAttr::Ignore)
.map(Field::ident_tokens);
quote!( #( #same_fns(&self.#fields, &other.#fields) )&&* )
} else {
Expand Down Expand Up @@ -100,13 +100,13 @@ fn derive_enum(
.variants
.iter()
.map(|variant| {
let fields = Fields::<DataAttrs>::parse_ast(&variant.fields)?;
let fields = Fields::<DataAttr>::parse_ast(&variant.fields)?;
let variant = &variant.ident;

// the various inner `same()` calls, to the right of the match arm.
let tests: Vec<_> = fields
.iter()
.filter(|field| !field.attrs.ignore)
.filter(|f| f.attrs != DataAttr::Ignore)
.map(|field| {
let same_fn = field.same_fn_path_tokens();
let var_left = ident_from_str(&format!("__self_{}", field.ident_string()));
Expand Down
4 changes: 3 additions & 1 deletion druid-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use syn::parse_macro_input;
/// - `#[data(same_fn="foo")]` uses the function `foo` for comparing this field. `foo` should
/// be the name of a function with signature `fn(&T, &T) -> bool`, where `T` is the type of
/// the field.
/// - `#[data(eq)]` is shorthand for `#[data(same_fn = "PartialEq::eq")]`
///
/// # Example
///
Expand All @@ -43,7 +44,8 @@ use syn::parse_macro_input;
/// struct State {
/// number: f64,
/// // `Vec` doesn't implement `Data`, so we need to either ignore it or supply a `same_fn`.
/// #[data(same_fn="PartialEq::eq")]
/// #[data(eq)]
/// // same as #[data(same_fn="PartialEq::eq")]
/// indices: Vec<usize>,
/// // This is just some sort of cache; it isn't important for sameness comparison.
/// #[data(ignore)]
Expand Down
25 changes: 25 additions & 0 deletions druid-derive/tests/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,28 @@ fn test_data_derive_same() {
assert!(v.same(&v));
assert!(!v.same(&TypeParamForUserTraitAndLifetimeEnum::V1(Value(12))));
}

#[derive(Data, Clone)]
struct DataAttrEq {
#[data(eq)]
maan2003 marked this conversation as resolved.
Show resolved Hide resolved
f: PanicOnPartialEq,
}

#[derive(Clone, Copy)]
struct PanicOnPartialEq;
impl PartialEq for PanicOnPartialEq {
fn eq(&self, _other: &Self) -> bool {
panic!("PartialEq::eq called");
}
}

#[test]
#[should_panic = "PartialEq::eq called"]
fn data_attr_eq() {
DataAttrEq {
f: PanicOnPartialEq,
}
.same(&DataAttrEq {
f: PanicOnPartialEq,
});
}
2 changes: 1 addition & 1 deletion druid-derive/tests/with_same.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ fn same_fn() {
#[derive(Clone, Data)]
struct Nanana {
bits: f64,
#[data(same_fn = "PartialEq::eq")]
#[data(eq)]
peq: f64,
}

Expand Down
2 changes: 1 addition & 1 deletion druid/examples/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ struct CircleView;
#[derive(Clone, Data)]
struct Circle {
pos: Point,
#[data(same_fn = "PartialEq::eq")]
#[data(eq)]
time: Instant,
}

Expand Down
2 changes: 1 addition & 1 deletion druid/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ use piet::ImageBuf;
/// #[derive(Clone, Data)]
/// struct PathEntry {
/// // There's no Data impl for PathBuf, but no problem
/// #[data(same_fn = "PartialEq::eq")]
/// #[data(eq)]
/// path: PathBuf,
/// priority: usize,
/// // This field is not part of our data model.
Expand Down