refactor ClassEntity.implements (#189)

Instead of taking a function to delay fetching ClassEntry, accept an Interface, and hide the late-fetching

Author: brettmc

phper-doc/doc/_06_module/_06_register_class/index.md

@@ -41,14 +41,14 @@ class Foo {}
 If you want the class `Foo` extends `Bar`, and implements interface `Stringable`:
 
 ```rust,no_run
-use phper::classes::{ClassEntity, ClassEntry};
+use phper::classes::{ClassEntity, ClassEntry, Interface};
 
 let mut foo = ClassEntity::new("Foo");
 foo.extends(|| ClassEntry::from_globals("Bar").unwrap());
-foo.implements(|| ClassEntry::from_globals("Stringable").unwrap());
+foo.implements(Interface::from_name("Stringable"));
 ```
 
-You should implements the method `Stringable::__toString` after implemented
+You should implement the method `Stringable::__toString` after implementing
 `Stringable`, otherwise the class `Foo` will become abstract class.
 
 ## Add properties

phper/src/classes.rs

@@ -376,22 +376,39 @@ impl<T> Clone for StateClass<T> {
 #[derive(Clone)]
 pub struct Interface {
     inner: Rc<RefCell<*mut zend_class_entry>>,
+    name: Option<String>,
 }
 
 impl Interface {
     fn null() -> Self {
         Self {
             inner: Rc::new(RefCell::new(null_mut())),
+            name: None,
+        }
+    }
+
+    /// Create a new interface from global name (eg "Stringable", "ArrayAccess")
+    pub fn from_name(name: impl Into<String>) -> Self {
+        Self {
+            inner: Rc::new(RefCell::new(null_mut())),
+            name: Some(name.into()),
         }
     }
 
     fn bind(&self, ptr: *mut zend_class_entry) {
+        if self.name.is_some() {
+            panic!("Cannot bind() an Interface created with from_name()");
+        }
         *self.inner.borrow_mut() = ptr;
     }
 
     /// Converts to class entry.
     pub fn as_class_entry(&self) -> &ClassEntry {
-        unsafe { ClassEntry::from_mut_ptr(*self.inner.borrow()) }
+        if let Some(name) = &self.name {
+            ClassEntry::from_globals(name).unwrap()
+        } else {
+            unsafe { ClassEntry::from_mut_ptr(*self.inner.borrow()) }
+        }
     }
 }
 
@@ -411,7 +428,7 @@ pub struct ClassEntity<T: 'static> {
     method_entities: Vec<MethodEntity>,
     property_entities: Vec<PropertyEntity>,
     parent: Option<Box<dyn Fn() -> &'static ClassEntry>>,
-    interfaces: Vec<Box<dyn Fn() -> &'static ClassEntry>>,
+    interfaces: Vec<Interface>,
     constants: Vec<ConstantEntity>,
     bind_class: StateClass<T>,
     state_cloner: Option<Rc<StateCloner>>,
@@ -552,23 +569,22 @@ impl<T: 'static> ClassEntity<T> {
     /// implement multi interface, so this method can be called multi time.
     ///
     /// *Because in the `MINIT` phase, the class starts to register, so the*
-    /// *closure is used to return the `ClassEntry` to delay the acquisition of*
-    /// *the class.*
+    /// *`ClassEntry` is not fetched until later*
     ///
     /// # Examples
     ///
     /// ```no_run
-    /// use phper::classes::{ClassEntity, ClassEntry};
+    /// use phper::classes::{ClassEntity, ClassEntry, Interface};
     ///
     /// let mut class = ClassEntity::new("MyClass");
-    /// class.implements(|| ClassEntry::from_globals("Stringable").unwrap());
+    /// class.implements(Interface::from_name("Stringable"));
     ///
     /// // Here you have to the implement the method `__toString()` in `Stringable`
     /// // for `MyClass`, otherwise `MyClass` will become abstract class.
     /// // ...
     /// ```
-    pub fn implements(&mut self, interface: impl Fn() -> &'static ClassEntry + 'static) {
-        self.interfaces.push(Box::new(interface));
+    pub fn implements(&mut self, interface: Interface) {
+        self.interfaces.push(interface);
     }
 
     /// Add the state clone function, called when cloning PHP object.
@@ -633,7 +649,7 @@ impl<T: 'static> ClassEntity<T> {
             self.bind_class.bind(class_ce);
 
             for interface in &self.interfaces {
-                let interface_ce = interface().as_ptr();
+                let interface_ce = interface.as_class_entry().as_ptr();
                 zend_class_implements(class_ce, 1, interface_ce);
             }
 

tests/integration/src/classes.rs

@@ -11,7 +11,8 @@
 use phper::{
     alloc::RefClone,
     classes::{
-        ClassEntity, ClassEntry, InterfaceEntity, Visibility, array_access_class, iterator_class,
+        ClassEntity, ClassEntry, Interface, InterfaceEntity, Visibility, array_access_class,
+        iterator_class,
     },
     functions::{Argument, ReturnType},
     modules::Module,
@@ -83,8 +84,8 @@ fn integrate_foo(module: &mut Module) {
         array: Default::default(),
     });
 
-    class.implements(iterator_class);
-    class.implements(array_access_class);
+    class.implements(Interface::from_name("Iterator"));
+    class.implements(Interface::from_name("ArrayAccess"));
 
     // Implement Iterator interface.
     class
@@ -228,7 +229,7 @@ fn integrate_stringable(module: &mut Module) {
     use phper::{functions::ReturnType, types::ReturnTypeHint};
 
     let mut cls = ClassEntity::new(r"IntegrationTest\FooString");
-    cls.implements(|| ClassEntry::from_globals("Stringable").unwrap());
+    cls.implements(Interface::from_name("Stringable"));
     cls.add_method("__toString", Visibility::Public, |_this, _: &mut [ZVal]| {
         phper::ok("string")
     })

tests/integration/src/typehints.rs

@@ -92,11 +92,8 @@ fn make_foo_handler() -> ClassEntity<()> {
 
 fn make_foo_class(i_foo: Interface) -> ClassEntity<()> {
     let mut class = ClassEntity::new(r"IntegrationTest\TypeHints\Foo");
+    class.implements(i_foo);
 
-    // leak Interface so that ClassEntry can be retrieved later, during module
-    // startup
-    let i_foo_copy: &'static Interface = Box::leak(Box::new(i_foo));
-    class.implements(move || i_foo_copy.as_class_entry());
     class
         .add_method("getValue", Visibility::Public, |this, _| {
             let value = this