Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore suppressed E_USER_* errors again #5326

Merged
merged 1 commit into from
Apr 14, 2023
Merged

Ignore suppressed E_USER_* errors again #5326

merged 1 commit into from
Apr 14, 2023

Conversation

sbuerk
Copy link
Contributor

@sbuerk sbuerk commented Apr 13, 2023

Basically, there are two ways to silence error levels:

  • set error_reporting to a value, not containing the
    error levels which should be silenced/not displayed
  • using the silence operator @ to enforce silencing
    the error level, unrelated to the set error reporting
    level

For example, it's possible that E_ALL has been set
as global error_level() and therefore triggering a
user deprecation/warning/error using the trigger_error
method will be displayed. If the @ silence operator
is used before the trigger_error() call, the error
is silenced.

error_reporting(E_ALL);
trigger_error('', E_USER_DEPRECATED);  // will be shown
@trigger_error('', E_USER_DEPRECATED); // will not be shown

However, registered error handler will be called for all
errors, despite it has been silenced or not for both possible
ways to silence a error levels. ErrorHandler check the
passed $errorNumber against the result of error_reporting()
to determine if the errorNumber/errorLevel is supressed or
not. That check will also detect a errorNumber as beeing
silenced if the @ silence operator has been used, which
could and should be taken as a intentional and enforced
silencing of an error (knowing that it would fail).

There are multple use-cases, where for e.g. silenced user
deprecation should not be taken as such, displayed and fail.

MockBuilder converts @deprecated method docblock annotations
to silenced @trigger_error('', E_USER_DEPRECATED) calls.
Having a @deprecated annonation does not mean, that the
original method call would trigger an E_USER_DEPRECATED
error - and it's arguable that it should be a hard enforcement
by PHPUnit to decide this and doing the conversion.

Therefore, this change removes the possible silenceable error
level list check from the ErrorHander. This restores how it
has worked in PHPUnit v9 without opening follow up issues.

This can be reintroduced, either in a configurable way or the
same if related questions has been properly thought about and
solved before:

  • MockBuilder @Deprecation -> trigger_error() configurable or
    removed
  • proper way to configure between own code / 3rd party code
    if silenced error levels should be stay silenced or not.
    This means not only in one direction.

Related: #5325

Basically, there are two ways to silence error levels:

* set error_reporting to a value, not containing the
  error levels which should be silenced/not displayed
* using the silence operator `@` to enforce silencing
  the error level, unrelated to the set error reporting
  level

For example, it's possible that `E_ALL` has been set
as global error_level() and therefore triggering a
user deprecation/warning/error using the `trigger_error`
method will be displayed. If the `@` silence operator
is used before the `trigger_error()` call, the error
is silenced.

```php
error_reporting(E_ALL);
trigger_error('', E_USER_DEPRECATED);  // will be shown
@trigger_error('', E_USER_DEPRECATED); // will not be shown
```

However, registered error handler will be called for all
errors, despite it has been silenced or not for both possible
ways to silence a error levels. ErrorHandler check the
passed `$errorNumber` against the result of error_reporting()
to determine if the errorNumber/errorLevel is supressed or
not. That check will also detect a errorNumber as beeing
silenced if the `@` silence operator has been used, which
could and should be taken as a intentional and enforced
silencing of an error (knowing that it would fail).

There are multple use-cases, where for e.g. silenced user
deprecation should not be taken as such, displayed and fail.

MockBuilder converts `@deprecated` method docblock annotations
to silenced `@trigger_error('', E_USER_DEPRECATED)` calls.
Having a `@deprecated` annonation does not mean, that the
original method call would trigger an `E_USER_DEPRECATED`
error - and it's arguable that it should be a hard enforcement
by PHPUnit to decide this and doing the conversion.

Therefore, this change removes the possible silenceable error
level list check from the `ErrorHander`. This restores how it
has worked in PHPUnit v9 without opening follow up issues.

This can be reintroduced, either in a configurable way or the
same if related questions has been properly thought about and
solved before:

* MockBuilder @Deprecation -> trigger_error() configurable or
  removed
* proper way to configure between own code / 3rd party code
  if silenced error levels should be stay silenced or not.
  This means not only in one direction.

Related: #5325
@codecov
Copy link

codecov bot commented Apr 13, 2023

Codecov Report

Merging #5326 (c5b0855) into main (fcff1c5) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #5326      +/-   ##
============================================
- Coverage     84.77%   84.77%   -0.01%     
+ Complexity     6049     6048       -1     
============================================
  Files           651      651              
  Lines         19209    19208       -1     
============================================
- Hits          16284    16283       -1     
  Misses         2925     2925              
Impacted Files Coverage Δ
src/Runner/ErrorHandler.php 71.05% <100.00%> (-0.38%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sebastianbergmann sebastianbergmann changed the title [TASK] Take silenced E_USER_* errors as silenced again Ignore suppressed E_USER_* errors again Apr 14, 2023
@sebastianbergmann sebastianbergmann added this to the PHPUnit 10.1 milestone Apr 14, 2023
@sebastianbergmann sebastianbergmann merged commit 95b5cb8 into sebastianbergmann:main Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants