Problem/Motivation
Modules implementing OutboundPathProcessorInterface will trigger a phpstan warning re: "parameter $options with no value type specified in iterable type array."
Steps to reproduce
Run phpstan analyze at level 6 or higher, on a module implementing OutboundPathProcessorInterface.
Proposed resolution
Let's specify the $options array shape as best we can. Note there are some additional option keys that may be present - I didn't (yet) try to document these exhaustively.
Remaining tasks
This may need to be made more generic, as it looks like arbitrary options can be passed in? In that case, the $options array could be specified as mixed[]
Other options include ajax, set_active_class, path_processing, alias, entity_type, entity, weight...
User interface changes
API changes
No API change, this just adds a more specific type definition to the phpdoc.
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 3443818-nr-bot.txt | 851 bytes | needs-review-queue-bot |
Issue fork drupal-3443818
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
Comment #3
mfbComment #4
mfbComment #5
smustgrave commentedSo this seems to make it harder to read. Is it worth that hit?
Comment #6
mfb@smustgrave well, as I mentioned in remaining tasks, the $options might in fact be designed to be open-ended? In which case this could just be documented as
mixed[]. (If so, should be needs work for that)Comment #7
smustgrave commentedLets do that, adding mixed[] and maybe a comment about how this open-ended?
Comment #8
smustgrave commented@mfb you can address it as the reported but tagging as novice in case a new user from portland is around.
Comment #9
mfbCan you/someone confirm that options are intended to be open-ended (e.g. for contrib module to add/use arbitrary options)? If so, yes, that would be good to document
Comment #10
rodrigoaguileraThe Drupal Contribution Mentoring team is triaging issues for DrupalCon Barcelona 2024, and we are reserving this issue for Mentored Contribution during the event.
After September 27, 2024, this issue returns to being open to all. Thanks!
I feel that array is not supposed to be extended. At least I am not aware of any module that does it.
I think the current approach in the MR is right. Leaving it for others to review
Comment #11
mfb@rodrigoaguilera thanks for looking into it! Did you notice that core itself uses some additional undocumented option keys? See the remaining tasks section of the issue summary.
Comment #12
rodrigoaguileraOh, thanks for pointing that out. Then yes, they are probably a bucket for any arbitrary property.
The task of figuring out what was the original intention I think it might be difficult so I don't think is novice anymore.
Comment #13
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #14
needs-review-queue-bot commentedFalse positive
Comment #15
smustgrave commentedSo I still kinda maintain this looks ugly and kind already mentioned in the comments. If we ever get around to adding typehints will we have to use this long string now?
Comment #16
mfb@smustgrave I tried to make it a multiline array shape definition, which works fine with phpstan, but apparently Drupal\Sniffs\Commenting\FunctionCommentSniff (part of the drupal/coder project) cannot parse such phpdoc properly, see #3366735: Array shapes in multiple lines are not supported., so I guess we should postpone this