Skip to content

Commit

Permalink
Improve diagnostics and add ui tests
Browse files Browse the repository at this point in the history
  • Loading branch information
coolreader18 committed May 13, 2024
1 parent 61613ca commit f0f37c6
Show file tree
Hide file tree
Showing 12 changed files with 716 additions and 66 deletions.
25 changes: 25 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,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"
url = "2.3.1"
urlencoding = "2.1.2"
Expand Down
132 changes: 79 additions & 53 deletions crates/bindings-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use module::{derive_deserialize, derive_satstype, derive_serialize};
use proc_macro2::{Span, TokenStream};
use quote::{format_ident, quote, quote_spanned, TokenStreamExt};
use spacetimedb_primitives::ColumnAttribute;
use std::collections::HashMap;
use std::time::Duration;
use syn::parse::{Parse, ParseStream};
use syn::spanned::Spanned;
Expand Down Expand Up @@ -294,6 +293,24 @@ fn gen_reducer(original_function: ItemFn, reducer_name: &str, extra: ReducerExtr
let func_name = &original_function.sig.ident;
let vis = &original_function.vis;

for param in &original_function.sig.generics.params {
match param {
syn::GenericParam::Lifetime(_) => {}
syn::GenericParam::Type(_) => {
return Err(syn::Error::new_spanned(
param,
"type parameters are not allowed on reducers",
))
}
syn::GenericParam::Const(_) => {
return Err(syn::Error::new_spanned(
param,
"const parameters are not allowed on reducers",
))
}
}
}

// let errmsg = "reducer should have at least 2 arguments: (identity: Identity, timestamp: u64, ...)";
// let ([arg1, arg2], args) = validate_reducer_args(&original_function.sig, errmsg)?;

Expand Down Expand Up @@ -348,6 +365,9 @@ fn gen_reducer(original_function: ItemFn, reducer_name: &str, extra: ReducerExtr

let mut extra_impls = TokenStream::new();

let lt_params = &original_function.sig.generics;
let lt_where_clause = &lt_params.where_clause;

if !matches!(extra, ReducerExtra::None) {
let arg_names = typed_args
.iter()
Expand All @@ -359,31 +379,12 @@ fn gen_reducer(original_function: ItemFn, reducer_name: &str, extra: ReducerExtr
.collect::<Vec<_>>();

extra_impls.extend(quote!(impl #func_name {
pub fn schedule(__time: spacetimedb::Timestamp #(, #arg_names: #arg_tys)*) -> spacetimedb::ScheduleToken<#func_name> {
spacetimedb::rt::schedule(__time, (#(#arg_names,)*))
pub fn schedule #lt_params (__time: spacetimedb::Timestamp #(, #arg_names: #arg_tys)*) -> spacetimedb::ScheduleToken<#func_name> #lt_where_clause {
spacetimedb::rt::schedule(__time, #func_name, (#(#arg_names,)*))
}
}));
}

let generated_function = quote! {
fn __reducer(
__sender: spacetimedb::sys::Buffer,
__caller_address: spacetimedb::sys::Buffer,
__timestamp: u64,
__args: &[u8]
) -> spacetimedb::sys::Buffer {
#(spacetimedb::rt::assert_reducer_arg::<#arg_tys>();)*
#(spacetimedb::rt::assert_reducer_ret::<#ret_ty>();)*
spacetimedb::rt::invoke_reducer(
#func_name,
__sender,
__caller_address,
__timestamp,
__args,
)
}
};

let generated_describe_function = quote! {
#[export_name = #register_describer_symbol]
pub extern "C" fn __register_describer() {
Expand All @@ -397,13 +398,33 @@ fn gen_reducer(original_function: ItemFn, reducer_name: &str, extra: ReducerExtr
};
#[allow(non_camel_case_types)]
#vis struct #func_name { _never: ::core::convert::Infallible }
const _: () = {
fn _assert_args #lt_params () #lt_where_clause {
#(let _ = <#arg_tys as spacetimedb::rt::ReducerArg>::_ITEM;)*
#(let _ = <#ret_ty as spacetimedb::rt::ReducerResult>::into_result;)*
}
};
impl #func_name {
fn invoke(
__sender: spacetimedb::sys::Buffer,
__caller_address: spacetimedb::sys::Buffer,
__timestamp: u64,
__args: &[u8]
) -> spacetimedb::sys::Buffer {
spacetimedb::rt::invoke_reducer(
#func_name,
__sender,
__caller_address,
__timestamp,
__args,
)
}
}
#[automatically_derived]
impl spacetimedb::rt::ReducerInfo for #func_name {
const NAME: &'static str = #reducer_name;
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;
}
#extra_impls
#original_function
Expand Down Expand Up @@ -492,6 +513,24 @@ fn spacetimedb_tabletype_impl(item: syn::DeriveInput) -> syn::Result<TokenStream
return Err(syn::Error::new(Span::call_site(), "spacetimedb table must be a struct"));
};

for param in &item.generics.params {
match param {
syn::GenericParam::Lifetime(_) => {}
syn::GenericParam::Type(_) => {
return Err(syn::Error::new_spanned(
param,
"type parameters are not allowed on tables",
))
}
syn::GenericParam::Const(_) => {
return Err(syn::Error::new_spanned(
param,
"const parameters are not allowed on tables",
))
}
}
}

let mut columns = Vec::<Column>::new();

let get_table_id_func = quote! {
Expand Down Expand Up @@ -737,39 +776,24 @@ fn spacetimedb_tabletype_impl(item: syn::DeriveInput) -> syn::Result<TokenStream
};
};

// 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)*
}
};

