Improve bindings diagnostics and add ui tests

Cargo.toml

@@ -240,6 +240,7 @@ tracing-core = "0.1.31"
 tracing-flame = "0.2.0"
 tracing-log = "0.1.3"
 tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
+trybuild = "1"
 typed-arena = "2.0"
 unicode-normalization = "0.1.23"
 unicode-ident = "1.0.12"

crates/bindings-macro/src/lib.rs

@@ -17,7 +17,6 @@ use proc_macro::TokenStream as StdTokenStream;
 use proc_macro2::{Span, TokenStream};
 use quote::{format_ident, quote, quote_spanned, ToTokens};
 use std::borrow::Cow;
-use std::collections::HashMap;
 use std::time::Duration;
 use syn::ext::IdentExt;
 use syn::meta::ParseNestedMeta;
@@ -330,6 +329,15 @@ fn reducer_impl(args: ReducerArgs, original_function: &ItemFn) -> syn::Result<To
         }
     };
 
+    for param in &original_function.sig.generics.params {
+        let err = |msg| syn::Error::new_spanned(param, msg);
+        match param {
+            syn::GenericParam::Lifetime(_) => {}
+            syn::GenericParam::Type(_) => return Err(err("type parameters are not allowed on reducers")),
+            syn::GenericParam::Const(_) => return Err(err("const parameters are not allowed on reducers")),
+        }
+    }
+
     let lifecycle = args.lifecycle.iter().filter_map(|lc| lc.to_lifecycle_value());
 
     // Extract all function parameters, except for `self` ones that aren't allowed.
@@ -354,6 +362,8 @@ fn reducer_impl(args: ReducerArgs, original_function: &ItemFn) -> syn::Result<To
     });
 
     let arg_tys = typed_args.iter().map(|arg| arg.ty.as_ref()).collect::<Vec<_>>();
+    let first_arg_ty = arg_tys.first().into_iter();
+    let rest_arg_tys = arg_tys.iter().skip(1);
 
     // Extract the return type.
     let ret_ty = match &original_function.sig.output {
@@ -364,13 +374,8 @@ fn reducer_impl(args: ReducerArgs, original_function: &ItemFn) -> syn::Result<To
 
     let register_describer_symbol = format!("__preinit__20_register_describer_{reducer_name}");
 
-    let generated_function = quote! {
-        fn __reducer(__ctx: spacetimedb::ReducerContext, __args: &[u8]) -> spacetimedb::ReducerResult {
-            #(spacetimedb::rt::assert_reducer_arg::<#arg_tys>();)*
-            #(spacetimedb::rt::assert_reducer_ret::<#ret_ty>();)*
-            spacetimedb::rt::invoke_reducer(#func_name, __ctx, __args)
-        }
-    };
+    let lt_params = &original_function.sig.generics;
+    let lt_where_clause = &lt_params.where_clause;
 
     let generated_describe_function = quote! {
         #[export_name = #register_describer_symbol]
@@ -385,14 +390,24 @@ fn reducer_impl(args: ReducerArgs, original_function: &ItemFn) -> syn::Result<To
         };
         #[allow(non_camel_case_types)]
         #vis struct #func_name { _never: ::core::convert::Infallible }
+        const _: () = {
+            fn _assert_args #lt_params () #lt_where_clause {
+                #(let _ = <#first_arg_ty as spacetimedb::rt::ReducerContextArg>::_ITEM;)*
+                #(let _ = <#rest_arg_tys as spacetimedb::rt::ReducerArg>::_ITEM;)*
+                #(let _ = <#ret_ty as spacetimedb::rt::IntoReducerResult>::into_result;)*
+            }
+        };
+        impl #func_name {
+            fn invoke(__ctx: spacetimedb::ReducerContext, __args: &[u8]) -> spacetimedb::ReducerResult {
+                spacetimedb::rt::invoke_reducer(#func_name, __ctx, __args)
+            }
+        }
+        #[automatically_derived]
         impl spacetimedb::rt::ReducerInfo for #func_name {
             const NAME: &'static str = #reducer_name;
             #(const LIFECYCLE: Option<spacetimedb::rt::LifecycleReducer> = Some(#lifecycle);)*
             const ARG_NAMES: &'static [Option<&'static str>] = &[#(#opt_arg_names),*];
-            const INVOKE: spacetimedb::rt::ReducerFn = {
-                #generated_function
-                __reducer
-            };
+            const INVOKE: spacetimedb::rt::ReducerFn = #func_name::invoke;
         }
     })
 }
