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

Warnings invisible #644

Open
mokraemer opened this issue Jan 31, 2023 · 6 comments
Open

Warnings invisible #644

mokraemer opened this issue Jan 31, 2023 · 6 comments

Comments

@mokraemer
Copy link

Setting warnings with
@trigger_error
does not show any warnings, if an custom error handler is installed!

Since php 7.x the error handler should check for error_reporting() and the reported type.
If an @ is added to trigger_error, this disables the error_reporting for this type.

I don't see, why you add an @ before trigger_error... What is the intention to suppress here. I suggest to remove the @, so the warning is visible to the users.

@stof
Copy link
Member

stof commented Jan 31, 2023

Those deprecation warnings are reported following the Symfony convention, which avoids that an error handler turns them into exceptions (which would break things). Several projects (Symfony, Drupal, maybe Laravel but I'm not sure) have tooling that support that convention.

@mokraemer
Copy link
Author

I'm fine with deprecated. But the @ makes them invisible.
I just stumbled across those warnings and have still used "compile" instead of compileText beacuse I've never seen that warning.

@stof
Copy link
Member

stof commented Feb 6, 2023

This @ is precisely the part that avoid making them too intrusive.
Symfony 2.7.0 initially implemented deprecations without @ and this broke most projects because their error handlers were turning deprecations into exceptions.

And as said before, if you have a tooling that is aware of this convention, it can make them visible.
Also, for deprecated methods, we also put the @deprecated tag in the phpdoc, so IDEs and static analysis tools will also report such usages.

@mokraemer
Copy link
Author

if you add scssphp as a library, you never see the deprecation-flag.

I think the implementation of Symfony to turn deprecations to errors is nonsense. If someone intends to raise exceptions (errors) they will do this. Even php raises deprecations as a fundamental instrument to warn that this feature is soon removed.
E.g. old php-module mcrypt raises deprecation errors and if you don't want them and are still in progress of changing you write @mcrypt....() so all errors are suppressed, as intended. You do this intentionally so you know this will fail soon.

If you now suppress all warnings already in the lib, how should I distinguish between intenionally suppressed warnings and warnings in libs?
I assume your idea of setting this to deprecated is to remove that code in the near future.

Leave it, I don't put too much effort in this - but I don't think this is a wise decission, others like me, will never see this warning and get an fatal error in case you ever remove that piece of code.

@stof
Copy link
Member

stof commented Feb 6, 2023

We follow semver. So the removal will only happen in the next major version.

@mokraemer
Copy link
Author

like php - so you remove the @ at the last release before the next final? At least this is the point where it should be intrusive.

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

No branches or pull requests

2 participants