Add tool check to ensure that opaque types are always behind a reference

core/src/ast/methods.rs

@@ -1,7 +1,9 @@
+use std::collections::HashMap;
+
 use serde::{Deserialize, Serialize};
 use syn::*;
 
-use super::types::TypeName;
+use super::{CustomType, TypeName};
 
 /// A method declared in the `impl` associated with an FFI struct.
 /// Includes both static and non-static methods, which can be distinguished
@@ -72,6 +74,26 @@ impl Method {
             return_type: return_ty,
         }
     }
+
+    /// Checks that any references to opaque structs in parameters or return values
+    /// are always behind a box or reference.
+    ///
+    /// Any references to opaque structs that are invalid are pushed into the `errors` vector.
+    pub fn check_opaque<'a>(
+        &'a self,
+        env: &HashMap<String, CustomType>,
+        errors: &mut Vec<&'a TypeName>,
+    ) {
+        self.self_param
+            .iter()
+            .for_each(|m| m.ty.check_opaque(env, errors));
+        self.params
+            .iter()
+            .for_each(|m| m.ty.check_opaque(env, errors));
+        self.return_type
+            .iter()
+            .for_each(|t| t.check_opaque(env, errors));
+    }
 }
 
 /// A parameter taken by a [`Method`], including `self`.

core/src/ast/modules.rs

@@ -4,15 +4,29 @@ use std::collections::HashMap;
 use quote::ToTokens;
 use syn::{ImplItem, Item, ItemMod};
 
-use super::methods::Method;
-use super::structs::{OpaqueStruct, Struct};
-use super::types::CustomType;
+use super::{CustomType, Method, OpaqueStruct, Struct, TypeName};
 
 #[derive(Clone, Serialize, Deserialize, Debug)]
 pub struct Module {
     pub declared_types: HashMap<String, CustomType>,
 }
 
+impl Module {
+    /// Checks that any references to opaque structs in parameters or return values
+    /// are always behind a box or reference.
+    ///
+    /// Any references to opaque structs that are invalid are pushed into the `errors` vector.
+    pub fn check_opaque<'a>(
+        &'a self,
+        env: &HashMap<String, CustomType>,
+        errors: &mut Vec<&'a TypeName>,
+    ) {
+        self.declared_types
+            .values()
+            .for_each(|t| t.check_opaque(env, errors));
+    }
+}
+
 impl From<&ItemMod> for Module {
     /// Get all custom types defined in a module as a mapping from their name to
     /// the extracted metadata.
@@ -88,6 +102,37 @@ pub struct File {
     pub modules: HashMap<String, Module>,
 }
 
+impl File {
+    /// Checks that any references to opaque structs in parameters or return values
+    /// are always behind a box or reference.
+    ///
+    /// Any references to opaque structs that are invalid are pushed into the `errors` vector.
+    pub fn check_opaque<'a>(
+        &'a self,
+        env: &HashMap<String, CustomType>,
+        errors: &mut Vec<&'a TypeName>,
+    ) {
+        self.modules
+            .values()
+            .for_each(|t| t.check_opaque(env, errors));
+    }
+
+    /// Fuses all declared types into a single environment `HashMap`.
+    pub fn all_types(&self) -> HashMap<String, CustomType> {
+        let mut out = HashMap::new();
+        self.modules.values().for_each(|m| {
+            m.declared_types.iter().for_each(|(k, v)| {
+                if out.insert(k.clone(), v.clone()).is_some() {
+                    panic!(
+                        "Two types were declared with the same name, this needs to be implemented"
+                    );
+                }
+            })
+        });
+        out
+    }
+}
+
 impl From<&syn::File> for File {
     /// Get all custom types across all modules defined in a given file.
     fn from(file: &syn::File) -> File {
@@ -115,7 +160,7 @@ mod tests {
     use quote::quote;
     use syn;
 
-    use super::Module;
+    use super::{File, Module, TypeName};
 
     #[test]
     fn simple_mod() {
@@ -161,4 +206,74 @@ mod tests {
             ));
         });
     }
+
+    #[test]
+    fn opaque_checks_with_safe_use() {
+        let file_with_safe_opaque = File::from(
+            &syn::parse2(quote! {
+                #[diplomat::bridge]
+                mod ffi {
+                    struct NonOpaqueStruct {}
+
+                    impl NonOpaqueStruct {
+                        fn new(x: i32) -> NonOpaqueStruct {
+                            unimplemented!();
+                        }
+                    }
+
+                    #[diplomat::opaque]
+                    struct OpaqueStruct {}
+
+                    impl OpaqueStruct {
+                        fn new() -> Box<OpaqueStruct> {
+                            unimplemented!();
+                        }
+
+                        fn get_i32(&self) -> i32 {
+                            unimplemented!()
+                        }
+                    }
+                }
+            })
+            .unwrap(),
+        );
+
+        let mut errors = Vec::new();
+        file_with_safe_opaque.check_opaque(&file_with_safe_opaque.all_types(), &mut errors);
+        assert_eq!(errors.len(), 0);
+    }
+
+    #[test]
+    fn opaque_checks_with_error() {
+        let file_with_error_opaque = File::from(
+            &syn::parse2(quote! {
+                #[diplomat::bridge]
+                mod ffi {
+                    #[diplomat::opaque]
+                    struct OpaqueStruct {}
+
+                    impl OpaqueStruct {
+                        fn new() -> OpaqueStruct {
+                            unimplemented!();
+                        }
+
+                        fn get_i32(self) -> i32 {
+                            unimplemented!()
+                        }
+                    }
+                }
+            })
+            .unwrap(),
+        );
+
+        let mut errors = Vec::new();
+        file_with_error_opaque.check_opaque(&file_with_error_opaque.all_types(), &mut errors);
+        assert_eq!(
+            errors,
+            vec![
+                &TypeName::Named("OpaqueStruct".to_string()),
+                &TypeName::Named("OpaqueStruct".to_string())
+            ]
+        );
+    }
 }