@@ -436,15 +451,6 @@ fn add_scheduled_fields(item: &mut syn::DeriveInput) {
     }
 }
 
-/// Check if the Identifier provided in `scheduled()` is a valid reducer
-/// generate a function that tried to call reducer passing `ReducerContext`
-fn reducer_type_check(item: &syn::DeriveInput, reducer_name: &Path) -> TokenStream {
-    let struct_name = &item.ident;
-    quote! {
-        const _: () = spacetimedb::rt::assert_reducer_typecheck::<(#struct_name,)>(#reducer_name);
-    }
-}
-
 struct IndexArg {
     name: Ident,
     kind: IndexType,
@@ -874,7 +880,10 @@ impl ColumnAttr {
 fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::Result<TokenStream> {
     let scheduled_reducer_type_check = args.scheduled.as_ref().map(|reducer| {
         add_scheduled_fields(&mut item);
-        reducer_type_check(&item, reducer)
+        let struct_name = &item.ident;
+        quote! {
+            const _: () = spacetimedb::rt::scheduled_reducer_typecheck::<#struct_name>(#reducer);
+        }
     });
 
     let vis = &item.vis;
@@ -887,6 +896,15 @@ fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::
         return Err(syn::Error::new(Span::call_site(), "spacetimedb table must be a struct"));
     };
 
+    for param in &item.generics.params {
+        let err = |msg| syn::Error::new_spanned(param, msg);
+        match param {
+            syn::GenericParam::Lifetime(_) => {}
+            syn::GenericParam::Type(_) => return Err(err("type parameters are not allowed on tables")),
+            syn::GenericParam::Const(_) => return Err(err("const parameters are not allowed on tables")),
+        }
+    }
+
     let table_id_from_name_func = quote! {
         fn table_id() -> spacetimedb::TableId {
             static TABLE_ID: std::sync::OnceLock<spacetimedb::TableId> = std::sync::OnceLock::new();
@@ -1069,28 +1087,6 @@ fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::
         })*
     };
 
-    // Attempt to improve the compile error when a table field doesn't satisfy
-    // the supertraits of `TableType`. We make it so the field span indicates
-    // which fields are offenders, and error reporting stops if the field doesn't
-    // implement `SpacetimeType` (which happens to be the derive macro one is
-    // supposed to use). That is, the user doesn't see errors about `Serialize`,
-    // `Deserialize` not being satisfied, which they wouldn't know what to do
-    // about.
-    let assert_fields_are_spacetimetypes = {
-        let trait_ident = Ident::new("AssertSpacetimeFields", Span::call_site());
-        let field_impls = fields
-            .iter()
-            .map(|field| (field.ty, field.span))
-            .collect::<HashMap<_, _>>()
-            .into_iter()
-            .map(|(ty, span)| quote_spanned!(span=> impl #trait_ident for #ty {}));
-
-        quote_spanned! {item.span()=>
-            trait #trait_ident: spacetimedb::SpacetimeType {}
-            #(#field_impls)*
-        }
-    };
-
     let row_type_to_table = quote!(<#row_type as spacetimedb::table::__MapRowTypeToTable>::Table);
 
     // Output all macro data
@@ -1116,7 +1112,7 @@ fn table_impl(mut args: TableArgs, mut item: MutItem<syn::DeriveInput>) -> syn::
 
     let emission = quote! {
         const _: () = {
-            #assert_fields_are_spacetimetypes
+            #(let _ = <#field_types as spacetimedb::rt::TableColumn>::_ITEM;)*
         };
 
         #trait_def

crates/bindings-macro/src/module.rs

@@ -32,7 +32,6 @@ pub(crate) struct SatsField<'a> {
     pub name: Option<String>,
     pub ty: &'a syn::Type,
     pub original_attrs: &'a [syn::Attribute],
-    pub span: Span,
 }
 
 pub(crate) struct SatsVariant<'a> {
@@ -57,7 +56,6 @@ pub(crate) fn sats_type_from_derive(
                 name: field.ident.as_ref().map(syn::Ident::to_string),
                 ty: &field.ty,
                 original_attrs: &field.attrs,
-                span: field.span(),
             });
             SatsTypeData::Product(fields.collect())
         }

crates/bindings/Cargo.toml

@@ -37,3 +37,4 @@ getrandom = { workspace = true, optional = true, features = ["custom"] }
 
 [dev-dependencies]
 insta.workspace = true
+trybuild.workspace = true

crates/bindings/src/rt.rs

@@ -32,6 +32,15 @@ pub fn invoke_reducer<'a, A: Args<'a>>(
     with_timestamp_set(ctx.timestamp, || reducer.invoke(&ctx, args))
 }
 /// A trait for types representing the *execution logic* of a reducer.
+#[diagnostic::on_unimplemented(
+    message = "invalid reducer signature",
+    label = "this reducer signature is not valid",
+    note = "",
+    note = "reducer signatures must match the following pattern:",
+    note = "    `Fn(&ReducerContext, [T1, ...]) [-> Result<(), impl Display>]`",
+    note = "where each `Ti` type implements `SpacetimeType`.",
+    note = ""
+)]
 pub trait Reducer<'de, A: Args<'de>> {
     fn invoke(&self, ctx: &ReducerContext, args: A) -> ReducerResult;
 }
@@ -67,6 +76,10 @@ pub trait Args<'de>: Sized {
 }
 
 /// A trait of types representing the result of executing a reducer.
