Re: [RFC] Add true as type

From: Date: Mon, 11 Apr 2022 13:32:58 +0000
Subject: Re: [RFC] Add true as type
References: 1 2  Groups: php.internals 
Request: Send a blank email to internals+get-117511@lists.php.net to get a copy of this message
On Sat, 9 Apr 2022 at 12:05, Tim Düsterhus <tim@bastelstu.be> wrote: > Hi > > On 4/8/22 18:47, G. P. B. wrote: > > Now that null and false can be used as standalone types, I think it > makes a > > lot of sense to add true as a value type for the sake of completeness > with > > false. > > I agree that it would make sense to have 'true' as the counterpart of > false for completeness, but … > > > It also has many use cases within internals and userland to declare > proper > > type signatures to functions and methods. > > > … seeing the examples of functions that might return true, > but won't > return false, I feel like the correct solution is not adding > true as a > standalone type, but instead making those function return something more > obvious. > > - The sort() family should rather be be ': void' > which would make it > more obvious to static analysis that '$sorted = sort($unsorted)' is not > a correct usage. > - DiagnoseCommand::checkComposerSchema() in composer would > benefit > from having support for tagged unions, as currently both the error case > (string) and the success case > (true) are truthy. With the current > PHP 8.1 feature set I can imagine that returning a proper bool and > putting the reason in an string &$reason reference > parameter would > make it more obvious to use this function in an if (). > - JsonFile::validateSchema() in composer should be ': > void' as errors > appear to be communicated using Exceptions. > - The closure in ComposerRepository::asyncFetchFile() would > benefit > from having support for tagged unions, but even with the current PHP 8.1 > feature set I can imagine that: > > enum CacheStatus { case Hit; } > > and then making the function mixed[]|CacheStatus instead of > mixed[]|true would make it more obvious what the meaning of the > second > option in the union is. > > Best regards > Tim Düsterhus > There are many many many more internal functions in PHP which only return true, but only since PHP 8.0.0, and this is due to the huge amount of E_WARNING to ValueError/TypeError promotion which has happened. These functions previously did return false in certain circumstances, and although I agree that changing these to void *would* be the most ideal, being able to do this communicating that these functions only return true (which means one can ignore the return value) is the first step. Moreover, it is even more of a BC break compared to changing something which only returns false to void as code like: if (always_true(...)){ ... } would stop executing this code path. Not adding true as a type prevents extending methods which return bool to always return true to clearly document this within the typesystem. Best regards, George P. Banyard

« previous php.internals (#117511) next »