// Output all macro data
let emission = quote! {
const _: () = {
#describe_table_func
};

const _: () = {
#assert_fields_are_spacetimetypes
};

impl #original_struct_ident {
// 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.
const ___ASSERT_SPACETIMETYPE: () = {
#(spacetimedb::rt::assert_spacetimetype::<#field_types>();)*
};

#db_insert
#(#unique_filter_funcs)*
#(#unique_update_funcs)*
Expand Down Expand Up @@ -811,7 +835,9 @@ fn spacetimedb_index(
let output = quote! {
#original_struct

const _: () = spacetimedb::rt::assert_table::<#original_struct_name>();
const _: () = {
let _ = <#original_struct_name as spacetimedb::TableType>::TABLE_NAME;
};
};

if std::env::var("PROC_MACRO_DEBUG").is_ok() {
Expand Down
4 changes: 1 addition & 3 deletions crates/bindings-macro/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,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> {
Expand All @@ -53,7 +52,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())
}
Expand Down Expand Up @@ -198,7 +196,7 @@ pub(crate) fn derive_deserialize(ty: &SatsType<'_>) -> TokenStream {
let spacetimedb_lib = &ty.krate;
let (impl_generics, ty_generics, where_clause) = ty.generics.split_for_impl();

let mut de_generics = ty.generics.clone();
let mut de_generics = ty.generics.clone();
let de_lifetime = syn::Lifetime::new("'de", Span::call_site());
for lp in de_generics.lifetimes_mut() {
lp.bounds.push(de_lifetime.clone());
Expand Down
1 change: 1 addition & 0 deletions crates/bindings/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ scoped-tls.workspace = true
[dev-dependencies]
rand.workspace = true
bytes.workspace = true
trybuild.workspace = true
34 changes: 24 additions & 10 deletions crates/bindings/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ fn cvt_result(res: Result<(), Box<str>>) -> Buffer {
/// A trait for types representing the *execution logic* of a reducer.
///
/// The type parameter `T` is used for determining whether there is a context argument.
#[diagnostic::on_unimplemented(
message = "invalid reducer signature",
label = "this reducer signature is not valid; parameters and return type must implement `SpacetimeType`",
note = "",
note = "reducer signatures must match the following pattern:",
note = " Fn([ReducerContext,] [T where T: SpacetimeType, ...]) [-> Result<(), impl Display>]",
note = ""
)]
pub trait Reducer<'de, A: Args<'de>, T> {
fn invoke(&self, ctx: ReducerContext, args: A) -> Result<(), Box<str>>;
}
Expand Down Expand Up @@ -179,15 +187,17 @@ impl<E: fmt::Debug> ReducerResult for Result<(), E> {
}

/// 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: ReducerResult`.
pub fn assert_reducer_ret<T: ReducerResult>() {}
/// Assert that `T: TableType`.
pub const fn assert_table<T: TableType>() {}
#[diagnostic::on_unimplemented(message = "the reducer argument `{Self}` does not implement `SpacetimeType`")]
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 {}
impl ReducerArg for ReducerContext {}

// the macro generates <T as SpacetimeType>::make_type::<DummyTypespace>
pub const fn assert_spacetimetype<T: SpacetimeType>() {}

/// Used in the last type parameter of `Reducer` to indicate that the
/// context argument *should* be passed to the reducer logic.
Expand Down Expand Up @@ -339,7 +349,11 @@ pub fn schedule_in(duration: Duration) -> Timestamp {
/// Schedule reducer `R` to be executed async at `time`stamp with arguments `args`.
///
/// Returns a token for the schedule that can be used to cancel the schedule.
pub fn schedule<'de, R: ReducerInfo>(time: Timestamp, args: impl ScheduleArgs<'de>) -> ScheduleToken<R> {
pub fn schedule<'de, R: ReducerInfo, A: Args<'de>, T>(
time: Timestamp,
_: impl Reducer<'de, A, T>,
args: impl ScheduleArgs<'de, Args = A>,
) -> ScheduleToken<R> {
// bsatn serialize the arguments into a vector.
let arg_bytes = bsatn::to_vec(&SerDeArgs(args.into_args())).unwrap();

Expand Down
5 changes: 5 additions & 0 deletions crates/bindings/tests/ui.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[test]
fn ui() {
let t = trybuild::TestCases::new();
t.compile_fail("tests/ui/*.rs");
}
30 changes: 30 additions & 0 deletions crates/bindings/tests/ui/reducers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use spacetimedb::*;

struct Test;

#[spacetimedb(reducer)]
fn bad_type(_a: Test) {}

#[spacetimedb(reducer)]
fn bad_return_type() -> Test {
Test
}

#[spacetimedb(reducer)]
fn bad_type_ctx(_ctx: ReducerContext, _a: Test) {}

#[spacetimedb(reducer)]
fn bad_return_type_ctx(_ctx: ReducerContext) -> Test {
Test
}

#[spacetimedb(reducer)]
fn lifetime<'a>(_a: &'a str) {}

#[spacetimedb(reducer)]
fn type_param<T>() {}

#[spacetimedb(reducer)]
fn const_param<const X: u8>() {}

fn main() {}
Loading

0 comments on commit f0f37c6

Please sign in to comment.