Hi Marco Pivetta,

> > In fact, if reflection were to switch to the actual runtime return types of
> > those methods, I don't see a reason why downstream consumers would break
> > (stubbing tools, code generators, type checkers, dependency solvers, etc.)
> 
> If the published library/application had to support older versions (e.g. php 
> 7.4),
> but the tentative return types contained types/syntaxes that required php 8.0 
> (e.g. union types such as `string|false`, new types such as `mixed`/`never`, 
> etc,)
> then the code generators and type checkers and stubbing tools would need to 
> be 
> updated to exclude the new tentative return types much earlier than 
> absolutely needed.
> 
> From experience, code generated with tooling while running on newer PHP 
> versions is already incompatible with older PHP versions: you re-generate the 
> code when changing any of the dependencies anyway (think "no ABI 
> compatibility").
> 
> This is at least true for all codegen tools I worked/contributed to/used on 
> so far.

Mocking libraries and static analyzers that don't result in published code were 
my largest concern, generated code that gets published was a smaller one.

Changing getReturnType would significantly increase the scope of that 
incompatibility earlier on for users that don't install multiple php versions
(users/maintainers may default to whatever is provided by their package manager 
for convenience)

I'd rather have a larger time window with deprecations to change those and have 
any potentially breaking changes 
(from the perspective of users of older versions of code generation tools, test 
libraries, static analyzers)
in 9.0 instead of 8.1, to put the (small) BC breaks in major releases where 
possible.

The introduction of many `mixed` tentative types which makes sense from a type 
system perspective,
but with your alternate proposal for changing getReturnType(),  
but would result in code generating tools generating a lot of `: mixed` return 
types (requiring php 8.0+ runtime) in various interfaces and classes 
which would be incompatible with a missing return type override due to 
https://wiki.php.net/rfc/mixed_type_v2#explicit_returns

> We're mostly breaking BC (new methods on reflection symbols, requiring 
> special treatment) for stuff that is really an edge case that is only 
> affecting tooling that would really work just fine even if the reflection API 
> started to report the real return types now (no API change whatsoever).
> 
> What's the plan for PHP 9 about these methods? Deprecation/removal? Or are we 
> adding something that we'll have to drag on forever?

The RFC proposal https://wiki.php.net/rfc/internal_method_return_types stated 
those plans.

Unless new information comes up in the case of specific methods such as 
breaking commonly used frameworks,
in almost all cases, I'd assume tentative types in php 8.x would become real 
types in the next major version (php 9.0).

> Non-final internal method return types - when possible - are declared 
> tentatively in PHP 8.1,
> **and they will become enforced in PHP 9.0.** It means that in PHP 8.x 
> versions,
> a “deprecated” notice is raised during inheritance checks when an internal 
> method 
> is overridden in a way that the return types are incompatible, 
> **and PHP 9.0 will make these a fatal error.** A few examples:

Tentative return types would also be used by PECLs, so the 
getTentativeReturnType would continue to be used forever.
(I'd expect PECLs would generally add tentative types in `n.x.y` and change the 
real type in `(n+1).0.0`)

The alternate design you've proposed of changing getReturnType seems to have 
issues
- For user-defined types (if we allow an annotation mentioning a tentative 
return type exists without indicating the type),
  it'd be possible for hasTenativeReturnType to be true but getReturnType to be 
null, which is the opposite of internal classes
- As I'd mentioned before, if return type functionality gets extended to also 
work on functions that already have return types (user-defined and/or internal),
  in which case php would need to add 
`ReflectionFunctionAbstract->getRealReturnType`, but I'd rather keep the 
current semantics of `getReturnType`.

I personally expect your alternate proposal to be more controversial due to the 
larger potential bc break and barriers to upgrading in a minor release rather 
than a major release, but may be mistaken.

Thanks,
- Tyson

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to