SplFileObject::__toString potentially returns false View as Markdown

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?

I don't really use this particular object, but from a distance I think
something should change. Returning false seems like a bug or design
flaw. I can probably be persuaded for any of options 1-3 though I
prefer them in 2, 1, 3 order.

Looking it up in the helly docs
(http://www.php.net/~helly/php/ext/spl/classSplFileObject.html#a79319d744507f828a752981e6f63bf15)
I see that this was always the intended behavior, though I don't know
why.

----- Ursprüngliche Message -----

Von: Charles Sprayberry cspray.php@gmail.com
An: "internals@lists.php.net" internals@lists.php.net
CC:
Gesendet: 1:56 Dienstag, 5.Februar 2013
Betreff: [PHP-DEV] SplFileObject::__toString potentially returns false

T here 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.
[...]
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?

I often use this object. A few things I can notice:

• In practice I never ran over this error. Mainly because I use foreach rather than the object itself in string context to obtain the line(s).

• Even I did not run into a concrete problem, I would consider this a bug because the object itself is violating the __toString() interface. When I get that error with my own code, I always consider this a bug and fix it. So this should be fixed in PHP core code when I keep myself as a mark.

• Because SplFileObject is a subtype of SplFileInfo, it should return the same for the same interface, e.g. __toString() must return the same type of value as of SplFileInfo to not break the substitution. Only the more specific parts of the interface are allowed to offer more specific behavior. Right now it violates LSP. This should be addressed and fixed behaving exactly like SplFileInfo.

IMHO fixing all these flaws require a BC break which I would consider something good in this specific case. So I welcome a BC break as soon as possible so that in the future functions accepting a SplFileInfo also work with SplFileObjects if they make use of the string context.

Until then, a workaround is to provide your own subclass of SplFileObject that overwrites __toString() again. Maybe this is something useful for projects that make use of the SplFileObject already.

-- hakre