Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I'm not sure this _is_ a bug in the code, might be a PHP thing, but in ConfigDiffer I noticed the following line (112) is causing me an issue:
<?php
return $source == $target;
?>
I have two config arrays and in one of them there's a nested array like this:
a => a
b => b
c => 0
d => d
As things stand this is being treated as equal to this one
a => a
b => b
c => c
d => d
If I change the "==" operator to "===" things work correctly, and the two arrays are marked as different.
I can't see any ill-effect on any of my other configurations. Is there any reason why the less strict operator was used?
Comment | File | Size | Author |
---|---|---|---|
#2 | config_compare-2863704-2.patch | 420 bytes | alfaguru |
|
Comments
Comment #2
alfaguru CreditAttribution: alfaguru commentedPatch attached.
Comment #3
jhodgdonThanks!
That seems like a reasonable change. Let's set the patch to Needs Review and make sure it doesn't break any of the tests. Then we should also add a test to reproduce the bug you are seeing before committing it to the module.
Can you export your configuration to a .yaml file (both versions) so I can use them to make a test?
Comment #4
jhodgdonhttp://php.net/manual/en/language.operators.comparison.php
So, 'c' == 0 returns TRUE. There you go. I had somehow misplaced this fact in my head.
Anyway, the patch definitely needs to be committed. But, we should also write a more comprehensive unit test for the ConfigDiffer that would verify the normalize/compare is not returning false differences if the incoming arrays are in different orders, and also take care of this test case.
Comment #6
jhodgdonOK! I created a unit test for the ConfigDiffer::same() method. I verified it failed as expected with the test case here, and then applied the patch, and it passed. I'll go ahead and commit this with the test.
Comment #7
nedjoNice catch.
AFAIK best practice in PHP is to always use the stricter identical/not identical comparison operators (
===
,!==
) unless the context specifically calls for the type conversion that comes with the more relaxed equals/not equals. But in Drupal we often default to the equals/not equals forms. The coding standards don't address this.Comment #8
jhodgdonI think you are right that === and !== should be a best practice for PHP, and also correct that Drupal often doesn't, which explains (but doesn't excuse) my bad habits. Anyway, this one instance is fixed now, and our compare for differences in ConfigDiffer should be more accurate!
Comment #9
alfaguru CreditAttribution: alfaguru commentedThanks for responding so quickly to this issue. There are occasional cases where == is the right operator to use, but mostly it's best avoided.