I've taken some time the last couple days to compile both the Scalare Type Hints v0.5 (heretofor STHv5) and Coercive Scalar Type Hints (heretofore STHcoerce) patches and test some code against them.
In each case, I modified the `Phly\Http\Response` class from my phly/http package to add scalar type hints, remove previous argument validation, and then ran the unit tests. Here's my details of the experience, and analysis. ### STHv5 With STHv5, my tests failed out of the box. First, I needed to add an error handler that would convert the E_RECOVERABLE_ERROR to an InvalidArgumentException so that I would get useful error messages from PHPUnit. Next, once I had done that, I had tests that were throwing invalid input at methods, and validating that the invalid arguments were caught. This worked in all but one specific case: passing a float value to an argument expecting an integer. In this particular case, the value was coerced to an integer, albeit lossily, and no error was raised. When I changed my test case to operate under strict_types mode, the test executed as I had originally expected, and succeeded. #### Analysis I did not expect the float value to be coerced, particularly as it had a fractional part. Yes, I understand that this is how casting _currently_ works in PHP, and that the patch uses the current casting rules. However, I'd expect that any float with a fractional value would not be cast to an integer; doing so is lossy, and can lead to unexpected results. The strict_types mode operated how I would expect it, but meant I was forced to add the declaration to the top of the file. Which leads to this: My tests operated differently in strict vs normal mode. That means my code does, too. Testing both modes would be difficult to do in an automated fashion; essentially, you're left needing to choose to support one mode or the other. This goes back to the point I was making earlier this week: I worry that having two modes will lead to a schism in userland libraries: those that support strict, and those that do not; there will be little possibility of testing both. ### STHcoerce With STHcoerce, my tests also failed out of the box, but not for the same reason. Instead, I had a bunch of errors reported based on code that PHPUnit was executing! In this case, I discovered that: - PHPUnit passes a boolean false to `debug_backtrace()`... which is documented as expecting an integer! (There are actually several constant values it accepts, all of which are integer values.) In this case, PHPUnit is relying on the fact that the engine casts booleans to the integers 0 and 1. (Zeev has written to the list already indicating that this coercion path will be supported in the patch.) - PHPUnit is passing the results of $reflector->getDocComment() blindly to substr() and preg_match*(). getDocComment() is documented as returning EITHER a string OR boolean false. Again, PHPUnit is relying on PHP to cast boolean false to an empty string. (Zeev has also indicated this coercion path may be re-introduced.) I was able to fix these in a matter of minutes, and then my tests ran, and passed without any changes. #### Analysis STHcoerce worked about how I expect STHv5 will work _if_ _every_ _file_ were declared in strict_types mode. In other words, it uncovered errors in calling both userland and internal functions. Userland typehints worked as I expected, with floats using fractional values not casting to integers. ### Wrap Up STHcoerce was actually far more strict than I found strict_types mode to be in STHv5. The reason is that it's a single, non-optional mode. This poses a potential challenge to migration, as noted in my problems using PHPUnit (Yes, I WILL be sending patches their way soon!): method calls and arguments that previously worked because of how PHP juggles types often do not work, particularly when going from boolean to other scalar values. However, the patch does what I'd expect in terms of preventing operations that would result in either data loss or data additions, all of which can have unexpected side effects if you don't understand the casting rules currently. The flip side to this is that you do not need to make any changes to your code in order to locate and fix errors. What this means from a library/framework author perspective is that, if the STHcoerce patch is merged, I can test against PHP 7, make fixes, and my code is not only forward-compatible, but also backwards to the various PHP 5 versions I might already be supporting — and that code now benefits from being more correct. Because STHv5 is opt-in only, the only way to see similar type errors is to somehow automate the addition of the strict_types declaration to all files in your project. While it can be done with tools like sed and awk, realistically it means that existing libraries cannot and/or will not benefit easily from the changes until they support PHP 7 as a minimum version. Additionally, it poses potential problems in terms of different testing results based on which mode you run in, leading to ambiguity for users. The only way to address the latter as a library author is to specify which mode your code is known to run in. Ironically, I'm seeing STHcoerce as more strict than STHv5 in terms of type handling, due to the fact that every call, userland or internal, is affected. The fact that it's always on makes it simpler for me to test against it now, and makes it easier to make my existing code both forward-compatible and more correct — even if I do not add type hints until I update my minimum supported version to PHP 7. I am slightly worried about the amount of work needed to make code run from v5 to v7 if STHcoerce is adopted. That said, the issues I found in PHPUnit were corrected in minutes (though a more thorough patch that allows early return from methods if doc comments are not strings would be better), and my own library's tests ran without problems once the PHPUnit issues were corrected (both code with and without typehints). Strict_types is definitely interesting, but I found that strict_types mode was more or less how I wanted code to operate, and to opt-in to that means changes to every file in my library — which is a much larger migration problem. In the end: kudos to everyone who is working on these patches and RFCs. I'm excited about scalar type hints in PHP 7! -- Matthew Weier O'Phinney Principal Engineer Project Lead, Zend Framework and Apigility matt...@zend.com http://framework.zend.com http://apigility.org PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
