[RFC] Adding return types to internal methods (PHP 8) View as Markdown

Hi Internals,

Last year, Nikita started a discussion about adding return types
to non-final internal methods: https://externals.io/message/106539 .

I'd like to restart the conversation, since I've just created an
implementation
for the first step of the migration:
https://github.com/php/php-src/pull/6548
(I had to start a new email thread because I don't have the original
one).

My implementation currently emits a deprecation notice during inheritance
validation
for each method which omits the return type of its parent. This means
that the parent return type is not yet enforced when the child omits it,
neither during
inheritance, nor at run-time. However, as soon as the child class
declares
return types,
everything will behave as usual, so variance and run-time return value
checks
both apply. Finally, in a couple of years, PHP 9.0 could make the
declaration of
return types required.

If there are concerns about methods which are already declared with the
wrong return
type, we could lax the restrictions even further, and only emit a
deprecation notice
instead of a fatal error in this case as well.

I chose the above approach over inheriting the return type implicitly
(which was suggested
by Sara) because I believe it has a few advantages:

• It causes less BC break: methods not declaring a return type will
  surely
  continue to work
  as before, and gradual migration it also supported
• It has negligible run-time impact because diagnostics is emitted
  maximum
  once per
  any method which misses the return type. If return types were inherited
  implicitly,
  incorrect return values would potentially trigger lots of errors.
• It is more straightforward behavior than implicit inheritance, no
  "magic"
  is involved

Even though I prefer the current implementation (or its more lax
variant),
I'm also not
opposed to going with implicit inheritance if that's what we settle on.

I appreciate any input, especially about the possible impact of the
different approaches.

Regards:
Máté

Is there any way we could make this mechanism more broadly available than
just for internal methods? This is a problem that also shows up when adding
return types in userland libraries.

A concern I have is that for methods with union return types, there
wouldn't be any way to avoid a deprecation warning without having PHP 8 as
the minimum version requirement. And given PHP's general error handling
story, deprecation warnings are not exactly graceful...

We have the same issue in Symfony. We have a special class loader that
triggers a deprecation when a parent class has an @return annotation but
the child class doesn't have a return type. Also, we don't trigger a
deprecation when the child class also has the @return annotation. The
reason for this is to allow child classes to be deprecation-free.
By duplicating the annotation from the parent, child classes tell they know
a return type should be added in their next major, which should happen
before the next major of their parent class.

PHP could adopt the same strategy using an attribute. When a method has
that attribute, it'd mean the same: skip the deprecation regarding it's
parent classes, and trigger a deprecation when child classes either 1.
don't have an explicit or 2. don't have the same annotation.

WDYT?