Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114132
To: "internals@lists.php.net" <internals@lists.php.net>
Thread-Topic: [PHP-DEV] [RFC] [Vote] Adding return types to internal methods
Thread-Index: AQHXN0sQDOXF4BnTpkyjI+fKo2GPt6rAKyIAgAO81wI=
Date: Sat, 24 Apr 2021 17:24:21 +0000
Message-ID:
 <DM6PR07MB66185BDCC679C2F1D621A9A7F9449@DM6PR07MB6618.namprd07.prod.outlook.com>
References:
 <CAH5C8xX87sVh5NDuQ1EfeEbSxdHawO5yNfk5zHEToAqYGJa7=Q@mail.gmail.com>,<CADyq6sJ5StVLhgEW86xXcVeETTBUodXWrFkv0Lpin+9+du_WJg@mail.gmail.com>
In-Reply-To:
 <CADyq6sJ5StVLhgEW86xXcVeETTBUodXWrFkv0Lpin+9+du_WJg@mail.gmail.com>
Accept-Language: en-CA, en-US
Content-Language: en-CA
x-ms-exchange-messagesentrepresentingtype: 1
x-tmn: [EtFtXbB23fb4xlsIJAlIo252g3+8yECCSlqinZmsVcLcLxfhbyGt6vgiEwTmVQiU]
x-ms-publictraffictype: Email
x-incomingheadercount: 44
x-eopattributedmessage: 0
x-ms-office365-filtering-correlation-id: 1a6ae933-2dba-4735-4e1f-08d90745ccd4
x-ms-traffictypediagnostic: BN8NAM04HT042:
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info:
 6ah0zadrr6p3cB4S4YlB+gx+tQHK4oMhS63EwzTZrKTexXmo/Zn/vzEvqx4nEewN+e57fxApJdNxOozDMexFV8xNN5R3K31tndbXQExtBsOCe379TBfbZt3hM8gnbRNjsA7AWWdCkrgW6la38asxvyg0ePUFoQ57AxDmI50RsrkcLY9tO1MfeIU2TRXKT7cgSrCbV7CXsWFp04UttwCBjrIVAtWTGK0Oo3aH2V/zKEl+IZIsehysFyvfoC6CUibUjp4nMZNy7lqhfr9tjI/xEDQk+o4+bo30P8+H2m7lpwZLPdhlwPX2xN48VFhoQZSfmRkzScMKOcNj17rg2jXWcdwjod8t5VjTTCeC0HVv1sRxztAyRmDz+zl77eoZnSR7cNf3iv8FiZ08MDnjPsgSS1bUE9t+OQBVDSoZZfnYUd8=
x-ms-exchange-antispam-messagedata:
 xaa5nFHOrd+u8mRfobnElMGcmTp/OjV/nLsr8M9Y533zkf/aFfTtKhACITsD+GkmPTpkT2gtvuVLCKOK1J8UIa2winTJlllh5coQXCZvUDQYTaxVGD5w9WFv+YCfh2Yfg9+XX6fX+Ihxspn19EAfkL835/d94T0WEBy9qiOuDCZCcO9fxRmRLfDfTnjzYpCR6q437/MW0Z8wl3wwc6Utjg==
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: hotmail.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-AuthSource: BN8NAM04FT032.eop-NAM04.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 1a6ae933-2dba-4735-4e1f-08d90745ccd4
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Apr 2021 17:24:21.9179
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Internet
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8NAM04HT042
Subject: Re: [PHP-DEV] [RFC] [Vote] Adding return types to internal methods
From: tysonandre775@hotmail.com (tyson andre)

Hi Marco Pivetta,=0A=
=0A=
> In fact, if reflection were to switch to the actual runtime return types =
of=0A=
> those methods, I don't see a reason why downstream consumers would break=
=0A=
> (stubbing tools, code generators, type checkers, dependency solvers, etc.=
)=0A=
=0A=
If the published library/application had to support older versions (e.g. ph=
p 7.4),=0A=
but the tentative return types contained types/syntaxes that required php 8=
.0 =0A=
(e.g. union types such as `string|false`, new types such as `mixed`/`never`=
, etc,)=0A=
then the code generators and type checkers and stubbing tools would need to=
 be =0A=
updated to exclude the new tentative return types much earlier than absolut=
ely needed.=0A=
=0A=
- Users would benefit from code/tooling working the same way in php 7.x and=
 8.1 when upgrading to 8.1=0A=
  and making the tooling unexpectedly stricter may be regarded as a breakin=
g change by end users, especially for unmaintained tools/libraries.=0A=
=0A=
=A0 I'd prefer if the tooling authors and end users had to opt in to use th=
e tentative return types=0A=
=A0 and upgrade to a version of tooling that was aware of the tentative ret=
urn types to start using them.=0A=
- Forcing getReturnType to change immediately would be a barrier to upgradi=
ng, especially for users that aren't deeply familiar with php, =0A=
=A0 if the php version or library versions being used in production weren't=
 compatible with the latest versions =0A=
=A0 of those stubbing tools, code generators, type checkers, dependency sol=
vers, etc. that were aware of tentative return types=0A=
  (e.g. a test mock no longer returning null)=0A=
=0A=
  To upgrade from php 7.3 (or older) to 8.1, a user may want applications a=
nd libraries that worked the same way in both 7.3 and 8.1,=0A=
  and would only want to upgrade the applications/libraries (and fix the te=
ntative type notices) after they stopped using php 7=0A=
- PHP 8.0 would be only one year older than 8.1 and automatically generatin=
g more user-defined subclasses with union types=0A=
=A0 this early on (e.g. and publishing to packagist) would be inconvenient =
for users still on php 7.=0A=
=0A=
Also, as mentioned by Nikita Popov in https://externals.io/message/113413#1=
14052=0A=
**Having a distinction between getReturnType and getTentativeReturnType als=
o allows the functionality in this RFC to be extended in the future,=0A=
e.g. from a getReturnType of `BaseType` to getTentativeReturnType of `SubTy=
pe`, rather than only being useful when return types are missing**=0A=
=0A=
Additionally, I agree with the points made by Nikita/M=E1t=E9 Kocsis - olde=
r releases of static analyzers would treat getReturnType as if it was defin=
itely the real type,=0A=
and falsely treat some type checks as **definitely** redundant/impossible r=
ather than **probably** redundant/impossible,=0A=
leading users to remove those checks with insufficient validation/testing/r=
eview, before it's definitely safe/correct to do so.=0A=
=0A=
- third party code in vendor dependencies (and mocks generated for unit tes=
ts) are typically not analyzed for issues,=0A=
=A0 but third party code might override internal classes and make those see=
mingly redundant/impossible checks actually required).=0A=
- E.g. if getReturnType were to change instead of adding getTentativeReturn=
Type,=0A=
  older releases of `phan` with `--redundant-condition-detection` would fal=
sely report that conditions were definitely impossible/redundant when it is=
 still possible for subclasses to return different types.=0A=
=0A=
(I'm a maintainer of the static analyzer http://github.com/phan/phan/ and w=
ould personally prefer the getTentativeReturnType approach,=0A=
Marco Pivetta(Ocramius) works on/contributes to various php projects/analyz=
ers such as BetterReflection)=0A=
=0A=
Thanks,=0A=
-Tyson=