Basing on Coding standards / Object-oriented code / Naming conventions, class properties should use lowerCamel naming. The LazyPluginCollection uses a $instanceIDs that doesn't follow those naming conventions.
Running git grep --fixed-strings --line-number 'instanceIDs' gives the following output.
core/lib/Drupal/Component/Plugin/LazyPluginCollection.php:24: protected $instanceIDs = [];
core/lib/Drupal/Component/Plugin/LazyPluginCollection.php:69: return isset($this->pluginInstances[$instance_id]) || isset($this->instanceIDs[$instance_id]);
core/lib/Drupal/Component/Plugin/LazyPluginCollection.php:119: if (!isset($this->instanceIDs[$id])) {
core/lib/Drupal/Component/Plugin/LazyPluginCollection.php:120: $this->instanceIDs[$id] = $id;
core/lib/Drupal/Component/Plugin/LazyPluginCollection.php:131: return $this->instanceIDs;
core/lib/Drupal/Component/Plugin/LazyPluginCollection.php:141: unset($this->instanceIDs[$instance_id]);
core/lib/Drupal/Component/Plugin/LazyPluginCollection.php:157: return count($this->instanceIDs);
core/lib/Drupal/Core/Plugin/DefaultLazyPluginCollection.php:67: $this->instanceIDs = array_combine($instance_ids, $instance_ids);
core/lib/Drupal/Core/Plugin/DefaultLazyPluginCollection.php:69: $this->originalOrder = $this->instanceIDs;
core/lib/Drupal/Core/Plugin/DefaultLazyPluginCollection.php:90: uasort($this->instanceIDs, [$this, 'sortHelper']);
core/lib/Drupal/Core/Plugin/DefaultLazyPluginCollection.php:109: $current_order = $this->instanceIDs;
core/lib/Drupal/Core/Plugin/DefaultLazyPluginCollection.php:112: $this->instanceIDs = $this->originalOrder + $current_order;
core/lib/Drupal/Core/Plugin/DefaultLazyPluginCollection.php:123: $this->instanceIDs = $current_order;
core/lib/Drupal/Core/Plugin/DefaultSingleLazyPluginCollection.php:96: $this->instanceIDs = [];
core/modules/system/tests/modules/plugin_test/src/Plugin/TestLazyPluginCollection.php:30: $this->instanceIDs = array_combine($instance_ids, $instance_ids);
In all those cases, the $instanceIDs property should be renamed $instanceIds. BC is not necessary, since PHP is not case sensitive and the property is protected.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | rename-instanceIDs-3115987-2.patch | 5.3 KB | jungle |
Comments
Comment #2
jungleComment #3
avpadernoComment #4
naveenvalechaThanks @jungle! RTBC+1
Open Question: Is there any issue that exists to address this convention in the whole of the plugin system?
Comment #5
avpadernoActually, running
git grep --fixed-strings --line-number 'instanceIDs'gives me the following output.Since those are all plugin-related classes, that should be fixed in the same patch.
I changed the title to make the issue more generic.
Comment #6
avpadernoComment #7
jungleHi, @kiamlaluno. All you listed are in the following 4 files, which have been fixed by the patch already. I'd set it back to needs review.
Comment #8
avpadernoI apologize: Only the issue summary and the title needed to be updated.
Comment #9
jungleApologize? Closed(won't fix) @kiamlaluno, Easy easy :p Thank you for helping/reviewing/commenting on this issue and other issues.
@naveenvalecha, very glad to meet here, and thanks!
Comment #10
quietone commentedPatch applies cleanly to 8.9.x. After applying the patch I ran `git grep --fixed-strings --line-number 'instanceIDs'` from the IS and nothing was returned. I reviewed the patch and it only changes the instanceIDs string as stated in the IS.
Comment #14
catchCommitted/pushed to 9.1.x/8.9.x/8.8.x, thanks!