replace -1 with E_ALL in error_reporting calls

[ThomasMeschke]

Description
Replaces error_reporting(-1); with error_reporting(E_ALL); for better readability.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• [O] Unit testing, with >80% coverage
• [O] User guide updated
• [O] Conforms to style guide

O = does not apply

[kenjis]

Thank you for sending PR!

But I don't see the benefit to change.

[ThomasMeschke]

Only benefit is readability =)
E_ALL is clear for everybody and no need to go and check the doc what "-1" means.
No impact when merging, no impact when not merging, but also no loss if merging. =)

[kenjis]

Related #8219

[kenjis]

Certainly E_ALL is easier to understand, but it seems difficult for someone who does not know what -1 is to understand the meaning of error_reporting().

I would like to hear what others say.

[datamweb]

I believe the first user contribution should be reviewed with kindness. I'm fine with that.

[ThomasMeschke]

No further comments for a week now. Let's not make this a long-runner, shall we?

[paulbalandan]

From ChatGPT:

In PHP, both error_reporting(-1); and error_reporting(E_ALL); are used to set the error reporting level to the maximum, meaning that all types of errors, warnings, and notices will be reported.

The difference between the two is subtle:

1. error_reporting(-1);: This sets the error reporting level to the maximum possible value. It includes all error types, even those that might be added in future PHP versions. Using -1 is more future-proof, as it ensures that your code will catch all errors, including any new ones introduced in later PHP versions.

2. error_reporting(E_ALL);: This explicitly sets the error reporting level to include all errors, warnings, and notices. While this achieves the same result as -1 in current PHP versions, it might not include potential new error types introduced in future PHP versions.

In practice, either of these statements is generally acceptable for development environments where you want to catch and address all types of errors. However, if you want to be as future-proof as possible, using error_reporting(-1); might be a slightly safer choice.

For production environments, it's recommended to log errors instead of displaying them to users, and the error reporting level might be set to a lower value to avoid exposing sensitive information.

https://chat.openai.com/c/c76a03d0-714d-493b-b990-f5b92c3e51ac

[kenjis]

The ChatGPT answer is not correct.

And since error levels will be added over time, the maximum value (for E_ALL) will likely change.
https://www.php.net/manual/en/errorfunc.configuration.php

[ThomasMeschke]

I would prefer to trust the official documentation which in the Example and the Note declares: They are the same.
https://www.php.net/manual/en/function.error-reporting

From ChatGPT:

In PHP, both error_reporting(-1); and error_reporting(E_ALL); are used to set the error reporting level to the maximum, meaning that all types of errors, warnings, and notices will be reported.

The difference between the two is subtle:

1. error_reporting(-1);: This sets the error reporting level to the maximum possible value. It includes all error types, even those that might be added in future PHP versions. Using -1 is more future-proof, as it ensures that your code will catch all errors, including any new ones introduced in later PHP versions.
2. error_reporting(E_ALL);: This explicitly sets the error reporting level to include all errors, warnings, and notices. While this achieves the same result as -1 in current PHP versions, it might not include potential new error types introduced in future PHP versions.

In practice, either of these statements is generally acceptable for development environments where you want to catch and address all types of errors. However, if you want to be as future-proof as possible, using error_reporting(-1); might be a slightly safer choice.

For production environments, it's recommended to log errors instead of displaying them to users, and the error reporting level might be set to a lower value to avoid exposing sensitive information.

https://chat.openai.com/c/c76a03d0-714d-493b-b990-f5b92c3e51ac

[datamweb]

php.ini say:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
; Error handling and logging ;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

