Unexpected Php8.4 Change Regarding CSV's

[oleibman]

Very recently, the PHP nightly build has changed, and the unit test suite now has over 100 new errors and 7 new risky tests as a result:

Exception: fgetcsv(): Passing a non-empty string to the $escape parameter is deprecated since 8.4 

I am not yet sure if this is truly an exception, or if the test suite has detected a deprecation message and converted it to an exception. I think the latter is probably the case, but I wanted to document this before I started investigating.

I find no documentation that such a change is even supposed to happen with 8.4, let alone any rationale for it. For the record, while I'm not a fan of using anything other than null string for the escape character, Php did not even allow the escape parameter to be a null string until 7.4, which is why Php (and, by extension, PhpSpreadsheet) uses backslash as the default.

The short-term fix might be as simple as using an at-sign to suppress the deprecation message, while working out what a good long-term strategy might be.

[oleibman]

Yes, the code issues a deprecation message, not an exception, so the at-sign method will work short-term. As one would expect, the default for the fgetcsv $escape parameter appears to have changed to null-string from backslash. The change does lead to a different result in Reader\Csv\CsvTest testEscapeCharacters, and presumably will lead to some problems for end-users when Php9 arrives.

This is a bit of a problem for us long-term. We can't really change our default default escape character in a non-breaking way. And, if we don't change it, then users will have to explicitly change the default before loading the Csv, even if they don't care and even if they are till now blissfully unaware of this option. Neither choice is good. At least we have some time to make a decision.

[oleibman]

I finally found some official documentation here. My plan for now is to continue to let escape default to backslash when running Php8, changing the default to null string when running Php 9. Furthermore, under Php 9, I plan to throw an exception if the user tries to set escape to anything other than null string. This could be a breaking change, one which we cannot avoid. However, I suspect that the vast majority of usage uses the default (whatever it is) and processes files that do not include backslashes, and, for such usage, this is not a break and does not require re-coding.

I will leave this ticket open for a while in order to give others the opportunity to comment.

[oleibman]

Ticket has been open long enough. No comments. Issue has been addressed. Closing.