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

Command icon 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:

Comments

berdir created an issue. See original summary.

berdir’s picture

Issue summary: View changes
ishani patel’s picture

Assigned: Unassigned » ishani patel

ishani patel’s picture

Assigned: ishani patel » Unassigned
Status: Active » Needs review

I've raised MR.
Kindly review it.

Thank you!

berdir’s picture

Status: Needs review » Needs work

The 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.

sivaji_ganesh_jojodae made their first commit to this issue’s fork.

sivaji_ganesh_jojodae’s picture

Status: Needs work » Needs review

Addressed the comment in MR and #6.

berdir’s picture

Status: Needs review » Needs work

One more review. Not so novice after all, nothing is ever easy.

sivaji_ganesh_jojodae’s picture

Status: Needs work » Needs review

I've added the type hinting mentioned in MR comment.

berdir’s picture

Title: Add return types to CacheTagsChecksumTrait » Add return types and strict types to CacheTagsChecksumTrait
Status: Needs review » Reviewed & tested by the community

Looks 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)

catch’s picture

Status: Reviewed & tested by the community » Fixed

This 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!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 22bb0108 on main
    task: #3584766 Add return types and strict types to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.