; This directive informs PHP of which errors, warnings and notices you would like
; it to take action for. The recommended way of setting values for this
; directive is through the use of the error level constants and bitwise
; operators. The error level constants are below here for convenience as well as
; some common settings and their meanings.
; By default, PHP is set to take action on all errors, notices and warnings EXCEPT
; those related to E_NOTICE and E_STRICT, which together cover best practices and
; recommended coding standards in PHP. For performance reasons, this is the
; recommend error reporting setting. Your production server shouldn't be wasting
; resources complaining about best practices and coding standards. That's what
; development servers and development settings are for.
; Note: The php.ini-development file has this setting as E_ALL. This
; means it pretty much reports everything which is exactly what you want during
; development and early testing.
;
; Error Level Constants:
; E_ALL             - All errors and warnings
; E_ERROR           - fatal run-time errors
; E_RECOVERABLE_ERROR  - almost fatal run-time errors
; E_WARNING         - run-time warnings (non-fatal errors)
; E_PARSE           - compile-time parse errors
; E_NOTICE          - run-time notices (these are warnings which often result
;                     from a bug in your code, but it's possible that it was
;                     intentional (e.g., using an uninitialized variable and
;                     relying on the fact it is automatically initialized to an
;                     empty string)
; E_STRICT          - run-time notices, enable to have PHP suggest changes
;                     to your code which will ensure the best interoperability
;                     and forward compatibility of your code
; E_CORE_ERROR      - fatal errors that occur during PHP's initial startup
; E_CORE_WARNING    - warnings (non-fatal errors) that occur during PHP's
;                     initial startup
; E_COMPILE_ERROR   - fatal compile-time errors
; E_COMPILE_WARNING - compile-time warnings (non-fatal errors)
; E_USER_ERROR      - user-generated error message
; E_USER_WARNING    - user-generated warning message
; E_USER_NOTICE     - user-generated notice message
; E_DEPRECATED      - warn about code that will not work in future versions
;                     of PHP
; E_USER_DEPRECATED - user-generated deprecation warnings
;
; Common Values:
;   E_ALL (Show all errors, warnings and notices including coding standards.)
;   E_ALL & ~E_NOTICE  (Show all errors, except for notices)
;   E_ALL & ~E_NOTICE & ~E_STRICT  (Show all errors, except for notices and coding standards warnings.)
;   E_COMPILE_ERROR|E_RECOVERABLE_ERROR|E_ERROR|E_CORE_ERROR  (Show only errors)
; Default Value: E_ALL
; Development Value: E_ALL
; Production Value: E_ALL & ~E_DEPRECATED & ~E_STRICT
; https://php.net/error-reporting
error_reporting = E_ALL

[kenjis]

E_ALL and -1 mean the same thing.
However, the value of E_ALL may change in the future.

It is a bit special that -1 means all, but the value does not change, and -1 is easier to understand than 32767 when checking the value of error_reporting().

Before:

error_reporting() integer -1

After:

error_reporting() integer 32767

<?php

namespace App\Controllers;

class Home extends BaseController
{
    public function index(): string
    {
        dd(error_reporting());
    }
}

[kenjis]

I prefer not to change it because -1 is easier to understand.

By the way, I am also against changing this value in production environment.
See #8219

[MGatner]

Well, today I learned that E_ALL != -1 😅

The docs are clear using either is equivalent, as @ThomasMeschke noted:

Passing in the value -1 will show every possible error, even when new levels and constants are added in future PHP versions. The behavior is equivalent to passing E_ALL constant.

E_ALL is easier to read in the code but -1 is more discernible in debug. I really don't have a preference - I would be inclined to agree with @datamweb that we merge just for the joy of having a new contributor (thanks @ThomasMeschke!) but @kenjis pours his life essence into this project so if he believes in -1 then let's stick with that.

[MGatner]

Also: shoutout to the ChatGPT cameo - showing up just in time with a confabulation 🙄

[datamweb]

E_ALL and -1 mean the same thing. However, the value of E_ALL may change in the future.

It is a bit special that -1 means all, but the value does not change, and -1 is easier to understand than 32767 when checking the value of error_reporting().

Before:

error_reporting() integer -1

After:

error_reporting() integer 32767

<?php

namespace App\Controllers;

class Home extends BaseController
{
    public function index(): string
    {
        dd(error_reporting());
    }
}

It was an interesting explanation.

but @kenjis pours his life essence into this project

Yes, it really is.

The good thing about this conversation was that I learned new things.
Thanks all!

[lonnieezell]

First off, @ThomasMeschke thanks for your first PR.

So what was decided? Sounds like we're keeping it at -1? If that's so can we close this? Or are we still debating?

I don't have strong feelings either way. I do think that E_ALL is more readable so would be inclined to switch it, especially since the official docs say they're the same thing.

Tip Passing in the value -1 will show every possible error, even when new levels and constants are added in future PHP versions. The behavior is equivalent to passing E_ALL constant.

It also seems we just switched production to use E_ALL as part of it's default setting.

Again - no strong opinion but I do lean toward more readable. Always. :)

[kenjis]

In production, we use and will use E_ALL. So let's use it in other places.