Still on Drupal 7? Security support for Drupal 7 ended on 5 January 2025. Please visit our Drupal 7 End of Life resources page to review all of your options.Problem/Motivation
This is a child issue of #3404246: [META] Fix strict type errors detected by phpstan
The scope includes any @param docs identified by the checkFunctionArgumentTypes phpstan parameter. This may include changing the array type from concrete class to an interface, or an incorrect scalar type such as string to string[], and other similar fixes.
This does not include any actual parameter typing, only the @param docs.
Steps to reproduce
See parent
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3463440
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:
- 3463440-fix-array-param
changes, plain diff MR !8898
Comments
Comment #3
mstrelan CreditAttribution: mstrelan at PreviousNext commentedAnnotated the MR to make it easy to understand why each of the param docs needed changing.
Comment #4
mstrelan CreditAttribution: mstrelan at PreviousNext commentedComment #5
smustgrave CreditAttribution: smustgrave at Mobomo commentedMuch appreciate @mstrelan for doing the review also :)
I agree the type updates are correct in the comments. Now if the functions really shouldn't accept these types believe those can all be follow ups as your findings show code uses them in a certain way.
Comment #8
nod_Committed e07e27a and pushed to 11.x. Thanks!
Comment #9
longwaveIMHO as a docs fix this can be backported down, to help make future cherry-picks easier.
Comment #11
nod_actually one change doesn't conform to our coding standardspoke too soon, I'm not comfortable enough with this to make the call. reverting for nowComment #13
nod_Comment #14
nod_Comment #15
nod_sorry for the noise, last update
Comment #16
mstrelan CreditAttribution: mstrelan at PreviousNext commentedCurious which change was non-conformant? I'm guessing maybe
array<string, string|false>. If that's the case, we need to change our coding standards, or rather, we shouldn't be the ones dictating how to write @param docs, we should rely on what phpcs and phpstan accepts.Comment #17
quietone CreditAttribution: quietone at PreviousNext commentedSetting to NW for another look at one comment.
Comment #18
mstrelan CreditAttribution: mstrelan at PreviousNext commentedOpened #3468236: Adopt phpstan phpdoc types as canonical reference of allowed types for follow up. Left some responses on the MR. Personally I feel it should go in as-is and anything else should be a follow-up.
Comment #19
smustgrave CreditAttribution: smustgrave at Mobomo commentedIf I'm understanding correct there is no official coding standard around the one change in question. So would agree that probably makes sense to go in as is to get some net improvement. And if the rule gets adopted can update, would assume there would then be a whole other set of files that will need to be updated. Just my 2cents.
Comment #25
nod_Discussed on slack, +1 from longwave for commit. Follow-up for coding standard is created.
Committed and pushed fa418559a4 to 11.x and 948c93276e to 11.0.x and 224c7ea5f0 to 10.4.x and c7a2f29450 to 10.3.x. Thanks!