+#[diagnostic::on_unimplemented(
+    message = "`{Self}` is not a valid reducer return type",
+    note = "reducers cannot return values -- you can only return `()` or `Result<(), impl Display>`"
+)]
 pub trait IntoReducerResult {
     /// Convert the result into form where there is no value
     /// and the error message is a string.
@@ -85,16 +98,72 @@ impl<E: fmt::Display> IntoReducerResult for Result<(), E> {
     }
 }
 
+#[diagnostic::on_unimplemented(
+    message = "the first argument of a reducer must be `&ReducerContext`",
+    note = "all reducers must take `&ReducerContext` as their first argument"
+)]
+pub trait ReducerContextArg {
+    // a little hack used in the macro to make error messages nicer. it generates <T as ReducerContextArg>::_ITEM
+    #[doc(hidden)]
+    const _ITEM: () = ();
+}
+impl ReducerContextArg for &ReducerContext {}
+
 /// A trait of types that can be an argument of a reducer.
-pub trait ReducerArg<'de> {}
-impl<'de, T: Deserialize<'de>> ReducerArg<'de> for T {}
-impl ReducerArg<'_> for &ReducerContext {}
-/// Assert that `T: ReducerArg`.
-pub fn assert_reducer_arg<'de, T: ReducerArg<'de>>() {}
-/// Assert that `T: IntoReducerResult`.
-pub fn assert_reducer_ret<T: IntoReducerResult>() {}
+#[diagnostic::on_unimplemented(
+    message = "the reducer argument `{Self}` does not implement `SpacetimeType`",
+    note = "if you own the type, try adding `#[derive(SpacetimeType)]` to its definition"
+)]
+pub trait ReducerArg {
+    // a little hack used in the macro to make error messages nicer. it generates <T as ReducerArg>::_ITEM
+    #[doc(hidden)]
+    const _ITEM: () = ();
+}
+impl<T: SpacetimeType> ReducerArg for T {}
+
 /// Assert that a reducer type-checks with a given type.
