Problem/Motivation
Running a test with redis is enabled on the parent site causes two deprecation messages:
1) /var/www/html/vendor/symfony/error-handler/DebugClassLoader.php:348
Method "Drupal\Core\Cache\CacheTagsChecksumInterface::getCurrentChecksum()" might add "string" as a native return type declaration in the future. Do the same in implementation "Drupal\redis\Cache\RedisCacheTagsChecksum" now to avoid errors or add an explicit @return annotation to suppress this message.
Triggered by:
* Drupal\Tests\menu_ui\Functional\MenuUiContentModerationTest::testMenuUiWithPendingRevisions
/var/www/html/core/modules/menu_ui/tests/src/Functional/MenuUiContentModerationTest.php:62
2) /var/www/html/vendor/symfony/error-handler/DebugClassLoader.php:348
Method "Drupal\Core\Cache\CacheTagsChecksumInterface::isValid()" might add "bool" as a native return type declaration in the future. Do the same in implementation "Drupal\redis\Cache\RedisCacheTagsChecksum" now to avoid errors or add an explicit @return annotation to suppress this message.
But redis doesn't even implement those methods, they're from a core trait, but that can't be excluded, as traits are essentially just a compile-time copy-paste thing.
Steps to reproduce
Proposed resolution
Add the return types to \Drupal\Core\Cache\CacheTagsChecksumTrait. According to #3529510: [meta] Add return types to test traits, this has no BC impact, a specific class can have this method without the type:
rootTransactionEndCallback: void
invalidateTags: void
getCurrentChecksum: string
isValid: bool
calculateChecksum: array
reset: void
Do not add return types to the abstract methods, that would be an API change.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3584766
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3584766-add-return-types
changes, plain diff MR !15460
Comments
Comment #2
berdirComment #3
ishani patel commentedComment #5
ishani patel commentedI've raised MR.
Kindly review it.
Thank you!
Comment #6
berdirThe merge request is failing, you nee to remove those entries from the phpstan baseline file. You can also download the updated baseline file from https://git.drupalcode.org/issue/drupal-3584766/-/jobs/9369080/artifacts... I thin and replace that, should result in the same difference.
Comment #8
sivaji_ganesh_jojodae commentedAddressed the comment in MR and #6.
Comment #9
berdirOne more review. Not so novice after all, nothing is ever easy.
Comment #10
sivaji_ganesh_jojodae commentedI've added the type hinting mentioned in MR comment.
Comment #11
berdirLooks good to me. Not sure if the declare strict types is in scope and honestly not even sure what it does in scope of a trait, happy to remove that again if there are any concerns about that.
The argument that it is in scope would be that we now make it explicit how the different types work together (external view: checksum is a string, internal view: checksums are a set of incrementing integers that are summed up)
Comment #13
catchThis looks fine. Since we haven't done this much so far, committing only to main just in case there's a bc issue we're unaware of, should still allow contrib to add the return types against 11.x. Thanks!