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

CommentFileSizeAuthor
#3 types_documented-forms_system-d8.patch1.37 KBiuana

Comments

fgm created an issue. See original summary.

iuana’s picture

Assigned: Unassigned » iuana
iuana’s picture

Assigned: iuana » Unassigned
Status: Active » Needs review
StatusFileSize
new1.37 KB

I've added the types documented for functions.

wengerk’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @iuana for your contribution ! Seems good to me.

fgm’s picture

LGTM too.

mradcliffe’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs issue summary update

I 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.

tashaharrison80’s picture

Confirm that all of the functions affected have been updated. Updated the summary.

tashaharrison80’s picture

Issue summary: View changes
tashaharrison80’s picture

Issue summary: View changes
tashaharrison80’s picture

Status: Needs review » Reviewed & tested by the community
mradcliffe’s picture

Thanks, @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.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

There 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!

  • alexpott committed 00d864d on 8.7.x
    Issue #2999387 by iuana, tashaharrison80, fgm, mradcliffe: Types...

  • alexpott committed a25282b on 8.6.x
    Issue #2999387 by iuana, tashaharrison80, fgm, mradcliffe: Types...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.