added version detection rather then suppressing obvious error

[brainfoolong]

This is:

- [x] a bugfix

Checklist:

• Changes are covered by unit tests
• Code style is respected
• Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
• CHANGELOG.md contains a short summary of the change
• Documentation is updated as necessary

Why this change is needed?

With PHP 8.1 ini_set('auto_detect_line_endings') is deprecated, as already correctly marked in the code.
But adding an explicit version detection instead of just letting the error silently happen, which fills internal error buffer and later still is returned by error_get_last() which many use to detect if any uncatchable php error happened.

In our case, we catch open errors with error_get_last() on shutdown and throw an exception when a error exist. This function also returns errors that are suppressed with @ and there is no way to detect if this error was suppressed.

With the version detection, we omit the error before it can happen, which is a nicer way instead of ducking.

[brainfoolong]

The failed test for PHP 8.1 does fail, as the version detection disable setAutoDetect.
In the official changelog there is already this entry, which already says that starting from PHP 8.1 the feature is not available anymore. The test should reflect this and should skip the test starting from 8.1. But i am not sure how you handle this in the repository. This should be fixed by someone that knows that.

• PHP 8.1 will deprecate auto_detect_line_endings. As a result of this change, Csv Reader using PHP8.1+ will no longer be able to handle a Csv with Mac line endings.

[oleibman]

It sounds like you should be running a custom error handler which prevents error_get_last from seeing this message. Your solution has 2 problems. It takes code which works under 8.1 (the setting is deprecated, not eliminated), as you can see from the failing test, and makes it not work. Now, the documentation does say it won't work with 8.1+, so perhaps that is an out.

A bigger problem is that, although eliminating this particular message is straightforward, there are other places in the code which suppress messages. Those may not have an easy route to elimination, and, even if they have, I don't know that we want to get in the business of eliminating messages which we have already suppressed.

The following establishes an error_handler which would handle this situation:

set_error_handler(
    function (int $severity, string $message, string $file, int $line): bool
    {
        if (!(bool) (error_reporting() & $severity)) {
            return true;
        }
        return false;
    }
);

[brainfoolong]

Ok i understand. I thought the cleaner way is version checking instead of suppression. Also because PhpSpreadsheet already have a lot of code that has version checks for deprecated features, insead of suppression, especially from PHP 7 to PHP 8.

Changing our error handler is not an easy thing to do and would only require doing this for this library. We generally not support error suppression here in our environment.

Maybe you may reconsider this, if not, no problem, than you can close this. I can deal with both variants.

[oleibman]

Closing for now. This will need a full-fledged solution when a version of PHP which officially stops supporting Mac line endings (8.2?) is available for testing.