Preference on removed class is not marked as problem

[norgeindian]

I just had a strange issue and wondered if this might be a general thing. We have a preference on a specific observer class of a third party module. Between two versions, this observer class has been removed from the third party module. I would have expected to get a warning regarding that, but the upgrade helper did not notify me at all. I just saw the issue, after running setup:di:compile afterwords. Might it be, that it just happened because it is an observer we override, or might this be a general thing?

[convenient]

Hey @norgeindian

Can you please run with --filter "vendor/file/path/that/was/removed.php" and -vvv

This will scan only for the affected file you're filtering for, in verbose mode so we can see whats going on under the hood

[norgeindian]

@convenient , unfortunately not much. The output is the following:

Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in /var/www/html/ampersand-magento2-upgrade-patch-helper/src/Ampersand/PatchHelper/Helper/Magento2Instance.php on line 461
Magento has been instantiated
Patch file has been parsed
+-------+------+------+----------+
| Level | Type | File | To Check |
+-------+------+------+----------+
WARN count: 0
INFO count: 0
For docs on each check see https://github.com/AmpersandHQ/ampersand-magento2-upgrade-patch-helper/blob/master/docs/CHECKS_AVAILABLE.md
You should review the above 0 items alongside /var/www/html/vendor_files_to_check.patch

I would say, the deprecated notice should not be the issue. In vendor.patch I see some interesting output:

Only in vendor/xxx/magento2/Observer: SalesOrderShipmentSaveBefore
Only in vendor_orig/xxx/magento2/Observer: SalesOrderShipmentSaveBefore.php

What the module actually did, was to move the Observer from the directory Observer to the subdirectory Observer\SalesOrderShipmentSaveBefore and rename it to CreateShipment.php

So the first entry says, that the folder is new, and the second says, that the file is missing. But when I run the command without any filter, neither SalesOrderShipmentSaveBefore nor CreateShipment is mentioned.

Do you already see a reason why this might happen? What further information could I give you or what could I try / check?

[convenient]

Hey @norgeindian did you generate your diff by running diff -ur -N ?

I don't think I'd expect to see any Only in

[norgeindian]

@convenient , thanks a lot for your fast reply. Indeed, we forgot the -N, as we use our own documentation and obviously did not update it. Now, it breaks at this point:

Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in /var/www/html/ampersand-magento2-upgrade-patch-helper/src/Ampersand/PatchHelper/Helper/Magento2Instance.php on line 461

Fatal error: Uncaught Error: Class "xxx\Payment\Observer\SalesOrderShipmentSaveBefore" not found in /var/www/html/src/xxx/yyy/Observer/SalesOrderShipmentSaveBeforeObserver.php:14
Stack trace:
#0 /var/www/html/ampersand-magento2-upgrade-patch-helper/vendor/composer/ClassLoader.php(582): include()
#1 /var/www/html/ampersand-magento2-upgrade-patch-helper/vendor/composer/ClassLoader.php(433): Composer\Autoload\{closure}('/var/www/html/v...')
#2 [internal function]: Composer\Autoload\ClassLoader->loadClass('XXX\\YYY...')
#3 /var/www/html/ampersand-magento2-upgrade-patch-helper/src/Ampersand/PatchHelper/Checks/ClassPreferencePhp.php(140): ReflectionClass->__construct('xxx\\yyy...')
#4 /var/www/html/ampersand-magento2-upgrade-patch-helper/src/Ampersand/PatchHelper/Checks/ClassPreferencePhp.php(97): Ampersand\PatchHelper\Checks\ClassPreferencePhp->getFilenameFromPhpClass('xxxx\\yyyy...')
#5 /var/www/html/ampersand-magento2-upgrade-patch-helper/src/Ampersand/PatchHelper/Checks/ClassPreferencePhp.php(64): Ampersand\PatchHelper\Checks\ClassPreferencePhp->isThirdPartyPreference('yyy\\Payment\\...', 'xxx\\yyyy...', Array)
#6 /var/www/html/ampersand-magento2-upgrade-patch-helper/src/Ampersand/PatchHelper/Helper/PatchOverrideValidator.php(225): Ampersand\PatchHelper\Checks\ClassPreferencePhp->check()
#7 /var/www/html/ampersand-magento2-upgrade-patch-helper/src/Ampersand/PatchHelper/Command/AnalyseCommand.php(177): Ampersand\PatchHelper\Helper\PatchOverrideValidator->validate()
#8 /var/www/html/ampersand-magento2-upgrade-patch-helper/vendor/symfony/console/Command/Command.php(255): Ampersand\PatchHelper\Command\AnalyseCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 /var/www/html/ampersand-magento2-upgrade-patch-helper/vendor/symfony/console/Application.php(1009): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#10 /var/www/html/ampersand-magento2-upgrade-patch-helper/vendor/symfony/console/Application.php(273): Symfony\Component\Console\Application->doRunCommand(Object(Ampersand\PatchHelper\Command\AnalyseCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /var/www/html/ampersand-magento2-upgrade-patch-helper/vendor/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /var/www/html/ampersand-magento2-upgrade-patch-helper/bin/patch-helper.php(10): Symfony\Component\Console\Application->run()
#13 {main}
  thrown in /var/www/html/src/xxxx/yyyy/Observer/SalesOrderShipmentSaveBeforeObserver.php on line 14

Would it be possible to just add it to the list instead of failing completely? Then one could handle all issues at once after the run. This way, I would need to fix that first, before I can take a look at other issues. But in any case, it is found now, thanks a lot for the hint.

[convenient]

Hey @norgeindian

Your preference SalesOrderShipmentSaveBefore does it extend the removed class? Or completely replace it?

I think the code needs updated to handle two scenarios

• The preference does not extend anything, then the class it is making a preference of is removed
• The preference extends the class it is making a preference of, then the class it is making a preference of is removed

Additionally I think the following may have helped in your initial scenario, to verify the patchfile is generated appropriately? https://github.com/AmpersandHQ/ampersand-magento2-upgrade-patch-helper/pull/92 (Havent tested it yet)

[norgeindian]

@convenient, it's actually the second case for us. So we have a preference AND extend the class.