8000 Qualification of deprecations as direct or indirect is wrong for autoloading ones · Issue #36036 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

Qualification of deprecations as direct or indirect is wrong for autoloading ones #36036

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.

Sign up for GitHub

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

Jump to bottom
Closed
stof opened this issue Mar 12, 2020 · 4 comments
Closed

Qualification of deprecations as direct or indirect is wrong for autoloading ones #36036

stof opened this issue Mar 12, 2020 · 4 comments
Labels
Bug Debug ErrorHandler PhpUnitBridge Stalled Status: Needs Review

Comments

@stof
Copy link
Member
stof commented Mar 12, 2020

Symfony version(s) affected: Symfony 4.4.5, PHPUnit Bridge 5.0.5

Description
For deprecations triggered by the DebugClassLoader (when extending a deprecated class for instance), the qualification of the deprecation as direct or indirect in the PHPUnit Bridge seems to be based on the place triggering the autoload call, rather than on the file containing the class definition for which the deprecation is triggered

How to reproduce

  • composer require xabbuh/panda-bundle 1.4.1 (which contains event classes which have not been migrated to the new base class in contracts, but any other vendor extending a deprecated class would do it)
  • write some code in the project using one of its event classes (in my case, it is the getSubscribedEvents() method of an event subscriber accessing the constant with the event name)
  • run tests with the PHPUnit bridge
  • The deprecation The "Xabbuh\PandaBundle\Event\EncodingCompleteEvent" class extends "Symfony\Component\EventDispatcher\Event" that is deprecated since Symfony 4.3, use "Symfony\Contracts\EventDispatcher\Event" instead. is reported as a direct deprecation while it should be invalid.

On the other hand, my own event classes (being in the same case as I haven't migrated them yet) are reported as indirect deprecations because the autoloading is triggered by Symfony itself when loading all classes defined in src.

Possible Solution
DebugClassLoader should have a way to force the source of the deprecation to be in the file rather than being in the autoloader IMO. But I don't know yet how to achieve it (/cc @nicolas-grekas)
@nicolas-grekas
Copy link
Member

I would to fix this from the outside : maybe the phpunit bridge can detect the situation and act accordingly?

@stof
Copy link
Member Author
stof commented Sep 16, 2020

@nicolas-grekas I don't see how that would work in the bridge alone. We can detect that the source of a deprecation is inside DebugClassLoader. But I don't see how we could then find out which file was loaded to find the class for which a deprecation is triggered, if the DebugClassLoader does not help us.

Maybe a trick could be to inspect the arguments of DebugClassLoader::checkClass in a stack trace to identify the class and then call DebugClassLoader::findFile() or another API to find the file again, but I'm not sure we can get that info (do we have the stack trace with arguments ?)

@fancyweb fancyweb mentioned this issue Oct 16, 2020
Merged
derrabus added a commit that referenced this issue Nov 26, 2020
@derrabus
bug #38597 [PhpUnitBridge] Fix qualification of deprecations triggere…
6224ffa 
…d by the debug class loader (fancyweb)

This PR was merged into the 4.4 branch.

Discussion
----------

[PhpUnitBridge] Fix qualification of deprecations triggered by the debug class loader

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #36036
| License       | MIT
| Doc PR        | -

I tested it successfully against the two examples stof gave in the issue. However, I don't see how to write a working real test for this.

The solution I found is to add the missing checked file in the original files stack so when the type of the deprecation is computed instead of having "self" -> "self", we have "vendor" -> "self".

Commits
-------

dff5394 [PhpUnitBridge] Fix qualification of deprecations triggered by the debug class loader
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@nicolas-grekas
Copy link
Member

Fixed by #40508

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Debug ErrorHandler PhpUnitBridge Stalled Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nicolas-grekas @stof @carsonbot
0