AmpersandHQ / ampersand-magento2-upgrade-patch-helper

Helper script to aid upgrading magento 2 websites by detecting overrides. Now supports third party module detections
GNU Lesser General Public License v3.0
322 stars 39 forks source link

Warning 'Undefined array key "instance"' on plugins that don't define a classname #103

Closed hostep closed 1 year ago

hostep commented 1 year ago

Hi

Saw the following warning block 4 times in the output:

PHP Warning:  Undefined array key "instance" in src/Ampersand/PatchHelper/Checks/ClassPluginPhp.php on line 75

Warning: Undefined array key "instance" in src/Ampersand/PatchHelper/Checks/ClassPluginPhp.php on line 75
PHP Deprecated:  ltrim(): Passing null to parameter #1 ($string) of type string is deprecated in src/Ampersand/PatchHelper/Checks/ClassPluginPhp.php on line 76

Deprecated: ltrim(): Passing null to parameter #1 ($string) of type string is deprecated in src/Ampersand/PatchHelper/Checks/ClassPluginPhp.php on line 76

After outputting the variable $pluginName and searching the codebase for the 4 plugins, I've found them being defined like so in the etc/di.xml file of the Amasty Improved Layered Navigation module (version 2.21.0):

    <type name="Magento\InventoryIndexer\Indexer\Stock\Strategy\Sync">
        <plugin name="configurable_product_full_index" sortOrder="1"/>
        <plugin name="configurable_product_index_list" sortOrder="1"/>
    </type>

    <type name="Magento\InventoryIndexer\Indexer\Stock\Strategy\Sync">
        <plugin name="grouped_product_index_list" sortOrder="2"/>
        <plugin name="grouped_product_index_full" sortOrder="2"/>
    </type>

Notice that these don't contain a type attribute. I'm assuming Amasty defines those like this only to change the sort order of those plugins.

It probably makes sense to add a check like the following before this line I think?

                    if (!isset($pluginConf['instance'])) {
                        continue;
                    }
convenient commented 1 year ago

100% agree @hostep

hostep commented 1 year ago

Just one extra tidbit, we have the following modules disabled in the app/etc/config.php file. These modules contain those 4 original definitions:

This might be part of what is needed to reproduce the problem (not 100% sure though).

Update: indeed, enabling one of those modules, results in 2 fewer warnings being outputted.

convenient commented 1 year ago

That might be the cause of the issue @hostep , possibly if the original config is missing then the merged config won't have this value.

Also I've added a note to https://github.com/AmpersandHQ/ampersand-magento2-upgrade-patch-helper/blob/master/docs/TROUBLESHOOTING.md#debug-a-warning-or-a-notice which may help you in the future

-vvv --php-strict-errors

Should fail loud and fast and clearly on the affected files.

hostep commented 1 year ago

Thanks! 🙂