Problem/Motivation

Language negotiation methods (plugins and entities) have integer weights. In #2195573: Add a dedicated @LanguageNegotiation annotation class we discovered a few places in which they were documented as floats (fixed there) or not documented at all. This issue aims to fix the remaining type errors.

Proposed resolution

Correct all documentation to say these values are integers.

Remaining tasks

None.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#9 drupal_2494679_9.patch1.94 KBXano
#9 interdiff.txt652 bytesXano
#1 drupal_2494679_1.patch1.94 KBXano
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xano’s picture

Title: Language negotiation method weights are not always correctly documented as being integers » Fix LanguageNegotiatorInterface type hints in docblocks
Status: Active » Needs review
FileSize
1.94 KB

There seemed to be only one occurrence, so let's repurpose this issue to fix all type hint docblocks in LanguageNegotiatorInterface.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

These changes look good to me, thanks!

webchick’s picture

Status: Reviewed & tested by the community » Needs review

Hm. "mixed[]" feels less clear to me than "array." Especially given the definition is "An array of..." Is that the standard? It doesn't seem like it...

$ grep -r '@return array' . | wc -l
    2115
vs.

$ grep -r '@return mixed\[\]' . | wc -l
       7
cilefen’s picture

Issue tags: +Novice
Xano’s picture

mixed[] means "an array of items of any type", whereas array means "an array of figure it out yourself". The former is definitely much more explicit.

jhodgdon’s picture

I kind of agree with #5 but apparently we are not doing that in Core much if at all, so maybe we should make it just array again and make sure that the docs for the param/return actually say what is in the array?

Xano’s picture

mixed[] is allowed by our coding standards, and it's more specific than array. If the rest of core doesn't use it, that's because this format is relatively new (it was introduced sometime during the last 12 months). There is no reason not start using it.

jhodgdon’s picture

IMO mixed[] doesn't really give any more information than just "array" though. All it says is "it's an array of stuff", and you'd still need to see the code or docs to figure out what the "stuff" is, just like if it just said "array". Right?

So can we be more specific on this one? The docs say:

+   * @return mixed[]
    *   An array of language negotiation method definitions keyed by method id.

What is a "language negotiation method definition" -- is it an array or an object or some kind of class?

Xano’s picture

FileSize
652 bytes
1.94 KB

IMO mixed[] doesn't really give any more information than just "array" though. All it says is "it's an array of stuff", and you'd still need to see the code or docs to figure out what the "stuff" is, just like if it just said "array". Right?

mixed[] explicitly says you're getting an array of differently typed data. array only says it's an array, but you're going to have to do some research first to find out the array's contents are of different types. We already know the contents are of different types, so let's be nice to developers and tell them what they can expect as specifically as possible, so they won't have to spend time figuring that out themselves.

So can we be more specific on this one? The docs say:

Ah, I admit that in this case I should have used array[] instead, as language negotiator definitions are arrays. I completely missed that in earlier comments, for which my apologies. Once #2195573: Add a dedicated @LanguageNegotiation annotation class gets in, we can add an @see to the annotation to this method.

Status: Needs review » Needs work

The last submitted patch, 9: drupal_2494679_9.patch, failed testing.

Status: Needs work » Needs review

Xano queued 9: drupal_2494679_9.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 9: drupal_2494679_9.patch, failed testing.

Status: Needs work » Needs review

Xano queued 9: drupal_2494679_9.patch for re-testing.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

This all looks reasonable to me (again). Thanks!

alexpott’s picture

Docs only changes are permitted in beta. Committed 2466bc0 and pushed to 8.0.x. Thanks!

  • alexpott committed 2466bc0 on 8.0.x
    Issue #2494679 by Xano: Fix LanguageNegotiatorInterface type hints in...
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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