Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:65656
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.212.48 as permitted sender)
MIME-Version: 1.0
Date: Mon, 4 Feb 2013 19:56:54 -0500
Message-ID: <CALeP-ExAiAbiAPgtw8Dh_U5BpBtNGB=sGqMpOCzeecE3WrbP5g@mail.gmail.com>
To: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=bcaec502d7188d9ae704d4efaf4d
Subject: SplFileObject::__toString potentially returns false
From: cspray.php@gmail.com (Charles Sprayberry)

--bcaec502d7188d9ae704d4efaf4d
Content-Type: text/plain; charset=ISO-8859-1

There was a previous discussion on the return value of
SplFileObject::__toString being
an alias to SplFileObject::current() but that was about the design decision
why and not
really suggesting any kind of change or a technical error. In the
discussion it was
established that:

(1) Changing this would be a BC break
(2) It isn't really a bug in the codebase, the docs were just wrong

The current implementation has a bug in that current() can potentially
return false, if
false is returned from the __toString() magic method a catchable fatal
error occurs.

Code ran on 5.4.11 *nix, Apache 2.2

--TEST--
Ensuring we get appropriate output for SplFileObject::__toString
--FILE--
<?php

$file = fopen('file.txt', 'w');
fwrite($file, 'A');
fclose($file);

$FileObject = new SplFileObject('file.txt');
foreach ($FileObject as $line) {
    echo $FileObject;
}

echo $FileObject;
?>
--EXPECTF--
A


---------
Diff

002+ Catchable fatal error: Method SplFileObject::__toString() must return
a string value
in /path/to/splfileobject_tostring.php on line 12

---------

I would say that SplFileObject:__toString() being an alias to
SplFileObject::current()
makes the use cases for the method very limited and that the current
implementation is
mistaken in returning current(). I can think of no use case where
SplFileObject::__toString()
should return this value. If need to get the current line you're probably
iterating and in
that case should be using foreach() and don't really need to manually take
care of that.
Even if there is a use case for manually invoking
`SplFileObject::current()` you probably
aren't casting a string in that situation and a more expressive method call
is better.

(1) Leave the implementation the way it is and simply return a blank string
when
current() would return false

(2) Change the implementation to return SplFileInfo::__toString()

(3) Change the implementation to return entire file contents

(4) Return some other unthought of string but remains consistent and logical

I am personally partial to (2) or (3) perhaps in 5.5 but (1) could probably
happen
sooner? Thoughts?

--bcaec502d7188d9ae704d4efaf4d--