Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:84019
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.217.169 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <4ED7146272E04A47B986ED49E771E347D3111ACD63@Ikarus.ameusgmbh.intern>
References: <d238d736f8fcde9e649cb38c9b05fcc1@mail.gmail.com>
	<CALWK-fghdLMjt5yMMb23mnf0dNt+Mnx_tOYdLopfxLZFZvzTKA@mail.gmail.com>
	<4ED7146272E04A47B986ED49E771E347D3111ACD63@Ikarus.ameusgmbh.intern>
Date: Fri, 27 Feb 2015 10:31:03 -0500
Message-ID: <CAAyV7nG03-UxH_R91C4nHv2s_mhNrXpiFkfku5mCRD_g3RDBjQ@mail.gmail.com>
To: Christian Stoller <stoller@leonex.de>
Cc: Damien Tournoud <damz@damz.org>, Zeev Suraski <zeev@zend.com>, 
	PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Coercive STH - some real world tests and updated RFC
From: ircmaxell@gmail.com (Anthony Ferrara)

Cristian,

>> All those are due to a bug in substr(), that we see now only thanks to
>> proper type identification. There is no reason for substr() to ever return
>> a boolean. It really needs to be fix to always return a string.
>>
>> Damien
>
> It is not a bug. FALSE as a return value of substr() is the identificator
> for an error (e.g. invalid arguments), as it is stated in the documentation:
>
> "Returns the extracted part of string; or FALSE on failure, or an
> empty string." [1]
>
> This is an example which shows, that Zeevs RFC helps to find bugs in
> applications. Because here are given invalid arguments to the method. In
> other languages for example Java, you'll get an IndexOutOfBoundsException [2]
> if you are trying the same ;-)

Well, it depends on your definition of bug.

If you look at PHPUnit's TestSuite::isTestMethod method, you'll see this:

$doc_comment = $method->getDocComment();
return strpos($doc_comment, '@test') !== false ||
strpos($doc_comment, '@scenario') !== false;

Now, with the coercive mode (and my strict mode), that will error if
the method does not have a docblock.

The reason is that `$method->getDocComment()` returns false if one
does not exist.

Is that a bug? Well, semantically, maybe. But from the application
level, no. Passing the boolean result to `strpos` directly ensures
that the result will be `bool(false)`. So the overall method behaves
as expected returning true in the correct context (it's a test method)
and false in incorrect contexts (it's not).

So did the coercive mode find a bug? Not really. It found something
that may have worked in a way you didn't expect it to, but it wasn't a
bug (at least to the application).

Now, if you care about types and type changes, then yes, that is a
bug. And that's why the dual-mode RFC gives you the choice. You can
choose to care about types or not. Not some weird hybrid where you
have to care even if the code is practically correct...

Anthony