From 131ef2f79475e5449062f91b6dc08356038a2c7e Mon Sep 17 00:00:00 2001 From: Noa Date: Wed, 11 Dec 2024 14:04:45 -0600 Subject: [PATCH] Tweak scheduled_at macro syntax --- crates/bindings-macro/src/lib.rs | 4 +- crates/bindings-macro/src/table.rs | 143 +++++++++++++---------- crates/bindings/tests/ui/reducers.rs | 1 - crates/bindings/tests/ui/reducers.stderr | 9 +- modules/rust-wasm-test/src/lib.rs | 1 - modules/sdk-test/src/lib.rs | 1 - smoketests/tests/modules.py | 1 - smoketests/tests/schedule_reducer.py | 16 ++- 8 files changed, 97 insertions(+), 79 deletions(-) diff --git a/crates/bindings-macro/src/lib.rs b/crates/bindings-macro/src/lib.rs index 8357650559..f9fa5df95e 100644 --- a/crates/bindings-macro/src/lib.rs +++ b/crates/bindings-macro/src/lib.rs @@ -31,6 +31,7 @@ mod sym { }; } + symbol!(at); symbol!(auto_inc); symbol!(btree); symbol!(client_connected); @@ -45,7 +46,6 @@ mod sym { symbol!(public); symbol!(sats); symbol!(scheduled); - symbol!(scheduled_at); symbol!(unique); symbol!(update); @@ -242,7 +242,7 @@ pub fn table(args: StdTokenStream, item: StdTokenStream) -> StdTokenStream { /// Provides helper attributes for `#[spacetimedb::table]`, so that we don't get unknown attribute errors. #[doc(hidden)] -#[proc_macro_derive(__TableHelper, attributes(sats, unique, auto_inc, primary_key, index, scheduled_at))] +#[proc_macro_derive(__TableHelper, attributes(sats, unique, auto_inc, primary_key, index))] pub fn table_helper(_input: StdTokenStream) -> StdTokenStream { Default::default() } diff --git a/crates/bindings-macro/src/table.rs b/crates/bindings-macro/src/table.rs index a4c0dfb1dc..2dbfa09db2 100644 --- a/crates/bindings-macro/src/table.rs +++ b/crates/bindings-macro/src/table.rs @@ -14,7 +14,7 @@ use syn::{parse_quote, Ident, Path, Token}; pub(crate) struct TableArgs { access: Option, - scheduled: Option, + scheduled: Option, name: Ident, indices: Vec, } @@ -36,6 +36,12 @@ impl TableAccess { } } +struct ScheduledArg { + span: Span, + reducer: Path, + at: Option, +} + struct IndexArg { name: Ident, kind: IndexType, @@ -70,9 +76,7 @@ impl TableArgs { sym::index => indices.push(IndexArg::parse_meta(meta)?), sym::scheduled => { check_duplicate(&scheduled, &meta)?; - let in_parens; - syn::parenthesized!(in_parens in meta.input); - scheduled = Some(in_parens.parse::()?); + scheduled = Some(ScheduledArg::parse_meta(meta)?); } }); Ok(()) @@ -94,6 +98,35 @@ impl TableArgs { } } +impl ScheduledArg { + fn parse_meta(meta: ParseNestedMeta) -> syn::Result { + let span = meta.path.span(); + let mut reducer = None; + let mut at = None; + + meta.parse_nested_meta(|meta| { + if meta.input.peek(syn::Token![=]) || meta.input.peek(syn::token::Paren) { + match_meta!(match meta { + sym::at => { + check_duplicate(&at, &meta)?; + let ident = meta.value()?.parse()?; + at = Some(ident); + } + }) + } else { + check_duplicate_msg(&reducer, &meta, "can only specify one scheduled reducer")?; + reducer = Some(meta.path); + } + Ok(()) + })?; + + let reducer = reducer.ok_or_else(|| { + meta.error("must specify scheduled reducer associated with the table: scheduled(reducer_name)") + })?; + Ok(Self { span, reducer, at }) + } +} + impl IndexArg { fn parse_meta(meta: ParseNestedMeta) -> syn::Result { let mut name = None; @@ -161,11 +194,7 @@ impl IndexArg { } fn validate<'a>(&'a self, table_name: &str, cols: &'a [Column<'a>]) -> syn::Result> { - let find_column = |ident| { - cols.iter() - .find(|col| col.field.ident == Some(ident)) - .ok_or_else(|| syn::Error::new(ident.span(), "not a column of the table")) - }; + let find_column = |ident| find_column(cols, ident); let kind = match &self.kind { IndexType::BTree { columns } => { let cols = columns.iter().map(find_column).collect::>>()?; @@ -353,12 +382,23 @@ struct Column<'a> { ty: &'a syn::Type, } +fn try_find_column<'a, 'b, T: ?Sized>(cols: &'a [Column<'b>], name: &T) -> Option<&'a Column<'b>> +where + Ident: PartialEq, +{ + cols.iter() + .find(|col| col.field.ident.is_some_and(|ident| ident == name)) +} + +fn find_column<'a, 'b>(cols: &'a [Column<'b>], name: &Ident) -> syn::Result<&'a Column<'b>> { + try_find_column(cols, name).ok_or_else(|| syn::Error::new(name.span(), "not a column of the table")) +} + enum ColumnAttr { Unique(Span), AutoInc(Span), PrimaryKey(Span), Index(IndexArg), - ScheduledAt(Span), } impl ColumnAttr { @@ -378,9 +418,6 @@ impl ColumnAttr { } else if ident == sym::primary_key { attr.meta.require_path_only()?; Some(ColumnAttr::PrimaryKey(ident.span())) - } else if ident == sym::scheduled_at { - attr.meta.require_path_only()?; - Some(ColumnAttr::ScheduledAt(ident.span())) } else { None }) @@ -427,7 +464,6 @@ pub(crate) fn table_impl(mut args: TableArgs, item: &syn::DeriveInput) -> syn::R let mut unique_columns = vec![]; let mut sequenced_columns = vec![]; let mut primary_key_column = None; - let mut scheduled_at_column = None; for (i, field) in fields.iter().enumerate() { let col_num = i as u16; @@ -436,7 +472,6 @@ pub(crate) fn table_impl(mut args: TableArgs, item: &syn::DeriveInput) -> syn::R let mut unique = None; let mut auto_inc = None; let mut primary_key = None; - let mut scheduled_at = None; for attr in field.original_attrs { let Some(attr) = ColumnAttr::parse(attr, field_ident)? else { continue; @@ -455,10 +490,6 @@ pub(crate) fn table_impl(mut args: TableArgs, item: &syn::DeriveInput) -> syn::R primary_key = Some(span); } ColumnAttr::Index(index_arg) => args.indices.push(index_arg), - ColumnAttr::ScheduledAt(span) => { - check_duplicate(&scheduled_at, span)?; - scheduled_at = Some(span); - } } } @@ -484,27 +515,10 @@ pub(crate) fn table_impl(mut args: TableArgs, item: &syn::DeriveInput) -> syn::R check_duplicate_msg(&primary_key_column, span, "can only have one primary key per table")?; primary_key_column = Some(column); } - if let Some(span) = scheduled_at { - check_duplicate_msg( - &scheduled_at_column, - span, - "can only have one scheduled_at column per table", - )?; - scheduled_at_column = Some(column); - } columns.push(column); } - let scheduled_at_typecheck = scheduled_at_column.map(|col| { - let ty = col.ty; - quote!(let _ = |x: #ty| { let _: spacetimedb::ScheduleAt = x; };) - }); - let scheduled_id_typecheck = primary_key_column.filter(|_| args.scheduled.is_some()).map(|col| { - let ty = col.ty; - quote!(spacetimedb::rt::assert_scheduled_table_primary_key::<#ty>();) - }); - let row_type = quote!(#original_struct_ident); let mut indices = args @@ -552,47 +566,60 @@ pub(crate) fn table_impl(mut args: TableArgs, item: &syn::DeriveInput) -> syn::R let primary_col_id = primary_key_column.iter().map(|col| col.index); let sequence_col_ids = sequenced_columns.iter().map(|col| col.index); - let scheduled_reducer_type_check = args.scheduled.as_ref().map(|reducer| { - quote! { - spacetimedb::rt::scheduled_reducer_typecheck::<#original_struct_ident>(#reducer); - } - }); - let schedule = args + let (schedule, schedule_typecheck) = args .scheduled .as_ref() - .map(|reducer| { + .map(|sched| { + let scheduled_at_column = match &sched.at { + Some(at) => Some(find_column(&columns, at)?), + None => try_find_column(&columns, "scheduled_at"), + }; // better error message when both are missing if scheduled_at_column.is_none() && primary_key_column.is_none() { return Err(syn::Error::new( - original_struct_ident.span(), + sched.span, "scheduled table missing required columns; add these to your struct:\n\ #[primary_key]\n\ #[auto_inc]\n\ scheduled_id: u64,\n\ - #[scheduled_at]\n\ scheduled_at: spacetimedb::ScheduleAt,", )); } let scheduled_at_column = scheduled_at_column.ok_or_else(|| { syn::Error::new( - original_struct_ident.span(), - "scheduled tables must have a `#[scheduled_at] scheduled_at: spacetimedb::ScheduleAt` column.", + sched.span, + "scheduled tables must have a `scheduled_at: spacetimedb::ScheduleAt` column. \ + if the column has a name besides `scheduled_at`, you can specify it with \ + `scheduled(my_reducer, at = custom_scheduled_at)`", + ) + })?; + let primary_key_column = primary_key_column.ok_or_else(|| { + syn::Error::new( + sched.span, + "scheduled tables must have a `#[primary_key] #[auto_inc] scheduled_id: u64` column", ) })?; + + let reducer = &sched.reducer; let scheduled_at_id = scheduled_at_column.index; - if primary_key_column.is_none() { - return Err(syn::Error::new( - original_struct_ident.span(), - "scheduled tables should have a `#[primary_key] #[auto_inc] scheduled_id: u64` column", - )); - } - Ok(quote!(spacetimedb::table::ScheduleDesc { + let desc = quote!(spacetimedb::table::ScheduleDesc { reducer_name: <#reducer as spacetimedb::rt::ReducerInfo>::NAME, scheduled_at_column: #scheduled_at_id, - })) + }); + + let primary_key_ty = primary_key_column.ty; + let scheduled_at_ty = scheduled_at_column.ty; + let typecheck = quote! { + spacetimedb::rt::scheduled_reducer_typecheck::<#original_struct_ident>(#reducer); + spacetimedb::rt::assert_scheduled_table_primary_key::<#primary_key_ty>(); + let _ = |x: #scheduled_at_ty| { let _: spacetimedb::ScheduleAt = x; }; + }; + + Ok((desc, typecheck)) }) .transpose()? - .into_iter(); + .unzip(); + let schedule = schedule.into_iter(); let unique_err = if !unique_columns.is_empty() { quote!(spacetimedb::UniqueConstraintViolation) @@ -665,9 +692,7 @@ pub(crate) fn table_impl(mut args: TableArgs, item: &syn::DeriveInput) -> syn::R let emission = quote! { const _: () = { #(let _ = <#field_types as spacetimedb::rt::TableColumn>::_ITEM;)* - #scheduled_reducer_type_check - #scheduled_at_typecheck - #scheduled_id_typecheck + #schedule_typecheck }; #trait_def diff --git a/crates/bindings/tests/ui/reducers.rs b/crates/bindings/tests/ui/reducers.rs index 404a4c588c..f35b22db27 100644 --- a/crates/bindings/tests/ui/reducers.rs +++ b/crates/bindings/tests/ui/reducers.rs @@ -39,7 +39,6 @@ struct ScheduledTable { #[primary_key] #[auto_inc] scheduled_id: u64, - #[scheduled_at] scheduled_at: spacetimedb::ScheduleAt, x: u8, y: u8, diff --git a/crates/bindings/tests/ui/reducers.stderr b/crates/bindings/tests/ui/reducers.stderr index c8277182b0..78b10d4ec1 100644 --- a/crates/bindings/tests/ui/reducers.stderr +++ b/crates/bindings/tests/ui/reducers.stderr @@ -14,12 +14,11 @@ error: scheduled table missing required columns; add these to your struct: #[primary_key] #[auto_inc] scheduled_id: u64, - #[scheduled_at] scheduled_at: spacetimedb::ScheduleAt, - --> tests/ui/reducers.rs:29:8 + --> tests/ui/reducers.rs:28:59 | -29 | struct ScheduledTableMissingRows { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ +28 | #[spacetimedb::table(name = scheduled_table_missing_rows, scheduled(scheduled_table_missing_rows_reducer))] + | ^^^^^^^^^ error[E0593]: function is expected to take 2 arguments, but it takes 3 arguments --> tests/ui/reducers.rs:37:56 @@ -30,7 +29,7 @@ error[E0593]: function is expected to take 2 arguments, but it takes 3 arguments | | expected function that takes 2 arguments | required by a bound introduced by this call ... -49 | fn scheduled_table_reducer(_ctx: &ReducerContext, _x: u8, _y: u8) {} +48 | fn scheduled_table_reducer(_ctx: &ReducerContext, _x: u8, _y: u8) {} | ----------------------------------------------------------------- takes 3 arguments | = note: required for `for<'a> fn(&'a ReducerContext, u8, u8) {scheduled_table_reducer}` to implement `Reducer<'_, (ScheduledTable,)>` diff --git a/modules/rust-wasm-test/src/lib.rs b/modules/rust-wasm-test/src/lib.rs index e80b2aa833..537994e83b 100644 --- a/modules/rust-wasm-test/src/lib.rs +++ b/modules/rust-wasm-test/src/lib.rs @@ -107,7 +107,6 @@ pub struct RepeatingTestArg { #[primary_key] #[auto_inc] scheduled_id: u64, - #[scheduled_at] scheduled_at: spacetimedb::ScheduleAt, prev_time: Timestamp, } diff --git a/modules/sdk-test/src/lib.rs b/modules/sdk-test/src/lib.rs index 529b160f5f..fe83c7a97a 100644 --- a/modules/sdk-test/src/lib.rs +++ b/modules/sdk-test/src/lib.rs @@ -647,7 +647,6 @@ pub struct ScheduledTable { #[primary_key] #[auto_inc] scheduled_id: u64, - #[scheduled_at] scheduled_at: spacetimedb::ScheduleAt, text: String, } diff --git a/smoketests/tests/modules.py b/smoketests/tests/modules.py index 37d42e9ee2..e93f6b4148 100644 --- a/smoketests/tests/modules.py +++ b/smoketests/tests/modules.py @@ -147,7 +147,6 @@ class UploadModule2(Smoketest): #[primary_key] #[auto_inc] scheduled_id: u64, - #[scheduled_at] scheduled_at: spacetimedb::ScheduleAt, prev: Timestamp, } diff --git a/smoketests/tests/schedule_reducer.py b/smoketests/tests/schedule_reducer.py index 1e1016ac51..1ef37e4840 100644 --- a/smoketests/tests/schedule_reducer.py +++ b/smoketests/tests/schedule_reducer.py @@ -28,7 +28,6 @@ class CancelReducer(Smoketest): #[primary_key] #[auto_inc] scheduled_id: u64, - #[scheduled_at] scheduled_at: spacetimedb::ScheduleAt, num: i32, } @@ -56,24 +55,23 @@ class SubscribeScheduledTable(Smoketest): MODULE_CODE = """ use spacetimedb::{log, duration, ReducerContext, Table, Timestamp}; -#[spacetimedb::table(name = scheduled_table, public, scheduled(my_reducer))] +#[spacetimedb::table(name = scheduled_table, public, scheduled(my_reducer, at = sched_at))] pub struct ScheduledTable { #[primary_key] #[auto_inc] scheduled_id: u64, - #[scheduled_at] - scheduled_at: spacetimedb::ScheduleAt, + sched_at: spacetimedb::ScheduleAt, prev: Timestamp, } #[spacetimedb::reducer] fn schedule_reducer(ctx: &ReducerContext) { - ctx.db.scheduled_table().insert(ScheduledTable { prev: Timestamp::from_micros_since_epoch(0), scheduled_id: 2, scheduled_at: Timestamp::from_micros_since_epoch(0).into(), }); + ctx.db.scheduled_table().insert(ScheduledTable { prev: Timestamp::from_micros_since_epoch(0), scheduled_id: 2, sched_at: Timestamp::from_micros_since_epoch(0).into(), }); } #[spacetimedb::reducer] fn schedule_repeated_reducer(ctx: &ReducerContext) { - ctx.db.scheduled_table().insert(ScheduledTable { prev: Timestamp::from_micros_since_epoch(0), scheduled_id: 1, scheduled_at: duration!(100ms).into(), }); + ctx.db.scheduled_table().insert(ScheduledTable { prev: Timestamp::from_micros_since_epoch(0), scheduled_id: 1, sched_at: duration!(100ms).into(), }); } #[spacetimedb::reducer] @@ -93,7 +91,7 @@ def test_scheduled_table_subscription(self): # scheduled reducer should be ran by now self.assertEqual(lines, 1) - row_entry = {'prev': 0, 'scheduled_id': 2, 'scheduled_at': {'Time': 0}} + row_entry = {'prev': 0, 'scheduled_id': 2, 'sched_at': {'Time': 0}} # subscription should have 2 updates, first for row insert in scheduled table and second for row deletion. self.assertEqual(sub(), [{'scheduled_table': {'deletes': [], 'inserts': [row_entry]}}, {'scheduled_table': {'deletes': [row_entry], 'inserts': []}}]) @@ -114,8 +112,8 @@ def test_scheduled_table_subscription_repeated_reducer(self): # scheduling repeated reducer again just to get 2nd subscription update. self.call("schedule_reducer") - repeated_row_entry = {'prev': 0, 'scheduled_id': 1, 'scheduled_at': {'Interval': 100000}} - row_entry = {'prev': 0, 'scheduled_id': 2, 'scheduled_at': {'Time': 0}} + repeated_row_entry = {'prev': 0, 'scheduled_id': 1, 'sched_at': {'Interval': 100000}} + row_entry = {'prev': 0, 'scheduled_id': 2, 'sched_at': {'Time': 0}} # subscription should have 2 updates and should not have any deletes self.assertEqual(sub(), [{'scheduled_table': {'deletes': [], 'inserts': [repeated_row_entry]}}, {'scheduled_table': {'deletes': [], 'inserts': [row_entry]}}])