Hi Gina

On Mon, Jun 2, 2025 at 6:28 PM Gina P. Banyard <intern...@gpb.moe> wrote:
>
> RFC: https://wiki.php.net/rfc/void-as-null

After a read, I think I fundamentally disagree with the proposal. It
says (regarding the status-quo):

> * void is not a subtype of a function with a mixed return type

This is laid out as a downside, but I don't think it is. Consider this
example under the specified behavior:

    interface MapInterface {
        public function set(string $key, mixed $value): void;
    }

    class Map implements MapInterface  {
        public function set(string $key, mixed $value): void {
            // Store the key/value pair _somehow_
        }
    }

Let's assume the return type `MapInterface::set()` `mixed` instead,
where the intention is for `set()` to return the previous value, or
`null` if there was none. This change will go completely unnoticed by
`Map::set()`, because `void` is now just `null`, which is a subtype of
`mixed`. This is a bug that would previously have been caught,
notifying you that you're supposed to return _something_.

Similarly, the code `function foo(): void { return null; }` is now
proposed to be valid, and I assume the inverse for `void`/`return;` is
also true. In this example, we now update `Map::set()` to the new
return type.

    class Map implements MapInterface  {
        public function set(string $key, mixed $value): mixed {
            $oldValue = /* Fetch old value _somehow_ */;
            // Store the key/value pair _somehow_

            if (!$this->observers) {
                return; // This line was here before.
            }

            $this->observers->notify($key, $oldValue, $value);

            return $oldValue;
        }
    }

Good examples are hard, but imagine `Map::set()` would allow notifying
a list of observers about changes to the map. Previously, the
`return;` would have prevented an erroneous call to `notify()` on
`null`. However, now it is missing the returning of `$oldValue`. This
is another bug that would previously have been caught. In fact, even a
missing trailing `return $something;` would not be caught anymore.

IMO, these checks are useful enough not to be removed.

Please let me know if I'm missing anything.

Ilija

Reply via email to