Make name a required parameter of view macro (#3480)

# Description of Changes

<!-- Please describe your change, mention any related tickets, and so on
here. -->

Makes `name` a required parameter of the `#[view]` macro. This is to be
consistent with the `#[table]` macro since views should expose the same
interface as tables.

# API and ABI breaking changes

<!-- If this is an API or ABI breaking change, please apply the
corresponding GitHub label. -->

None

# Expected complexity level and risk

<!--
How complicated do you think these changes are? Grade on a scale from 1
to 5,
where 1 is a trivial change, and 5 is a deep-reaching and complex
change.

This complexity rating applies not only to the complexity apparent in
the diff,
but also to its interactions with existing and future code.

If you answered more than a 2, explain what is complex about the PR,
and what other components it interacts with in potentially concerning
ways. -->

1

# Testing

<!-- Describe any testing you've done, and any testing you'd like your
reviewers to do,
so that you're confident that all the changes work as expected! -->

- [x] Added new compile tests
- [x] Updated existing compile tests

Author: joshua-spacetime

crates/bindings-macro/src/lib.rs

@@ -125,7 +125,7 @@ pub fn reducer(args: StdTokenStream, item: StdTokenStream) -> StdTokenStream {
 #[proc_macro_attribute]
 pub fn view(args: StdTokenStream, item: StdTokenStream) -> StdTokenStream {
     cvt_attr::<ItemFn>(args, item, quote!(), |args, original_function| {
-        let args = view::ViewArgs::parse(args)?;
+        let args = view::ViewArgs::parse(args, &original_function.sig.ident)?;
         view::view_impl(args, original_function)
     })
 }

crates/bindings-macro/src/view.rs

@@ -1,55 +1,56 @@
-use proc_macro2::{Span, TokenStream};
+use heck::ToSnakeCase;
+use proc_macro2::{Ident, Span, TokenStream};
 use quote::quote;
+use syn::ext::IdentExt;
 use syn::parse::Parser;
 use syn::{FnArg, ItemFn};
 
 use crate::sym;
-use crate::util::{ident_to_litstr, match_meta};
+use crate::util::{check_duplicate_msg, match_meta};
 
 pub(crate) struct ViewArgs {
+    name: Ident,
     #[allow(unused)]
     public: bool,
 }
 
 impl ViewArgs {
-    /// Parse `#[view(public)]` where `public` is required.
-    pub(crate) fn parse(input: TokenStream) -> syn::Result<Self> {
-        if input.is_empty() {
-            return Err(syn::Error::new(
-                Span::call_site(),
-                "views must be declared as `#[view(public)]`; `public` is required",
-            ));
-        }
-        let mut public = false;
+    /// Parse `#[view(name = ..., public)]` where both `name` and `public` are required.
+    pub(crate) fn parse(input: TokenStream, func_ident: &Ident) -> syn::Result<Self> {
+        let mut name = None;
+        let mut public = None;
         syn::meta::parser(|meta| {
             match_meta!(match meta {
+                sym::name => {
+                    check_duplicate_msg(&name, &meta, "`name` already specified")?;
+                    name = Some(meta.value()?.parse()?);
+                }
                 sym::public => {
-                    if public {
-                        return Err(syn::Error::new(
-                            Span::call_site(),
-                            "duplicate attribute argument: `public`",
-                        ));
-                    }
-                    public = true;
+                    check_duplicate_msg(&public, &meta, "`public` already specified")?;
+                    public = Some(());
                 }
             });
             Ok(())
         })
         .parse2(input)?;
-        if !public {
-            return Err(syn::Error::new(
+        let name = name.ok_or_else(|| {
+            let view = func_ident.to_string().to_snake_case();
+            syn::Error::new(
                 Span::call_site(),
-                "views must be declared as `#[view(public)]`; `public` is required",
-            ));
-        }
-        Ok(Self { public })
+                format_args!("must specify view name, e.g. `#[spacetimedb::view(name = {view})]"),
+            )
+        })?;
+        let () = public
+            .ok_or_else(|| syn::Error::new(Span::call_site(), "views must be `public`, e.g. `#[view(public)]`"))?;
+        Ok(Self { name, public: true })
     }
 }
 
-pub(crate) fn view_impl(_args: ViewArgs, original_function: &ItemFn) -> syn::Result<TokenStream> {
-    let func_name = &original_function.sig.ident;
-    let view_name = ident_to_litstr(func_name);
+pub(crate) fn view_impl(args: ViewArgs, original_function: &ItemFn) -> syn::Result<TokenStream> {
     let vis = &original_function.vis;
+    let func_name = &original_function.sig.ident;
+    let view_ident = args.name;
+    let view_name = view_ident.unraw().to_string();
 
     for param in &original_function.sig.generics.params {
         let err = |msg| syn::Error::new_spanned(param, msg);
@@ -116,7 +117,7 @@ pub(crate) fn view_impl(_args: ViewArgs, original_function: &ItemFn) -> syn::Res
         }
     };
 
-    let register_describer_symbol = format!("__preinit__20_register_describer_{}", view_name.value());
+    let register_describer_symbol = format!("__preinit__20_register_describer_{}", view_name);
 
     let lt_params = &original_function.sig.generics;
     let lt_where_clause = &lt_params.where_clause;

crates/bindings/src/lib.rs

@@ -820,25 +820,25 @@ pub use spacetimedb_bindings_macro::procedure;
 /// }
 ///
 /// // A view that selects at most one row from a table
-/// #[view(public)]
+/// #[view(name = my_player, public)]
 /// fn my_player(ctx: &ViewContext) -> Option<Player> {
 ///     ctx.db.player().identity().find(ctx.sender)
 /// }
 ///
 /// // An example of column projection
-/// #[view(public)]
+/// #[view(name = my_player_id, public)]
 /// fn my_player_id(ctx: &ViewContext) -> Option<PlayerId> {
 ///     ctx.db.player().identity().find(ctx.sender).map(|Player { id, .. }| PlayerId { id })
 /// }
 ///
 /// // An example of a parameterized view
-/// #[view(public)]
+/// #[view(name = players_at_level, public)]
 /// fn players_at_level(ctx: &AnonymousViewContext, level: u32) -> Vec<Player> {
 ///     ctx.db.player().level().filter(level).collect()
 /// }
 ///
 /// // An example that is analogous to a semijoin in sql
-/// #[view(public)]
+/// #[view(name = players_at_coordinates, public)]
 /// fn players_at_coordinates(ctx: &AnonymousViewContext, x: u64, y: u64) -> Vec<Player> {
 ///     ctx
 ///         .db
@@ -850,7 +850,7 @@ pub use spacetimedb_bindings_macro::procedure;
 /// }
 ///
 /// // An example of a join that combines fields from two different tables
-/// #[view(public)]
+/// #[view(name = players_with_coordinates, public)]
 /// fn players_with_coordinates(ctx: &AnonymousViewContext, x: u64, y: u64) -> Vec<PlayerAndLocation> {
 ///     ctx
 ///         .db

crates/bindings/tests/ui/views.rs

@@ -72,62 +72,68 @@ struct Player {
 
 struct NotSpacetimeType {}
 
-/// Private views not allowed; must be `#[view(public)]`
-#[view]
+/// Private views not allowed; must be `#[view(public, ...)]`
+#[view(name = view_def_no_public)]
 fn view_def_no_public(_: &ViewContext) -> Vec<Player> {
     vec![]
 }
 
 /// Duplicate `public`
-#[view(public, public)]
-fn view_def_duplicate_attribute_arg() -> Vec<Player> {
+#[view(name = view_def_dup_public, public, public)]
+fn view_def_dup_public() -> Vec<Player> {
+    vec![]
+}
+
+/// Duplicate `name`
+#[view(name = view_def_dup_name, name = view_def_dup_name, public)]
+fn view_def_dup_name() -> Vec<Player> {
     vec![]
 }
 
 /// Unsupported attribute arg
-#[view(public, anonymous)]
-fn view_def_unsupported_attribute_arg() -> Vec<Player> {
+#[view(name = view_def_unsupported_arg, public, anonymous)]
+fn view_def_unsupported_arg() -> Vec<Player> {
     vec![]
 }
 
 /// A `ViewContext` is required
-#[view(public)]
+#[view(name = view_def_no_context, public)]
 fn view_def_no_context() -> Vec<Player> {
     vec![]
 }
 
 /// A `ViewContext` is required
-#[view(public)]
+#[view(name = view_def_wrong_context, public)]
 fn view_def_wrong_context(_: &ReducerContext) -> Vec<Player> {
     vec![]
 }
 
 /// Must pass the `ViewContext` by ref
-#[view(public)]
+#[view(name = view_def_pass_context_by_value, public)]
 fn view_def_pass_context_by_value(_: ViewContext) -> Vec<Player> {
     vec![]
 }
 
 /// The view context must be the first parameter
-#[view(public)]
+#[view(name = view_def_wrong_context_position, public)]
 fn view_def_wrong_context_position(_: &u32, _: &ViewContext) -> Vec<Player> {
     vec![]
 }
 
 /// Must return `Vec<T>` or `Option<T>` where `T` is a SpacetimeType
-#[view(public)]
+#[view(name = view_def_no_return, public)]
 fn view_def_no_return(_: &ViewContext) {}
 
 /// Must return `Vec<T>` or `Option<T>` where `T` is a SpacetimeType
-#[view(public)]
+#[view(name = view_def_wrong_return, public)]
 fn view_def_wrong_return(_: &ViewContext) -> Player {
     Player {
         identity: Identity::ZERO,
     }
 }
 
 /// Must return `Vec<T>` or `Option<T>` where `T` is a SpacetimeType
-#[view(public)]
+#[view(name = view_def_returns_not_a_spacetime_type, public)]
 fn view_def_returns_not_a_spacetime_type(_: &AnonymousViewContext) -> Option<NotSpacetimeType> {
     None
 }
@@ -144,7 +150,7 @@ struct ScheduledTable {
 }
 
 /// Cannot use a view as a scheduled function
-#[view(public)]
+#[view(name = scheduled_table_view, public)]
 fn scheduled_table_view(_: &ViewContext, _args: ScheduledTable) -> Vec<Player> {
     vec![]
 }