Problem/Motivation

This issue is a follow-up to #3549663-6: Automated tests: Add trait AdminToolbarHelperTestTrait where the reported PHPSTAN validation job errors were ignored, as a "quick" fix solution so the other merge request could have the jobs and tests passing and get merged, but it introduced errors for previous major core version (10.5.x):

------ ----------------------------------------------------------------------- 
  Line   admin_toolbar.module                                                   
 ------ ----------------------------------------------------------------------- 
  100    No error with identifier assign.propertyType is reported on line 100.  
  102    No error with identifier assign.propertyType is reported on line 102.  
 ------ ----------------------------------------------------------------------- 
 [ERROR] Found 2 errors

So we're probably going to have to look into this a bit closer in a separate ticket 😖
Try to find a better way to conditionally support different PHPSTAN versions 😅

This particular piece of code would probably need to be refactored so it does not cause any PHPSTAN issues in the first place.

Steps to reproduce

Run build pipelines and see for example:
https://git.drupalcode.org/project/admin_toolbar/-/jobs/7220273#L47

Proposed resolution

Refactor the code using function:
\Drupal\Component\Utility\NestedArray::mergeDeep()
see helpful comment in code:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork admin_toolbar-3557282

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:

About issue forks

Comments

dydave created an issue. See original summary.

dydave opened merge request !189

  • dydave committed a5f07bb1 on 3.x 
    Issue #3557282 by dydave: Fixed phpstan errors by using core utility '...
dydave’s picture

dydave commented
Status: Active » Fixed

Quick follow-up on this issue:

Since the changes required to fix the development pipelines were really small and all the tests and jobs were still passing 🟢, I went ahead and merged the changes directly above at #3, so other merge requests could be rebased and tested properly again 👌

Note: this change is effectively tested in several places in the module, where the class 'toolbar-icon' is checked, for example here:
https://git.drupalcode.org/project/admin_toolbar/-/blob/3.x/tests/src/Fu...

Marking issue Fixed, for now.

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.

Status: Fixed » Closed (fixed)

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