Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:98427
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain garfieldtech.com from 66.111.4.25 cause and error)
To: internals@lists.php.net
References: <CAPhkiZzs=kLF-JKBjX4bVgADz+nonERWuZYiYo8cXA=GrXKU=g@mail.gmail.com>
Message-ID: <ad8a0c6c-a0a8-3b9e-1300-07bce3913b66@garfieldtech.com>
Date: Wed, 8 Mar 2017 09:13:17 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.0
MIME-Version: 1.0
In-Reply-To: <CAPhkiZzs=kLF-JKBjX4bVgADz+nonERWuZYiYo8cXA=GrXKU=g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [Discussion] is_string(), string type and objects
 implementing __toString()
From: larry@garfieldtech.com (Larry Garfield)

On 03/08/2017 06:25 AM, Andrey Andreev wrote:
> Hi all,
>
> I submitted a GitHub PR* to allow objects implementing __toString() to
> *optionally* pass is_string() validation. More verbose wording of my
> motivation can be seen in the PR description, but here are the main
> points:
>
> - Simpler way to do checks like: is_string($var) ||
> method_exists($var, '__toString')
> - Can be used for stricter string parameter validation in
> strict_types=0 mode (otherwise any scalar type is accepted)
> - Can be used for looser string parameter validation in strict_types=1
> mode (__toString() objects aren't accepted there)
> - Regardless of the last 2 points, it is intentionally not limited to
> parameter types
>
> * https://github.com/php/php-src/pull/2408
>
> ---
>
> I didn't have time to write this email right after submitting the
> patch, and in the meantime got some feedback from Fleshgrinder on
> GitHub, which I'll quote and address here:
>
>> Thanks for your effort and initiative.
>>
>> However, I strongly believe that this is the wrong approach. Adding a flag to a function directly results in the fact that the function violates the single responsibility principle. What we actually need to make this work is a "stringable" pseudo-type like the iterable type that was introduced in PHP 7.1. This "stringable" pseudo-type is the union of the scalar primitive string and any class that implements the __toString method.
>>
>> This has the advantage that we are actually able to use it together with strict_types, plus we have separate dedicated functions like "is_stringable" that adhere to the single responsibility principle. I actually wanted to create an RFC for that along with an implementation since iterable was accepted, but did not find the time yet.
>>
>> Closing note: these pseudo-types are necessary in PHP because it has no coherent type system, and there is nothing we can do about this in short term. Hence, adding such pseudo-types is the only short term solution that we actually have.
> I ultimately wouldn't care if it's a separate function and did in fact
> think of an is_stringable() function, but wasn't happy with the naming
> - who's to say that e.g. integers aren't stringable? Bar
> horribly-verbose names like
> "string_or_objects_implementing__toString", I don't think there's a
> way to avoid that ambiguity. :/
> If we want a "stringable" type though, I guess we'll have to live with that.
>
> I feel that debating the actual type system is way broader than I
> intended this to be, so I'll refrain from going further on that for
> now, as I've got some more radical ideas about it.
>
> ---
>
> Thoughts?
>
> Cheers,
> Andrey.

I would concur with Fleshgrinder.  Given the type system we have to work 
with, a stringable pseudo-type that translates to is_string() (subject 
to the type mode) || method_exists($var, '__toString') (not subject to 
type mode) would be cleaner and more flexible than yet another flag.  
Flags on method signatures are almost always a code smell.

(I suppose there's a debate to be had if an int is stringable in strict 
mode; I'm not sure there myself.)

--Larry Garfield