# SplFileObject::__toString potentially returns false

- Thread: https://externals.io/message/65656
- Original archive: https://news-web.php.net/php.internals/65656
- Started by Charles Sprayberry on 2013-02-05T00:56:54+00:00
- Messages: 3
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #65656 — Charles Sprayberry — 2013-02-05T00:56:54+00:00

URL: https://externals.io/message/65656#65656

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?

---

## #65657 — Levi Morrison — 2013-02-05T01:51:55+00:00

URL: https://externals.io/message/65656#65657 · In reply to: #65656

> 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.

---

## #65670 — hakre — 2013-02-05T13:45:07+00:00

URL: https://externals.io/message/65656#65670 · In reply to: #65656

----- 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