Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83943
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.45 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CACouGvmRKX+qrexHfoW-9k0E9vW-ZPkRc6oz0vZ1g9CH7krQMQ@mail.gmail.com>
References: <CACouGvmRKX+qrexHfoW-9k0E9vW-ZPkRc6oz0vZ1g9CH7krQMQ@mail.gmail.com>
Date: Thu, 26 Feb 2015 16:51:08 -0800
Message-ID: <CAEZPtU60vw2-j9sNaPLfaM4zi5DQnOaEjg4ODojoSo-+UPdsPw@mail.gmail.com>
To: "Matthew Weier O'Phinney" <matthew@zend.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Follow-up to STH user experience, this time with actual testing
From: pierre.php@gmail.com (Pierre Joye)

On Thu, Feb 26, 2015 at 4:29 PM, Matthew Weier O'Phinney
<matthew@zend.com> wrote:


> 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.

Thanks for this detailed report, much appreciated!

> 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.

It seems you still are still confused about that one :) This opt-in
mode is the key value of this RFC. Nothing changes if you do not care
and do not want to care.

And if you care and want to port part of your code, you can, being
fully informed about what it means for this file.

> ### 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.)

And now let wait all other cases not covered by errors but casting to
different values, maybe only a few, maybe none, we simply do not know.

Cheers,