Why does php allow invalid return type declerations it knows it can't allow?

638 Views Asked by At

As far as I can tell php has the ability to prevent a return type from being declared where it knows it's problematic.

class Foo {
    public function __clone(): Baz {
        return new Baz;
    }
}

class Baz {

}

$foo = new Foo;
$newFoo = clone $foo;

This results in a Fatal error: Clone method Foo::__clone() cannot declare a return type, which is perfectly sensible.

But then why would php allow things like this:

class Foo {
    public function __toString(): float {
        return "WAT!?!";
    }
}

echo new Foo;

This results in

Fatal error: Uncaught TypeError: Return value of Foo::__toString() must be of the type float, string returned

Which doesn't make sense, because were you to try and return a float:

Fatal error: Uncaught Error: Method Foo::__toString() must return a string value

Wouldn't it make more sense for php to prevent the declared return type of these types of methods rather than give those dubious errors? If not, what is the underlying reason behind this internally? Is there some mechanical barricade that prevents php from doing this where it can do it in cases like clone?

1

There are 1 best solutions below

0
bishop On

TL;DR: Supporting type inferences on magic methods breaks backwards compatibility.

Example: what does this code output?

$foo = new Foo;
$bar = $foo->__construct();
echo get_class($bar);

If you said Foo, you are incorrect: it's Bar.


PHP has a long, complicated evolution of its return type handling.

  • Before PHP 7.0, return type hints were a parse error.
  • In PHP 7.0, we got return type declarations with very simple rules (RFC), and after perhaps the most contentious internal debate ever, we got strict types (RFC).
  • PHP limped along with some oddities in co- and contra-variance until PHP 7.4, where we got many of these sorted (RFC).

The behavior of today reflects this organic growth, warts and all.

You indicate that the __clone() behavior is sensible, then compare that to the apparently non-sensical __toString() behavior. I challenge that neither of them are sensible, under any rational expectation of type inference.

Here's the __clone engine code:

6642     if (ce->clone) {                                                          
6643         if (ce->clone->common.fn_flags & ZEND_ACC_STATIC) {                   
6644             zend_error_noreturn(E_COMPILE_ERROR, "Clone method %s::%s() cannot be static",
6645                 ZSTR_VAL(ce->name), ZSTR_VAL(ce->clone->common.function_name));
6646         } else if (ce->clone->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {   
6647             zend_error_noreturn(E_COMPILE_ERROR,                              
6648                 "Clone method %s::%s() cannot declare a return type",         
6649                 ZSTR_VAL(ce->name), ZSTR_VAL(ce->clone->common.function_name));
6650         }                                                                     
6651     }                                                                         

Pay careful attention to that wording (emphasis mine):

Clone method ... cannot declare a return type

__clone() gave you an error not because the types were different, but because you gave a type at all! This also is a compile error:

class Foo {
    public function __clone(): Foo {
        return new Foo;
    }
}

"Why?!", you scream.

I believe there are two reasons:

  1. Internals is beholden to a high-bar of backwards compatibility maintenance.
  2. Incremental improvement comes slowly, each improvement building on earlier ones.

Let's talk about #1. Consider this code, which is valid all the way back to PHP 4.0:

<?php
class Amount {
    var $amount;
}
class TaxedAmount extends Amount {
    var $rate;
    function __toString() {
        return $this->amount * $this->rate;
    }
}
$item = new TaxedAmount;
$item->amount = 242.0;
$item->rate = 1.13;
echo "You owe me $" . $item->__toString() . " for this answer.";

Some poor soul used __toString as their own method, in a perfectly reasonable way. Now preserving its behavior is a top priority, so we can't make changes to the engine that break this code. That's the motivation for strict_types declaration: allowing opt-in changes to parser behavior so as to keep old behavior going while still adding new behavior.

You might ask: why don't we just fix this when declare(strict_types=1) is on? Well, because this code is perfectly valid in strict types mode, too! It even makes sense:

<?php declare(strict_types=1);

class Amount {
    var $amount;
}
class TaxedAmount extends Amount {
    var $rate;
    function __toString(): float {
        return $this->amount * $this->rate;
    }
}
$item = new TaxedAmount;
$item->amount = 242.0;
$item->rate = 1.13;
echo "You owe me $" . $item->__toString() . " for this answer.";

Nothing about this code smells. It's valid PHP code. If the method were called getTotalAmount instead of __toString, no one would bat an eye. The only odd part: the method name is "reserved".

So the engine can neither (a) enforce __toString returns a string type nor (b) prevent you from setting your own return type. Because to do either would violate backwards compatibility.

What we could do, however, is implement a new affirmative opt-in that says these methods aren't directly callable. Once we do that, then we can add type inference to them. Hypothetically:

<?php declare(strict_magic=1);

class Person {
    function __construct(): Person {
    }
    function __toString(): string {
    }
    // ... other magic
}

(new Person)->__construct(); // Fatal Error: cannot call magic method on strict_magic object

And this is point #2: once we have a way to protect backwards compatibility, we can add a way to enforce types on magic methods.

In summary, __construct, __destruct, __clone, __toString, etc. are both (a) functions the engine calls in certain circumstances for which it can reasonably infer types and (b) functions that - historically - can be called directly in ways that violate the reasonable type inference from (1).

This is the reason PR 4117 to fix Bug #69718 is blocked.

The only way to break this stale-mate: the developer opts in to a promise that these methods cannot be called directly. Doing that frees the engine to enforce strict type inference rules.