Final fixes, address review comments

crates/sats/src/proptest.rs

@@ -60,7 +60,11 @@ fn generate_algebraic_type_from_leaves(
                 .prop_map(|vec| vec
                     .into_iter()
                     .enumerate()
-                    .map(|(i, ty)| (format!("element_{i}").into()), ty))
+                    .map(|(i, ty)| ProductTypeElement {
+                        // Generate names because the validation code in the `schema` crate requires them.
+                        name: Some(format!("field_{i}").into()),
+                        algebraic_type: ty
+                    })
                     .collect())
                 .prop_map(Vec::into_boxed_slice)
                 .prop_map(AlgebraicType::product),

crates/schema/src/def.rs

@@ -20,7 +20,7 @@ use std::hash::Hash;
 
 use crate::error::{IdentifierError, ValidationErrors};
 use crate::identifier::Identifier;
-use crate::type_for_generate::{AlgebraicTypeUse, TypespaceForGenerate};
+use crate::type_for_generate::{AlgebraicTypeUse, ProductTypeDef, TypespaceForGenerate};
 use hashbrown::Equivalent;
 use itertools::Itertools;
 use spacetimedb_data_structures::error_stream::{CollectAllErrors, CombineErrors, ErrorStream};
@@ -103,17 +103,6 @@ pub struct ModuleDef {
 }
 
 impl ModuleDef {
-    /// Construct a `ModuleDef` by validating a `RawModuleDef`.
-    /// This is the only way to construct a `ModuleDef`.
-    /// (The `TryFrom` impls for this type just call this method.)
-    pub fn validate(raw: RawModuleDef) -> Result<Self, ValidationErrors> {
-        match raw {
-            RawModuleDef::V8BackCompat(v8_mod) => validate::v8::validate(v8_mod),
-            RawModuleDef::V9(v9_mod) => validate::v9::validate(v9_mod),
-            _ => unimplemented!(),
-        }
-    }
-
     /// The tables of the module definition.
     pub fn tables(&self) -> impl Iterator<Item = &TableDef> {
         self.tables.values()
@@ -203,15 +192,13 @@ impl ModuleDef {
                     continue;
                 }
 
-                let column_names = columns
-                    .iter()
-                    .map(|col_id| &*table.get_column(col_id).expect("validated unique constraint").name)
-                    .join("_");
-
-                // TODO(1.0): ensure generated index names are identical when upgrading the Rust module bindings.
-                let mut index_name =
-                    Identifier::new(format!("idx_{}_{}_{}_unique", table.name, column_names, constraint.name).into())
-                        .expect("validated identifier parts");
+                // This replicates the logic from `RawIndexDefV8::for_column`.
+                let constraint_name = &constraint
+                    .name
+                    .trim_start_matches(&format!("ct_{}_", table.name))
+                    .trim_end_matches("_unique");
+                let mut index_name = Identifier::new(format!("idx_{}_{}_unique", table.name, constraint_name).into())
+                    .expect("validated identifier parts");
 
                 // incredibly janky loop to avoid name collisions.
                 // hey, somebody could be being malicious.
@@ -805,7 +792,9 @@ pub struct ReducerDef {
     pub params: ProductType,
 
     /// The parameters of the reducer, formatted for client codegen.
-    pub params_for_generate: crate::type_for_generate::ProductTypeDef,
+    ///
+    /// This `ProductType` need not be registered in the module's `TypespaceForGenerate`.
+    pub params_for_generate: ProductTypeDef,
 
     /// The special role of this reducer in the module lifecycle, if any.
     pub lifecycle: Option<Lifecycle>,

crates/schema/src/def/validate/v8.rs

@@ -690,48 +690,32 @@ mod tests {
     }
 
     #[test]
-    fn recursive_type_ref() {
+    fn recursive_ref() {
         let recursive_type = AlgebraicType::product([("a", AlgebraicTypeRef(0).into())]);
 
         let mut builder = RawModuleDefV8Builder::default();
         let ref_ = builder.add_type_for_tests("Recursive", recursive_type.clone());
         builder.add_reducer_for_tests("silly", ProductType::from([("a", ref_.into())]));
-        let result: Result<ModuleDef> = builder.finish().try_into();
+        let result: ModuleDef = builder.finish().try_into().unwrap();
 
-        expect_error_matching!(result, ValidationError::ClientCodegenError { location, error: ClientCodegenError::TypeRefError(_)  } => {
-            location == &TypeLocation::ReducerArg {
-                reducer_name: "silly".into(),
-                position: 0,
-                arg_name: Some("a".into())
-            }
-        });
-        expect_error_matching!(result, ValidationError::ClientCodegenError { location, error: ClientCodegenError::TypeRefError(_)  } => {
-            location == &TypeLocation::InTypespace { ref_: AlgebraicTypeRef(0) }
-        });
+        assert!(result.typespace_for_generate[ref_].is_recursive());
     }
 
     #[test]
-    fn invalid_type_ref() {
+    fn out_of_bounds_ref() {
         let invalid_type_1 = AlgebraicType::product([("a", AlgebraicTypeRef(31).into())]);
         let mut builder = RawModuleDefV8Builder::default();
         let ref_ = builder.add_type_for_tests("Invalid", invalid_type_1.clone());
         builder.add_reducer_for_tests("silly", ProductType::from([("a", ref_.into())]));
         let result: Result<ModuleDef> = builder.finish().try_into();
 
-        expect_error_matching!(result, ValidationError::ClientCodegenError { location, error: ClientCodegenError::TypeRefError(_)  } => {
-            location == &TypeLocation::ReducerArg {
-                reducer_name: "silly".into(),
-                position: 0,
-                arg_name: Some("a".into())
-            }
-        });
         expect_error_matching!(result, ValidationError::ClientCodegenError { location, error: ClientCodegenError::TypeRefError(_)  } => {
             location == &TypeLocation::InTypespace { ref_: AlgebraicTypeRef(0) }
         });
     }
 
     #[test]
-    fn type_invalid() {
+    fn invalid_use() {
         let inner_type_invalid_for_use = AlgebraicType::product([("b", AlgebraicType::U32)]);
         let invalid_type = AlgebraicType::product([("a", inner_type_invalid_for_use.clone())]);
         let mut builder = RawModuleDefV8Builder::default();
@@ -749,17 +733,6 @@ mod tests {
                 ty.0 == inner_type_invalid_for_use
             }
         );
-        expect_error_matching!(
-            result,
-            ValidationError::ClientCodegenError {
-                location,
-                error: ClientCodegenError::NonSpecialTypeNotAUse { .. }
-            } => location == &TypeLocation::ReducerArg {
-                reducer_name: "silly".into(),
-                position: 0,
-                arg_name: Some("a".into())
-            }
-        );
     }
 
     #[test]

crates/schema/src/def/validate/v9.rs

@@ -1,6 +1,6 @@
 use crate::def::*;
 use crate::error::{RawColumnName, ValidationError};
-use crate::type_for_generate::TypespaceForGenerateBuilder;
+use crate::type_for_generate::{ClientCodegenError, ProductTypeDef, TypespaceForGenerateBuilder};
 use crate::{def::validate::Result, error::TypeLocation};
 use spacetimedb_data_structures::error_stream::{CollectAllErrors, CombineErrors};
 use spacetimedb_data_structures::map::HashSet;
@@ -227,8 +227,6 @@ impl ModuleValidator<'_> {
 
     /// Validate a reducer definition.
     fn validate_reducer_def(&mut self, reducer_def: RawReducerDefV9) -> Result<ReducerDef> {
-        use crate::type_for_generate::ClientCodegenError;
-
         let RawReducerDefV9 {
             name,
             params,
@@ -277,8 +275,9 @@ impl ModuleValidator<'_> {
         Ok(ReducerDef {
             name,
             params: params.clone(),
-            params_for_generate: crate::type_for_generate::ProductTypeDef {
+            params_for_generate: ProductTypeDef {
                 elements: params_for_generate,
+                recursive: false, // A ProductTypeDef not stored in a Typespace cannot be recursive.
             },
             lifecycle,
         })
@@ -1200,41 +1199,25 @@ mod tests {
     }
 
     #[test]
-    fn recursive_type_ref() {
+    fn recursive_ref() {
         let recursive_type = AlgebraicType::product([("a", AlgebraicTypeRef(0).into())]);
 
         let mut builder = RawModuleDefV9Builder::new();
         let ref_ = builder.add_algebraic_type([], "Recursive", recursive_type.clone(), false);
         builder.add_reducer("silly", ProductType::from([("a", ref_.into())]), None);
-        let result: Result<ModuleDef> = builder.finish().try_into();
+        let result: ModuleDef = builder.finish().try_into().unwrap();
 
-        expect_error_matching!(result, ValidationError::ClientCodegenError { location, error: ClientCodegenError::TypeRefError(_)  } => {
-            location == &TypeLocation::ReducerArg {
-                reducer_name: "silly".into(),
-                position: 0,
-                arg_name: Some("a".into())
-            }
-        });
-        expect_error_matching!(result, ValidationError::ClientCodegenError { location, error: ClientCodegenError::TypeRefError(_)  } => {
-            location == &TypeLocation::InTypespace { ref_: AlgebraicTypeRef(0) }
-        });
+        assert!(result.typespace_for_generate[ref_].is_recursive());
     }
 
     #[test]
-    fn invalid_type_ref() {
+    fn out_of_bounds_ref() {
         let invalid_type_1 = AlgebraicType::product([("a", AlgebraicTypeRef(31).into())]);
         let mut builder = RawModuleDefV9Builder::new();
         let ref_ = builder.add_algebraic_type([], "Invalid", invalid_type_1.clone(), false);
         builder.add_reducer("silly", ProductType::from([("a", ref_.into())]), None);
         let result: Result<ModuleDef> = builder.finish().try_into();
 
-        expect_error_matching!(result, ValidationError::ClientCodegenError { location, error: ClientCodegenError::TypeRefError(_)  } => {
-            location == &TypeLocation::ReducerArg {
-                reducer_name: "silly".into(),
-                position: 0,
-                arg_name: Some("a".into())
-            }
-        });
         expect_error_matching!(result, ValidationError::ClientCodegenError { location, error: ClientCodegenError::TypeRefError(_)  } => {
             location == &TypeLocation::InTypespace { ref_: AlgebraicTypeRef(0) }
         });

crates/schema/src/type_for_generate.rs

@@ -118,6 +118,11 @@ impl TypespaceForGenerate {
     pub fn defs(&self) -> &HashMap<AlgebraicTypeRef, AlgebraicTypeDef> {
         &self.defs
     }
+
+    /// Get a definition in the typespace.
+    pub fn get(&self, ref_: AlgebraicTypeRef) -> Option<&AlgebraicTypeDef> {
+        self.defs.get(&ref_)
+    }
 }
 
 impl Index<AlgebraicTypeRef> for TypespaceForGenerate {
@@ -135,14 +140,6 @@ impl Index<&'_ AlgebraicTypeRef> for TypespaceForGenerate {
     }
 }
 
-impl std::ops::Index<AlgebraicTypeRef> for TypespaceForGenerate {
-    type Output = AlgebraicTypeDef;
-
-    fn index(&self, index: AlgebraicTypeRef) -> &Self::Output {
-        &self.defs[&index]
-    }
-}
-
 /// An algebraic type definition.
 #[derive(Debug, Clone, EnumAsInner)]
 pub enum AlgebraicTypeDef {