Fix lang.ClassLoader::defineType() for explicitely nullable types

xp-framework · Mar 24, 2024 · 7674ee7 · 7674ee7
1 parent d975372
commit 7674ee7
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 13 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -3,6 +3,12 @@ XP Framework Core ChangeLog
 
 ## ?.?.? / ????-??-??
 
+### Bugfixes
+
+* Fixed `lang.ClassLoader::defineType()` to generate correctly typed
+  method forwards for methods with explicitely nullable types.
+  (@thekid)
+
 ## 12.0.0 / 2024-03-23
 
 This major release adopts more PHP 8 semantics, getting rid of the XP

diff --git a/src/main/php/lang/ClassLoader.class.php b/src/main/php/lang/ClassLoader.class.php
@@ -224,13 +224,10 @@ protected static function defineForward($name, $func, $invoke) {
     foreach ($func->getParameters() as $param) {
       $p= $param->getName();
 
-      $t= $param->getType();
-      if (null === $t) {
-        $constraint= '';
-      } else if ($t->isBuiltin()) {
-        $constraint= $t->getName();
+      if ($t= $param->getType()) {
+        $constraint= ($t->allowsNull() ? '?' : '').($t->isBuiltin() ? '' : '\\').$t->getName();
       } else {
-        $constraint= '\\'.$t->getName();
+        $constraint= '';
       }
 
       if ($param->isVariadic()) {
@@ -246,13 +243,8 @@ protected static function defineForward($name, $func, $invoke) {
     }
 
     $decl= 'function '.$name.'('.substr($sig, 2).')';
-    $t= $func->getReturnType();
-    if (null === $t) {
-      // NOOP
-    } else if ($t->isBuiltin()) {
-      $decl.= ':'.$t->getName();
-    } else {
-      $decl.= ': \\'.$t->getName();
+    if ($t= $func->getReturnType()) {
+      $decl.= ':'.($t->allowsNull() ? '?' : '').($t->isBuiltin() ? '' : '\\').$t->getName();
     }
 
     if (null === $invoke) {

diff --git a/src/test/php/lang/unittest/NewInstanceTest.class.php b/src/test/php/lang/unittest/NewInstanceTest.class.php
@@ -195,6 +195,46 @@ public function arguments_are_passed_to_base_constructor_in_closuremap() {
     Assert::equals($this, newinstance($base->getName(), [$this], [])->test);
   }
 
+  #[Test]
+  public function scalar_typed_parameter() {
+    $instance= newinstance(Marker::class, ['Test'], [
+      'test'        => null,
+      '__construct' => function(string $test) { $this->test= $test; },
+      'get'         => function(): string { return $this->test; }
+    ]);
+    Assert::equals('Test', $instance->get());
+  }
+
+  #[Test]
+  public function value_typed_parameter() {
+    $instance= newinstance(Marker::class, [$this], [
+      'test'        => null,
+      '__construct' => function(NewInstanceTest $test) { $this->test= $test; },
+      'get'         => function(): NewInstanceTest { return $this->test; }
+    ]);
+    Assert::equals($this, $instance->get());
+  }
+
+  #[Test, Values(['Test', null])]
+  public function nullable_scalar($arg) {
+    $instance= newinstance(Marker::class, [$arg], [
+      'test'        => null,
+      '__construct' => function(?string $test= null) { $this->test= $test; },
+      'get'         => function(): ?string { return $this->test; }
+    ]);
+    Assert::equals($arg, $instance->get());
+  }
+
+  #[Test, Values(eval: '[new Name("Test"), null]')]
+  public function nullable_value_type($arg) {
+    $instance= newinstance(Marker::class, [$arg], [
+      'test'        => null,
+      '__construct' => function(?Name $test= null) { $this->test= $test; },
+      'get'         => function(): ?Name { return $this->test; }
+    ]);
+    Assert::equals($arg, $instance->test);
+  }
+
   #[Test, Condition(assert: 'self::processExecutionEnabled()')]
   public function variadic_argument_passing() {
     $r= $this->runInNewRuntime('