Problem/Motivation
PHPstan is giving a warning when using the editor_load function.
Parameter #1 $format_id of function editor_load expects int, string given.
When creating a text-format the machine name can be anything, not just an int.
Steps to reproduce
Run phpstan when using the function editor_load()
Proposed resolution
Change the expected argument to int|string
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3409040.patch | 514 bytes | sidharth_soman |
Issue fork drupal-3409040
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 #2
sidharth_soman commentedI have modified the argument's comment. Apply against 10.0.x branch.
Comment #3
cilefen commentedComment #4
smustgrave commentedWe should see what's calling it with a string and determine if that's incorrect.
Also would have to look at when that typehint was added if there was discussion about int vs string, which should be documented.
Comment #5
wim leersActually, this seems like a very obvious, trivial mistake. Text formats had numerical IDs in Drupal 7. They don't in Drupal >=8.
No further research necessary IMO.
Comment #6
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue.
While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)
Comment #7
wim leersComment #10
keshav patel commentedPatch Rerolled and all checks passed. please review.
Comment #11
smustgrave commentedReroll seems fine
Comment #12
longwaveIs it ever really int any more? Can we set it to string only?
Comment #13
smustgrave commentedWill try and take a deeper look tomorrow
Comment #14
smustgrave commentedSo looking into editor_filter_xss() there's a line
$editor = $format ? editor_load($format->id()) : NULL;and id() can actually return string|int|null so guess int can still be passed.
Comment #15
catchThat looks like a bug in editor_filter_xss(), it should check isNew() before trying to load. Or if not, it should specify string|int|null. But I think we should just type hint on string here.
Comment #17
shalini_jha commentedI Have addressed the feedback, moving this NR. kindly review.
Comment #18
smustgrave commentedDoesn't seem to address the feedback from #15.
Think the comments were missed when converting to an MR.