Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.In core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php, testCheckNamedRouteWithUpcastedValues() and testCheckNamedRouteWithDefaultValue() a variable is initialized to an empty array before adding items to the array.
On any PHP versions, the first line of the following code is not necessary.
$map = [];
$map[] = [
'test_route_1',
[],
'/test-route-1',
];
$map[] = [
'test_route_2',
[],
'/test-route-2',
];
There is no need to initialize the variable to an empty array and then adding items to the array.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | interdiff-2-3.txt | 559 bytes | Hardik_Patel_12 |
| #3 | 3156040-3.patch | 1015 bytes | Hardik_Patel_12 |
Comments
Comment #2
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 for Drupal India Association commentedKindly review a patch.
Comment #3
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 for Drupal India Association commentedComment #4
siddhant.bhosale CreditAttribution: siddhant.bhosale as a volunteer and at QED42 commentedComment #5
siddhant.bhosale CreditAttribution: siddhant.bhosale as a volunteer and at QED42 commentedHi have tested the patch and the test cases run as expected. The output of the test run is below:
Looks good to be merged.
Comment #6
alexpottThank you for your work on cleaning up Drupal core's code style!
In order to fix core coding standards in a maintainable way, all our coding standards issues should be done on a per-rule basis across all of core, rather than fixing standards in individual modules or files. We should also separate fixes where we need to write new documentation from fixes where we need to correct existing standards. This all should be done as part of #2571965: [meta] Fix PHP coding standards in core. A good place to for unused variables is #3106216: Remove unused variables from core.
For background information on why we usually will not commit coding standards fixes that aren't scoped in that way, see the core issue scope guidelines, especially the note about coding standards cleanups. That document also includes numerous suggestions for scoping issues including documentation coding standards cleanups.
Contributing to the overall plan above will help ensure that your fixes for core's coding standards remain in core the long term.
Comment #7
alexpottIn discussion with xim, catch and larowlan, my earlier comment is incorrect. We should handle each unused variable on its own merit and do the work to work out why it is not used.
This can point to broken code or incomplete testing see #3157369: Use unused variable $filters from DateTimeSchemaTest for example. A useful tool for this is
git log -S “SOME TEXT”which will search git commits for matching text to find out when the variable might have become unused. Without doing the work to show why the variable is unused the patch will not be committed. Alsogit blamecan be useful as well.Comment #8
apadernoComment #9
apadernoI edit the IS, since there aren't unused variables, but a variable that is first initialized to an empty array before adding items to the array.
Comment #10
apadernoComment #11
apadernoComment #12
paulocsI spent a little time to find when the variables was inserted and they were committed in the issue #2181293: AccessManager::checkNamedRoute() is not passing all route defaults (or building a complete route request).
It is no problem to remove them and it would be nice to clean the code.
Patch #3 looks good to me, so I'll set to RTBC.
Cheers, Paulo.
Comment #13
alexpottCommitted and pushed 63c85acafa to 9.1.x and 4d4753ff35 to 9.0.x and 103a1dc599 to 8.9.x. Thanks!
As this is a test backported to 8.9.x to keep the tests aligned.