Problem/Motivation
Four methods on ConfirmFormInterface are documented as returning a string:
getQuestion();getDescription();getConfirmText();getCancelText();
However, in practice, their implementations are typically like:
return $this->t('something');
meaning they really return a TranslatableMarkup, not a string.
Proposed resolution
Document the functions as returning \Drupal\Core\StringTranslation\TranslatableMarkup
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | types_documented-forms_system-d8.patch | 1.37 KB | iuana |
Comments
Comment #2
iuana commentedComment #3
iuana commentedI've added the types documented for functions.
Comment #4
wengerkThanks @iuana for your contribution ! Seems good to me.
Comment #5
fgmLGTM too.
Comment #6
mradcliffeI think it would be nice to confirm that there aren’t any other methods that are missing the documentation and update issue summary with that list of methods that apply. This will clarify the issue.
I'm going to move to Needs Review at the moment to update the issue summary.
Comment #7
tashaharrison80 commentedConfirm that all of the functions affected have been updated. Updated the summary.
Comment #8
tashaharrison80 commentedComment #9
tashaharrison80 commentedComment #10
tashaharrison80 commentedComment #11
mradcliffeThanks, @tashaharrison80. One thing that helps as well is to describe what you did exactly in your comment when making issue summary changes. I think this is good. We took a look at the patch and confirmed that those 4 methods are the only ones that required the change. The other methods have correct return values.
Comment #12
alexpottThere must be hundreds of these in core. The nice thing about documenting this is that then IDEs will tell people to return TranslatableMarkup and not strings - even though both will work. And this will improve the translatability of custom and contrib code. Nice.
Committed and pushed 00d864da07 to 8.7.x and a25282be4c to 8.6.x. Thanks!