Revert adding silent deprecations for legacy ClassLoader, Inflector and Lexer

[Majkl578]

Reverts 39bc396 in 2.9.

In #845 we added silent deprecations using the @trigger_error(..., E_USER_DEPRECATED) pattern (suggested in #845 (comment)).
This concept was originally created in a good faith to let users opt-in for deprecations only when they are interested in seeing them and aware it could happen (thus expecting deprecations may appear) and show none by default (for everyone else who doesn't opt-in).
Unfortunately Symfony had started misusing this concept and has turned it to opt-out, just like if these were non-silent deprecations without @ (messages logged, printed out and tests error out with exit code 1). In the end this means users are now seeing silenced deprecations unexpectedly without ever opting-in (in tests, debug toolbar etc.) and it breaks their test suite / CI.

Causes issues like: doctrine/orm#7306
Further (out-of-scope discussion): symfony/symfony#27936

Proposal: Revert now for 2.9, release as 2.9.1. Then reevaluate usefulness of the concept of silent deprecations for future use.

[Ocramius]

Proposal: Revert now for 2.9, release as 2.9.1. Then reevaluate usefulness of the concept of silent deprecations for future use.

I don't understand this bit: this would lead to a patch release with functional changes, no?

[stof]

Well, I would rather make a 2.6.3 release of the ORM which would have the fix merged in it to make the ORM stop using these deprecated APIs. The goal of these warnings is precisely to make it clear when a deprecated API is used, to push to migrate. The ORM was still using some APIs deprecated years ago.

The phpunit-bridge opting-in for the report of deprecations has precisely this goal: reporting when your code uses deprecated APIs.
And I would not say that Symfony turned it into an opt-out. The phpunit-bridge is not a mandatory requirement. If you don't want to opt-in for deprecation reporting in your tests, don't install it.

[nicolas-grekas]

This is not the right move for Doctrine and its users (including Symfony): triggering silenced deprecations is the way to go. As @stof explained above and on symfony/symfony#27936, depractions are opt-in on Symfony also.

[Majkl578]

triggering silenced deprecations is the way to go.

It would be if Symfony hadn't started turning them into hard errors for anything in vendor without intentionally enabling this behavior.
Package A calling deprecated API of package B should not make user's test suite error out, that is not acceptable.

As @stof explained above and on symfony/symfony#27936, depractions are opt-in on Symfony also.

No, they are not, see symfony/symfony#27936 (comment). This erroneous behavior is enabled by default and exits with code 1.

[nicolas-grekas]

The right move IMHO is to resubmit symfony/symfony#27609 now that Doctrine doesn't use its own deprecated things internally.

[Majkl578]

To make it clear what the expected behavior is (to me and also to other end-user consumers):

1. I do want reporting when my code in src/ calls any deprecated API from src/
2. I do want reporting when my code in src/ calls deprecated API from vendor/ directly
3. I do not want reporting when my code in src/ calls non-deprecated API in vendor/ that indirectly calls some deprecated API of another package in vendor/ (out of scope of my app)
4. I do not want reporting when some package in vendor/ calls deprecated API of another package in vendor/ (out of scope of my app)

Symfony's default behavior currently violates 3) and 4).

At this moment I recommend abandoning the concept of @trigger_error(...) and using https://github.com/phpstan/phpstan-deprecation-rules instead.

The right move IMHO is to resubmit symfony/symfony#27609 now that Doctrine doesn't use its own deprecated things internally.

Agreed that it should be resubmitted, but this PR is rather about issues like doctrine/orm#7306 because of doctrine/orm#7307.

[nicolas-grekas]

I'd recommend to close this PR.
On Symfony's side, tests are now green with dev branches of doctrine/*.
About the defaults, things will be fixed once symfony/recipes#442 is merged.
These deprecations will help ppl moving to v3. By adding them, we allow ppl to more easily notice when their code will break before it actually happens. Reciprocally, by removing these deprecations, some people will notice the BC break the hard way: by triggering fatal errors after moving to v3. This would be much worse (not everyone on Earth runs static analyses, nor even has 100% test coverage, so that for many this will happen in prod also.)

[Majkl578]

The default behavior of PHPUnit Bridge is still wrong in my eyes, by default it's reporting issues that are out-of-scope for the project where it's used.
Just like you don't run static analysis, CS or tests on vendor/ directory, PHPUnit bridge should not report deprecations that happen within vendor/ - In other words, 3) and 4) from my previous comment are still happening.

On Symfony's side, tests are now green with dev branches of doctrine/*.

This is primarily about everyone out there who now gets deprecation errors and test failures just because Doctrine ORM internally calls deprecated ClassLoader from Doctrine Common.

[alcaeus]

The default behavior of PHPUnit Bridge is still wrong in my eyes, by default it's reporting issues that are out-of-scope for the project where it's used.

That's not our problem, and that's not a reason to drop these deprecation warnings. The change to the phpunit-bridge prevents failing builds due to these deprecations while still reporting them (which people can turn off using weak_vendors if they want). If people complain about these deprecations, we tell them why they're not relevant to them and how they can ignore them. For another solution, see next point.

This is primarily about everyone out there who now gets deprecation errors and test failures just because Doctrine ORM internally calls deprecated ClassLoader from Doctrine Common.

The fix to that is publishing a release with these usages dropped and be done with it, not get into an even lengthier discussion about these deprecations. Obviously, the deprecations will show up for some users who aren't running the latest version, but if you're running the latest version of one dependency and an older version of another that's something you have to expect and deal with. After the deprecation is done, the focus should be to drop the deprecated behavior as soon as possible.

As for reevaluating "usefulness of the concept of silent deprecations for future use": these deprecations are extremely useful when preparing a major version upgrade, so we should keep them in at all costs. If the tooling around them is sub-optimal, that's not our problem. Instead, this should be fixed upstream instead of dropping the concept altogether. With that said, I'm closing this PR.