Problem/Motivation

#3468502: sebastianbergmann/comparator:5.0.2 Introduces (for Drupal) breaking changes was necessary because sebastianbergmann/comparator made changes.

Those changes were very superficial (only type annotations in docblocks that go beyond PHP's native yet simplistic array in actual code), but enabled sebastianbergmann/comparator to continue to work in unit tests.

However, projects that are developed with a higher PHPStan level than core — for example, Canvas is developed using level 8 — run into a remaining failure:

  102    Parameter #1 ...$cache_contexts of static method Drupal\Core\Cache\Cache::mergeContexts() expects list<str  
         ing>, array<string> given.                                                                                  
         🪪  argument.type                                                                                           
         💡  array<string> might not be a list.                                                                      
  102    Parameter #2 ...$cache_contexts of static method Drupal\Core\Cache\Cache::mergeContexts() expects list<str  
         ing>, array<string> given.                                                                                  
         🪪  argument.type                                                                                           
         💡  array<string> might not be a list.       

https://git.drupalcode.org/project/canvas/-/jobs/7375701

because

Steps to reproduce

PHPStan level 8, and something like:

Cache::mergeContexts([], User::load(1));

(or really, anything that implements CacheableDependencyInterface)

Proposed resolution

Adjust docs for \Drupal\Core\Cache\CacheableDependencyInterface::getCacheContexts()

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3559209

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

wim leers created an issue. See original summary.

wim leers’s picture

Status: Active » Needs review
Issue tags: +PHPStan
smustgrave’s picture

Version: 11.3.x-dev » 11.x-dev
Status: Needs review » Needs work

mind updating the MR for 11.x and I can mark it for you

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

ritarshi_chakraborty’s picture

Status: Needs work » Needs review

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @ritarshi_chakraborty but it's usually good to let the previous maintainer have a little time to respond, especially with such a small change.

ritarshi_chakraborty’s picture

Thanks for pointing that out — I honestly thought the previous maintainer might be inactive, so I went ahead with the change. I’ll definitely keep your advice in mind and make sure to give a bit more time for a response next time. Appreciate the guidance! 🙏

quietone’s picture

Title: Follow-up for #3468502 due to sebastianbergmann/comparator changes: cache tags & contexts should use `list<string>` everywhere » cache tags & contexts should use `list<string>` everywhere

Maybe the title can be shorter

smustgrave’s picture

No big deal at all! Just wanted to point out for next time. Thanks!

borisson_’s picture

Title: cache tags & contexts should use `list<string>` everywhere » cache contexts should use `list<string>` everywhere

The title is cache tags and contexts, but the pull request only changes the docs for getCacheContexts. This is because getCacheTags is already the right type.
I updated the title because of that. RTBC++

longwave’s picture

Version: 11.x-dev » 10.6.x-dev
Status: Reviewed & tested by the community » Fixed

Backported down to 10.6.x as a docs fix.

Committed and pushed 52899c7c616 to main and e657859095a to 11.x and 53f356512ad to 11.3.x and a42213654ec to 10.6.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.

  • longwave committed a4221365 on 10.6.x
    docs: #3559209 cache contexts should use `list<string>` everywhere
    
    By:...

  • longwave committed 53f35651 on 11.3.x
    docs: #3559209 cache contexts should use `list<string>` everywhere
    
    By:...

  • longwave committed e6578590 on 11.x
    docs: #3559209 cache contexts should use `list<string>` everywhere
    
    By:...

  • longwave committed 52899c7c on main
    docs: #3559209 cache contexts should use `list<string>` everywhere
    
    By:...

Status: Fixed » Closed (fixed)

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