On Mon, Dec 28, 2020 at 7:54 PM Máté Kocsis <kocsismat...@gmail.com> wrote: > > > 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?
