-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Removes usage of classes which don't exist from DB migration scripts. #22446
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
Removes usage of classes which don't exist from DB migration scripts. #22446
Conversation
|
Hi @hostep. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
orlangur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hostep really nice finding! Are you going to proceed with Magento code improving using phpstan? 😉
|
Hi @orlangur, thank you for the review. |
|
Hi @hostep, thank you for your contribution! |


Description (*)
This PR was sparked after some research in #22124
It might solve #22124 but it wasn't determined yet how to reproduce that issue, so I'm not going to mention it in the Fixed Issues list for now (but please let me know if I need to edit my post to add it).
Other issues solved in this PR were discovered by executing phpstan with level 0 over the DB migration scripts:
(fun fact: phpstan reports around 214 violations of these non-existing classes in the entire code base, if somebody wants to go fix all of these, be my guest :))
Issue 1 - BaseImage
The
BaseImageclass was removed a long time ago in 566be67#diff-6ab33404f7c9e2c38f69b1d172d923a9, but its usage wasn't removed from the DB Migration scripts.Issue 2 - Media
The
Mediaclass was removed an even longer time ago in 0f0063045b5#diff-7efc962fb113e767187c2ce8d5017415, and its usage was also not removed from the DB Migration script. Later on, in dcd8b32#diff-7b8ac6c93b8d79968c8d4d8237621404L587, theMediaclass usage namespace was changed fromMagento\Catalog\Model\Product\Attribute\BackendtoMagento\Catalog\Setupwithout anyone noticing the class never existed at that point.Issue 3 - AllowedCountriesFactory
The usage of class
AllowedCountriesFactorywas introduced in 6368e1eda02#diff-eceb8134fbc63b4fd17610ab3a3c95cc and later reverted in 8689dd7b0fa#diff-eceb8134fbc63b4fd17610ab3a3c95cc, but someone forgot to revert the changes to the docblock of the$allowedCountriesvariable.(Btw: I've checked if it makes sense to use a Factory over here, and it doesn't make sense).
Fixed Issues (if relevant)
Manual testing scenarios (*)
composer installcomposer require --dev phpstan/phpstanbin/magento setup:di:compilecomposer dump-autoloadvendor/bin/phpstan analyze -l 0 app/code/Magento/*/Setup/To test if DB migrations still work fine, run
bin/magento setup:install ... --cleanup-databasecommand before and after these changes, the following end result should remain the same:media_gallery(ID: 90) has thebackend_modelset toNULLin the tableeav_attributeimage(ID: 87) has thefrontend_input_rendererset toNULLin the tablecatalog_eav_attributeContribution checklist (*)