From f53613f8ae3ae8892b52d85136b44aef5b854228 Mon Sep 17 00:00:00 2001 From: Noa Date: Wed, 23 Oct 2024 13:03:05 -0500 Subject: [PATCH] Don't autogen schedule fields --- crates/bindings-macro/src/lib.rs | 5 +- crates/bindings-macro/src/table.rs | 113 +++++++++++------- crates/bindings-macro/src/util.rs | 33 +---- crates/bindings/src/table.rs | 13 -- crates/bindings/tests/ui/reducers.rs | 14 +++ crates/bindings/tests/ui/reducers.stderr | 17 ++- .../snapshots/codegen__codegen_csharp.snap | 10 +- .../snapshots/codegen__codegen_rust.snap | 2 +- .../codegen__codegen_typescript.snap | 4 +- modules/rust-wasm-test/src/lib.rs | 5 + smoketests/tests/modules.py | 5 + smoketests/tests/schedule_reducer.py | 10 ++ 12 files changed, 129 insertions(+), 102 deletions(-) diff --git a/crates/bindings-macro/src/lib.rs b/crates/bindings-macro/src/lib.rs index 07a88e18eec..8357650559c 100644 --- a/crates/bindings-macro/src/lib.rs +++ b/crates/bindings-macro/src/lib.rs @@ -45,6 +45,7 @@ mod sym { symbol!(public); symbol!(sats); symbol!(scheduled); + symbol!(scheduled_at); symbol!(unique); symbol!(update); @@ -154,7 +155,7 @@ mod sym { pub fn reducer(args: StdTokenStream, item: StdTokenStream) -> StdTokenStream { cvt_attr::(args, item, quote!(), |args, original_function| { let args = reducer::ReducerArgs::parse(args)?; - reducer::reducer_impl(args, &original_function) + reducer::reducer_impl(args, original_function) }) } @@ -241,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))] +#[proc_macro_derive(__TableHelper, attributes(sats, unique, auto_inc, primary_key, index, scheduled_at))] 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 004e512e512..1bb6ddbf1f3 100644 --- a/crates/bindings-macro/src/table.rs +++ b/crates/bindings-macro/src/table.rs @@ -1,6 +1,6 @@ use crate::sats::{self, derive_deserialize, derive_satstype, derive_serialize}; use crate::sym; -use crate::util::{check_duplicate, check_duplicate_msg, ident_to_litstr, match_meta, MutItem}; +use crate::util::{check_duplicate, check_duplicate_msg, ident_to_litstr, match_meta}; use heck::ToSnakeCase; use proc_macro2::{Span, TokenStream}; use quote::{format_ident, quote, quote_spanned, ToTokens}; @@ -36,21 +36,6 @@ impl TableAccess { } } -// add scheduled_id and scheduled_at fields to the struct -fn add_scheduled_fields(item: &mut syn::DeriveInput) { - if let syn::Data::Struct(struct_data) = &mut item.data { - if let syn::Fields::Named(fields) = &mut struct_data.fields { - let extra_fields: syn::FieldsNamed = parse_quote!({ - #[primary_key] - #[auto_inc] - pub scheduled_id: u64, - pub scheduled_at: spacetimedb::spacetimedb_lib::ScheduleAt, - }); - fields.named.extend(extra_fields.named); - } - } -} - struct IndexArg { name: Ident, kind: IndexType, @@ -365,6 +350,7 @@ enum ColumnAttr { AutoInc(Span), PrimaryKey(Span), Index(IndexArg), + ScheduledAt(Span), } impl ColumnAttr { @@ -384,23 +370,18 @@ 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 }) } } -pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem) -> syn::Result { - let scheduled_reducer_type_check = args.scheduled.as_ref().map(|reducer| { - add_scheduled_fields(&mut item); - let struct_name = &item.ident; - quote! { - const _: () = spacetimedb::rt::scheduled_reducer_typecheck::<#struct_name>(#reducer); - } - }); - +pub(crate) fn table_impl(mut args: TableArgs, item: &syn::DeriveInput) -> syn::Result { let vis = &item.vis; - let sats_ty = sats::sats_type_from_derive(&item, quote!(spacetimedb::spacetimedb_lib))?; + let sats_ty = sats::sats_type_from_derive(item, quote!(spacetimedb::spacetimedb_lib))?; let original_struct_ident = sats_ty.ident; let table_ident = &args.name; @@ -429,7 +410,7 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem u16::MAX.into() { return Err(syn::Error::new_spanned( - &*item, + item, "too many columns; the most a table can have is 2^16", )); } @@ -438,6 +419,7 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem args.indices.push(index_arg), + ColumnAttr::ScheduledAt(span) => { + check_duplicate(&scheduled_at, span)?; + scheduled_at = Some(span); + } } } @@ -489,10 +476,27 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem();) + }); + let row_type = quote!(#original_struct_ident); let mut indices = args @@ -543,17 +547,46 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem(#reducer); + } + }); let schedule = args .scheduled .as_ref() .map(|reducer| { - // scheduled_at was inserted as the last field - let scheduled_at_id = (fields.len() - 1) as u16; - quote!(spacetimedb::table::ScheduleDesc { + // 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(), + "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.", + ) + })?; + 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 { reducer_name: <#reducer as spacetimedb::rt::ReducerInfo>::NAME, scheduled_at_column: #scheduled_at_id, - }) + })) }) + .transpose()? .into_iter(); let unique_err = if !unique_columns.is_empty() { @@ -567,7 +600,6 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem>(); let field_types = fields.iter().map(|f| f.ty).collect::>(); let tabletype_impl = quote! { @@ -602,16 +634,6 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem for #original_struct_ident { - type Field = #field_types; - fn get_field(&self) -> &Self::Field { - &self.#field_names - } - })* - }; - let row_type_to_table = quote!(<#row_type as spacetimedb::table::__MapRowTypeToTable>::Table); // Output all macro data @@ -638,6 +660,9 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem::_ITEM;)* + #scheduled_reducer_type_check + #scheduled_at_typecheck + #scheduled_id_typecheck }; #trait_def @@ -672,10 +697,6 @@ pub(crate) fn table_impl(mut args: TableArgs, mut item: MutItem( args: StdTokenStream, item: StdTokenStream, extra_attr: TokenStream, - f: impl FnOnce(TokenStream, MutItem<'_, Item>) -> syn::Result, + f: impl FnOnce(TokenStream, &Item) -> syn::Result, ) -> StdTokenStream { let item: TokenStream = item.into(); - let mut parsed_item = match syn::parse2::(item.clone()) { + let parsed_item = match syn::parse2::(item.clone()) { Ok(i) => i, Err(e) => return TokenStream::from_iter([item, e.into_compile_error()]).into(), }; - let mut modified = false; - let mut_item = MutItem { - val: &mut parsed_item, - modified: &mut modified, - }; - let generated = f(args.into(), mut_item).unwrap_or_else(syn::Error::into_compile_error); - let item = if modified { - parsed_item.into_token_stream() - } else { - item - }; + let generated = f(args.into(), &parsed_item).unwrap_or_else(syn::Error::into_compile_error); TokenStream::from_iter([extra_attr, item, generated]).into() } @@ -35,23 +25,6 @@ pub(crate) fn ident_to_litstr(ident: &Ident) -> syn::LitStr { syn::LitStr::new(&ident.to_string(), ident.span()) } -pub(crate) struct MutItem<'a, T> { - val: &'a mut T, - modified: &'a mut bool, -} -impl std::ops::Deref for MutItem<'_, T> { - type Target = T; - fn deref(&self) -> &Self::Target { - self.val - } -} -impl std::ops::DerefMut for MutItem<'_, T> { - fn deref_mut(&mut self) -> &mut Self::Target { - *self.modified = true; - self.val - } -} - pub(crate) trait ErrorSource { fn error(self, msg: impl std::fmt::Display) -> syn::Error; } diff --git a/crates/bindings/src/table.rs b/crates/bindings/src/table.rs index 2e8a1256a72..0ae6355d574 100644 --- a/crates/bindings/src/table.rs +++ b/crates/bindings/src/table.rs @@ -262,19 +262,6 @@ impl MaybeError for AutoIncOverflow { } } -/// A trait for types exposing an operation to access their `N`th field. -/// -/// In other words, a type implementing `FieldAccess` allows -/// shared projection from `self` to its `N`th field. -#[doc(hidden)] -pub trait FieldAccess { - /// The type of the field at the `N`th position. - type Field; - - /// Project to the value of the field at position `N`. - fn get_field(&self) -> &Self::Field; -} - pub trait Column { type Row; type ColType; diff --git a/crates/bindings/tests/ui/reducers.rs b/crates/bindings/tests/ui/reducers.rs index 782f6355248..404a4c588cc 100644 --- a/crates/bindings/tests/ui/reducers.rs +++ b/crates/bindings/tests/ui/reducers.rs @@ -25,8 +25,22 @@ fn missing_ctx(_a: u8) {} #[spacetimedb::reducer] fn ctx_by_val(_ctx: ReducerContext, _a: u8) {} +#[spacetimedb::table(name = scheduled_table_missing_rows, scheduled(scheduled_table_missing_rows_reducer))] +struct ScheduledTableMissingRows { + x: u8, + y: u8, +} + +// #[spacetimedb::reducer] +// fn scheduled_table_missing_rows_reducer(_ctx: &ReducerContext, _: &ScheduledTableMissingRows) {} + #[spacetimedb::table(name = scheduled_table, scheduled(scheduled_table_reducer))] 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 bb48b5a371b..c8277182b03 100644 --- a/crates/bindings/tests/ui/reducers.stderr +++ b/crates/bindings/tests/ui/reducers.stderr @@ -10,16 +10,27 @@ error: const parameters are not allowed on reducers 20 | fn const_param() {} | ^^^^^^^^^^^ +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 + | +29 | struct ScheduledTableMissingRows { + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + error[E0593]: function is expected to take 2 arguments, but it takes 3 arguments - --> tests/ui/reducers.rs:28:56 + --> tests/ui/reducers.rs:37:56 | -28 | #[spacetimedb::table(name = scheduled_table, scheduled(scheduled_table_reducer))] +37 | #[spacetimedb::table(name = scheduled_table, scheduled(scheduled_table_reducer))] | -------------------------------------------------------^^^^^^^^^^^^^^^^^^^^^^^--- | | | | | expected function that takes 2 arguments | required by a bound introduced by this call ... -35 | fn scheduled_table_reducer(_ctx: &ReducerContext, _x: u8, _y: u8) {} +49 | 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/crates/cli/tests/snapshots/codegen__codegen_csharp.snap b/crates/cli/tests/snapshots/codegen__codegen_csharp.snap index 163116ed327..635be2136c9 100644 --- a/crates/cli/tests/snapshots/codegen__codegen_csharp.snap +++ b/crates/cli/tests/snapshots/codegen__codegen_csharp.snap @@ -372,22 +372,22 @@ namespace SpacetimeDB [DataContract] public partial class RepeatingTestArg : IDatabaseRow { - [DataMember(Name = "prev_time")] - public ulong PrevTime; [DataMember(Name = "scheduled_id")] public ulong ScheduledId; [DataMember(Name = "scheduled_at")] public SpacetimeDB.ScheduleAt ScheduledAt; + [DataMember(Name = "prev_time")] + public ulong PrevTime; public RepeatingTestArg( - ulong PrevTime, ulong ScheduledId, - SpacetimeDB.ScheduleAt ScheduledAt + SpacetimeDB.ScheduleAt ScheduledAt, + ulong PrevTime ) { - this.PrevTime = PrevTime; this.ScheduledId = ScheduledId; this.ScheduledAt = ScheduledAt; + this.PrevTime = PrevTime; } public RepeatingTestArg() diff --git a/crates/cli/tests/snapshots/codegen__codegen_rust.snap b/crates/cli/tests/snapshots/codegen__codegen_rust.snap index fb933f772ff..5ec56f4e8bc 100644 --- a/crates/cli/tests/snapshots/codegen__codegen_rust.snap +++ b/crates/cli/tests/snapshots/codegen__codegen_rust.snap @@ -1825,9 +1825,9 @@ pub(super) fn parse_table_update( #[derive(__lib::ser::Serialize, __lib::de::Deserialize, Clone, PartialEq, Debug)] #[sats(crate = __lib)] pub struct RepeatingTestArg { - pub prev_time: u64, pub scheduled_id: u64, pub scheduled_at: __sdk::ScheduleAt, + pub prev_time: u64, } diff --git a/crates/cli/tests/snapshots/codegen__codegen_typescript.snap b/crates/cli/tests/snapshots/codegen__codegen_typescript.snap index 11ca97edb36..0b5a7f6767c 100644 --- a/crates/cli/tests/snapshots/codegen__codegen_typescript.snap +++ b/crates/cli/tests/snapshots/codegen__codegen_typescript.snap @@ -2065,9 +2065,9 @@ import { deepEqual, } from "@clockworklabs/spacetimedb-sdk"; export type RepeatingTestArg = { - prevTime: bigint, scheduledId: bigint, scheduledAt: { tag: "Interval", value: bigint } | { tag: "Time", value: bigint }, + prevTime: bigint, }; /** @@ -2080,9 +2080,9 @@ export namespace RepeatingTestArg { */ export function getTypeScriptAlgebraicType(): AlgebraicType { return AlgebraicType.createProductType([ - new ProductTypeElement("prevTime", AlgebraicType.createU64Type()), new ProductTypeElement("scheduledId", AlgebraicType.createU64Type()), new ProductTypeElement("scheduledAt", AlgebraicType.createScheduleAtType()), + new ProductTypeElement("prevTime", AlgebraicType.createU64Type()), ]); } diff --git a/modules/rust-wasm-test/src/lib.rs b/modules/rust-wasm-test/src/lib.rs index cfc73fc7b23..e80b2aa8336 100644 --- a/modules/rust-wasm-test/src/lib.rs +++ b/modules/rust-wasm-test/src/lib.rs @@ -104,6 +104,11 @@ pub type TestAlias = TestA; #[spacetimedb::table(name = repeating_test_arg, scheduled(repeating_test))] pub struct RepeatingTestArg { + #[primary_key] + #[auto_inc] + scheduled_id: u64, + #[scheduled_at] + scheduled_at: spacetimedb::ScheduleAt, prev_time: Timestamp, } diff --git a/smoketests/tests/modules.py b/smoketests/tests/modules.py index e9a7e5f3025..203a6e2a16c 100644 --- a/smoketests/tests/modules.py +++ b/smoketests/tests/modules.py @@ -144,6 +144,11 @@ class UploadModule2(Smoketest): #[spacetimedb::table(name = scheduled_message, public, scheduled(my_repeating_reducer))] pub struct ScheduledMessage { + #[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 0b80d0f5c4e..01f6e57efdf 100644 --- a/smoketests/tests/schedule_reducer.py +++ b/smoketests/tests/schedule_reducer.py @@ -25,6 +25,11 @@ class CancelReducer(Smoketest): #[spacetimedb::table(name = scheduled_reducer_args, public, scheduled(reducer))] pub struct ScheduledReducerArgs { + #[primary_key] + #[auto_inc] + scheduled_id: u64, + #[scheduled_at] + scheduled_at: spacetimedb::ScheduleAt, num: i32, } @@ -53,6 +58,11 @@ class SubscribeScheduledTable(Smoketest): #[spacetimedb::table(name = scheduled_table, public, scheduled(my_reducer))] pub struct ScheduledTable { + #[primary_key] + #[auto_inc] + scheduled_id: u64, + #[scheduled_at] + scheduled_at: spacetimedb::ScheduleAt, prev: Timestamp, }