core/src/ast/structs.rs

@@ -1,7 +1,7 @@
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 
-use super::{methods::Method, types::TypeName};
+use super::{Method, TypeName};
 
 /// A struct declaration in an FFI module that is not opaque.
 #[derive(Clone, Serialize, Deserialize, Debug)]

core/src/ast/types.rs

@@ -7,10 +7,7 @@ use lazy_static::lazy_static;
 use std::collections::HashMap;
 use std::iter::FromIterator;
 
-use super::{
-    methods::Method,
-    structs::{OpaqueStruct, Struct},
-};
+use super::{Method, OpaqueStruct, Struct};
 
 /// A type declared inside a Diplomat-annotated module.
 #[derive(Clone, Serialize, Deserialize, Debug)]
@@ -38,12 +35,35 @@ impl CustomType {
             CustomType::Opaque(strct) => &strct.methods,
         }
     }
+
+    /// Checks that any references to opaque structs in parameters or return values
+    /// are always behind a box or reference.
+    ///
+    /// Any references to opaque structs that are invalid are pushed into the `errors` vector.
+    pub fn check_opaque<'a>(
+        &'a self,
+        env: &HashMap<String, CustomType>,
+        errors: &mut Vec<&'a TypeName>,
+    ) {
+        match self {
+            CustomType::Struct(strct) => {
+                for (_, field) in strct.fields.iter() {
+                    field.check_opaque(env, errors);
+                }
+            }
+            CustomType::Opaque(_) => {}
+        }
+
+        for method in self.methods().iter() {
+            method.check_opaque(env, errors);
+        }
+    }
 }
 
 /// A local type reference, such as the type of a field, parameter, or return value.
 /// Unlike [`CustomType`], which represents a type declaration, [`TypeName`]s can compose
 /// types through references and boxing, and can also capture unresolved paths.
-#[derive(Clone, Serialize, Deserialize, Debug)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)]
 pub enum TypeName {
     /// A built-in Rust scalar primitive.
     Primitive(PrimitiveType),
@@ -102,6 +122,40 @@ impl TypeName {
             _ => panic!(),
         }
     }
+
+    fn check_opaque_internal<'a>(
+        &'a self,
+        env: &HashMap<String, CustomType>,
+        behind_reference: bool,
+        errors: &mut Vec<&'a TypeName>,
+    ) {
+        match self {
+            TypeName::Reference(underlying, _) => {
+                underlying.check_opaque_internal(env, true, errors)
+            }
+            TypeName::Box(underlying) => underlying.check_opaque_internal(env, true, errors),
+            TypeName::Primitive(_) => {}
+            TypeName::Named(_) => {
+                if let CustomType::Opaque(_) = self.resolve(env) {
+                    if !behind_reference {
+                        errors.push(self)
+                    }
+                }
+            }
+        }
+    }
+
+    /// Checks that any references to opaque structs in parameters or return values
+    /// are always behind a box or reference.
+    ///
+    /// Any references to opaque structs that are invalid are pushed into the `errors` vector.
+    pub fn check_opaque<'a>(
+        &'a self,
+        env: &HashMap<String, CustomType>,
+        errors: &mut Vec<&'a TypeName>,
+    ) {
+        self.check_opaque_internal(env, false, errors);
+    }
 }
 
 impl From<&syn::Type> for TypeName {

tool/src/main.rs

@@ -39,7 +39,18 @@ fn gen_js(strcts: &HashMap<String, ast::CustomType>) {
 fn main() {
     let lib_file = syn_inline_mod::parse_and_inline_modules(Path::new("./src/main.rs"));
     let custom_types = ast::File::from(&lib_file);
-    dbg!(&custom_types);
+    let env = custom_types.all_types();
+    let mut opaque_errors = vec![];
+    custom_types.check_opaque(&env, &mut opaque_errors);
+    if !opaque_errors.is_empty() {
+        opaque_errors.iter().for_each(|e| {
+            println!(
+                "An opaque type crossed the FFI boundary as a value: {:?}",
+                e
+            )
+        });
+        panic!();
+    }
 
     custom_types.modules.iter().for_each(|(mod_name, module)| {
         println!("// {}", mod_name);