Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/http-client/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const EXCEPTION_CLASS_NAME: &str = "HttpClient\\HttpClientException";
pub fn make_exception_class() -> ClassEntity<()> {
let mut class = ClassEntity::new(EXCEPTION_CLASS_NAME);
// The `extends` is same as the PHP class `extends`.
class.extends(exception_class);
class.extends("Exception");
class
}

Expand Down
2 changes: 1 addition & 1 deletion examples/http-server/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@ impl From<HttpServerError> for phper::Error {
pub fn make_exception_class() -> ClassEntity<()> {
let mut class = ClassEntity::new(EXCEPTION_CLASS_NAME);
// As an Exception class, inheriting from the base Exception class is important.
class.extends(exception_class);
class.extends("Exception");
class
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ Now let's begin to finish the logic.
pub fn make_exception_class() -> ClassEntity<()> {
let mut class = ClassEntity::new(EXCEPTION_CLASS_NAME);
// The `extends` is same as the PHP class `extends`.
class.extends(exception_class);
class.extends("Exception");
class
}

Expand Down Expand Up @@ -155,7 +155,7 @@ Now let's begin to finish the logic.
# pub fn make_exception_class() -> ClassEntity<()> {
# let mut class = ClassEntity::new(EXCEPTION_CLASS_NAME);
# // The `extends` is same as the PHP class `extends`.
# class.extends(exception_class);
# class.extends("Exception");
# class
# }
#
Expand Down
2 changes: 1 addition & 1 deletion phper-doc/doc/_06_module/_06_register_class/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ If you want the class `Foo` extends `Bar`, and implements interface `Stringable`
use phper::classes::{ClassEntity, ClassEntry, Interface};

let mut foo = ClassEntity::new("Foo");
foo.extends(|| ClassEntry::from_globals("Bar").unwrap());
foo.extends("Bar");
foo.implements(Interface::from_name("Stringable"));
```

Expand Down
6 changes: 3 additions & 3 deletions phper-doc/doc/_06_module/_07_register_interface/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ interface Foo {}
If you want the interface `Foo` extends `ArrayAccess` and `Iterator` interfaces.

```rust,no_run
use phper::classes::{InterfaceEntity, ClassEntry};
use phper::classes::{Interface, InterfaceEntity, ClassEntry};
use phper::classes::{array_access_class, iterator_class};

let mut foo = InterfaceEntity::new("Foo");
foo.extends(|| array_access_class());
foo.extends(|| iterator_class());
foo.extends(Interface::from_name("ArrayAccess"));
foo.extends(Interface::from_name("Iterator"));
```

Same as:
Expand Down
37 changes: 21 additions & 16 deletions phper/src/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ pub struct ClassEntity<T: 'static> {
state_constructor: Rc<StateConstructor>,
method_entities: Vec<MethodEntity>,
property_entities: Vec<PropertyEntity>,
parent: Option<Box<dyn Fn() -> &'static ClassEntry>>,
parent: Option<String>,
interfaces: Vec<Interface>,
constants: Vec<ConstantEntity>,
bind_class: StateClass<T>,
Expand Down Expand Up @@ -550,7 +550,7 @@ impl<T: 'static> ClassEntity<T> {
/// Register class to `extends` the parent class.
///
/// *Because in the `MINIT` phase, the class starts to register, so the*
/// *closure is used to return the `ClassEntry` to delay the acquisition of*
/// *`ClassEntry` is looked up by name to delay the acquisition of*
/// *the class.*
///
/// # Examples
Expand All @@ -559,10 +559,10 @@ impl<T: 'static> ClassEntity<T> {
/// use phper::classes::{ClassEntity, ClassEntry};
///
/// let mut class = ClassEntity::new("MyException");
/// class.extends(|| ClassEntry::from_globals("Exception").unwrap());
/// class.extends("Exception");
/// ```
pub fn extends(&mut self, parent: impl Fn() -> &'static ClassEntry + 'static) {
self.parent = Some(Box::new(parent));
pub fn extends(&mut self, parent_name: impl Into<String>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be better if it accepted a ClassEntry (which could come from a just-created class or defer looking up from globals, similar to Interface::from_name), but I couldn't make it work. I think this is at least more ergonomic than the previous way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about StateClass<()>? Add method from_name just for StateClass<()>.

Copy link
Member

@jmjoy jmjoy Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter can accept StateClass<T>, which can be transmute to StateClass<()> internally. Anyway, T is just phantom data in StateClass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer. Some progress, now we can either extends(StateClass<T>) or extends_name(String), the second one being useful for built-ins that we don't have a StateClass for. I could allow lazily creating a StateClass for built-ins, to unify those two methods - WDYT?

Copy link
Member

@jmjoy jmjoy Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I prefer to change it this way to be consistent with the implements method. What do you think?

master...jmjoy:phper-fork:extends

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, it's much nicer. I will try to cherry-pick this into my PR.

self.parent = Some(parent_name.into());
}

/// Register class to `implements` the interface, due to the class can
Expand Down Expand Up @@ -631,12 +631,14 @@ impl<T: 'static> ClassEntity<T> {
#[allow(clippy::useless_conversion)]
pub(crate) unsafe fn init(&self) -> *mut zend_class_entry {
unsafe {
let parent: *mut zend_class_entry = self
.parent
.as_ref()
.map(|parent| parent())
.map(|entry| entry.as_ptr() as *mut _)
.unwrap_or(null_mut());
let parent: *mut zend_class_entry = if let Some(ref name) = self.parent {
let entry = ClassEntry::from_globals(name).unwrap_or_else(|err| {
panic!("Unable to resolve parent class: {}: {}", name,err);
});
entry.as_ptr() as *mut _
} else {
null_mut()
};

let class_ce = phper_init_class_entry_ex(
self.class_name.as_ptr().cast(),
Expand Down Expand Up @@ -800,20 +802,23 @@ impl InterfaceEntity {
/// Register interface to `extends` the interfaces, due to the interface can
/// extends multi interface, so this method can be called multi time.
///
/// *Because in the `MINIT` phase, the class starts to register, so the*
/// *Because in the `MINIT` phase, the class starts to register, a*
/// *closure is used to return the `ClassEntry` to delay the acquisition of*
/// *the class.*
///
/// # Examples
///
/// ```no_run
/// use phper::classes::{ClassEntry, InterfaceEntity};
/// use phper::classes::{Interface, InterfaceEntity};
///
/// let mut interface = InterfaceEntity::new("MyInterface");
/// interface.extends(|| ClassEntry::from_globals("Stringable").unwrap());
/// interface.extends(Interface::from_name("Stringable"));
/// ```
pub fn extends(&mut self, interface: impl Fn() -> &'static ClassEntry + 'static) {
self.extends.push(Box::new(interface));
pub fn extends(&mut self, interface: Interface) {
self.extends.push(Box::new(move || {
let entry: &'static ClassEntry = unsafe { std::mem::transmute(interface.as_class_entry()) };
entry
}));
}

#[allow(clippy::useless_conversion)]
Expand Down
17 changes: 13 additions & 4 deletions tests/integration/src/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
use phper::{
alloc::RefClone,
classes::{
ClassEntity, ClassEntry, Interface, InterfaceEntity, Visibility, array_access_class,
iterator_class,
ClassEntity, ClassEntry, Interface, InterfaceEntity, Visibility,
},
functions::{Argument, ReturnType},
modules::Module,
Expand All @@ -27,6 +26,7 @@ pub fn integrate(module: &mut Module) {
integrate_i_bar(module);
integrate_static_props(module);
integrate_i_constants(module);
integrate_bar_extends_foo(module);
#[cfg(phper_major_version = "8")]
integrate_stringable(module);
}
Expand Down Expand Up @@ -175,8 +175,8 @@ fn integrate_foo(module: &mut Module) {
fn integrate_i_bar(module: &mut Module) {
let mut interface = InterfaceEntity::new(r"IntegrationTest\IBar");

interface.extends(|| array_access_class());
interface.extends(|| iterator_class());
interface.extends(Interface::from_name("ArrayAccess"));
interface.extends(Interface::from_name("Iterator"));

interface
.add_method("doSomethings")
Expand Down Expand Up @@ -224,6 +224,15 @@ fn integrate_static_props(module: &mut Module) {
module.add_class(class);
}

fn integrate_bar_extends_foo(module: &mut Module) {
let mut cls = ClassEntity::new(r"IntegrationTest\BarExtendsFoo");
cls.extends(r"IntegrationTest\Foo");
cls.add_method("test", Visibility::Public, |_,_| {
phper::ok(())
});
module.add_class(cls);
}

#[cfg(phper_major_version = "8")]
fn integrate_stringable(module: &mut Module) {
use phper::{functions::ReturnType, types::ReturnTypeHint};
Expand Down
5 changes: 5 additions & 0 deletions tests/integration/tests/php/classes.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,8 @@ class Foo2 extends IntegrationTest\Foo {}
assert_false(IntegrationTest\IConstants::CST_FALSE);
assert_eq(100, IntegrationTest\IConstants::CST_INT);
assert_eq(10.0, IntegrationTest\IConstants::CST_FLOAT);

// Test module class extends module class
$bar = new \IntegrationTest\BarExtendsFoo; //Bar should extend Foo
$reflection = new ReflectionClass($bar);
assert_true($reflection->isSubclassOf(IntegrationTest\Foo::class));
Loading