Cache result of Groups::groups()

[staabm]

the function call is showing up in blackfire profiles of blackfire run php ./phpunit --testsuite unit

---

after this PR the calls are gone

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.09%. Comparing base (59ad5c9) to head (b121f81).

❗ Current head b121f81 differs from pull request most recent head e6d645a. Consider uploading reports for the commit e6d645a to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##               10.5    #5747   +/-   ##
=========================================
  Coverage     90.08%   90.09%           
- Complexity     6437     6438    +1     
=========================================
  Files           680      680           
  Lines         19535    19538    +3     
=========================================
+ Hits          17599    17603    +4     
+ Misses         1936     1935    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[staabm]

I don't know what todo with these psalm errors. they don't make sense to me.

It looks like this method never returned a list<string>, because array_unique might return a array with holes (which contradics the list type)

[mfn]

You need to wrap it with array_values(array_unique(…)), this gets rid of potential holes and is usually accepted by psalm & friends.

[staabm]

You need to wrap it with array_values(array_unique(…)), this gets rid of potential holes and is usually accepted by psalm & friends.

yes, but that would change the logic of the function. I am not sure the types are wrong or the implementation :).
(and I don't want to add even more function calls. I try to get rid of everything which is too much)

[staabm]

I had a closer look at the logic involved and the callers. it seems the callers don't need a list and fixing the types also fixes some baseline errors. I think thats the way to go.