-pub const fn assert_reducer_typecheck<'de, A: Args<'de>>(_: impl Reducer<'de, A> + Copy) {}
+pub const fn scheduled_reducer_typecheck<'de, Row>(_x: impl ReducerForScheduledTable<'de, Row>)
+where
+    Row: SpacetimeType + Serialize + Deserialize<'de>,
+{
+    core::mem::forget(_x);
+}
+
+#[diagnostic::on_unimplemented(
+    message = "invalid signature for scheduled table reducer",
+    note = "the scheduled reducer must take `{TableRow}` as its sole argument",
+    note = "e.g: `fn scheduled_reducer(ctx: &ReducerContext, arg: {TableRow})`"
+)]
+pub trait ReducerForScheduledTable<'de, TableRow> {}
+impl<'de, TableRow: SpacetimeType + Serialize + Deserialize<'de>, R: Reducer<'de, (TableRow,)>>
+    ReducerForScheduledTable<'de, TableRow> for R
+{
+}
+
+// the macro generates <T as SpacetimeType>::make_type::<DummyTypespace>
+pub struct DummyTypespace;
+impl TypespaceBuilder for DummyTypespace {
+    fn add(
+        &mut self,
+        _: std::any::TypeId,
+        _: Option<&'static str>,
+        _: impl FnOnce(&mut Self) -> spacetimedb_lib::AlgebraicType,
+    ) -> spacetimedb_lib::AlgebraicType {
+        unreachable!()
+    }
+}
+
+#[diagnostic::on_unimplemented(
+    message = "the column type `{Self}` does not implement `SpacetimeType`",
+    note = "table column types all must implement `SpacetimeType`",
+    note = "if you own the type, try adding `#[derive(SpacetimeType)]` to its definition"
+)]
+pub trait TableColumn {
+    // a little hack used in the macro to make error messages nicer. it generates <T as TableColumn>::_ITEM
+    #[doc(hidden)]
+    const _ITEM: () = ();
+}
+impl<T: SpacetimeType> TableColumn for T {}
 
 /// Used in the last type parameter of `Reducer` to indicate that the
 /// context argument *should* be passed to the reducer logic.

crates/bindings/tests/ui.rs

@@ -0,0 +1,5 @@
+#[test]
+fn ui() {
+    let t = trybuild::TestCases::new();
+    t.compile_fail("tests/ui/*.rs");
+}

crates/bindings/tests/ui/reducers.rs

@@ -0,0 +1,37 @@
+use spacetimedb::ReducerContext;
+
+struct Test;
+
+#[spacetimedb::reducer]
+fn bad_type(_ctx: &ReducerContext, _a: Test) {}
+
+#[spacetimedb::reducer]
+fn bad_return_type(_ctx: &ReducerContext) -> Test {
+    Test
+}
+
+#[spacetimedb::reducer]
+fn lifetime<'a>(_ctx: &ReducerContext, _a: &'a str) {}
+
+#[spacetimedb::reducer]
+fn type_param<T>() {}
+
+#[spacetimedb::reducer]
+fn const_param<const X: u8>() {}
+
+#[spacetimedb::reducer]
+fn missing_ctx(_a: u8) {}
+
+#[spacetimedb::reducer]
+fn ctx_by_val(_ctx: ReducerContext, _a: u8) {}
+
+#[spacetimedb::table(name = scheduled_table, scheduled(scheduled_table_reducer))]
+struct ScheduledTable {
+    x: u8,
+    y: u8,
+}
+
+#[spacetimedb::reducer]
+fn scheduled_table_reducer(_ctx: &ReducerContext, _x: u8, _y: u8) {}
+
+fn main() {}