There are several typehints missing in \Drupal\Core\Entity\Element\EntityAutocomplete. Attached patch adds those type hints.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

legolasbo created an issue. See original summary.

cilefen’s picture

Status: Needs review » Needs work
Issue tags: +Documentation
+++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
@@ -243,10 +249,10 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
-   * @return int|null
+   * @return integer|null

"int" was correct here.

legolasbo’s picture

@cilifen

Is that a coding standards thing?

cilefen’s picture

Yes.

legolasbo’s picture

Changed integer back to int as suggested in #2

heykarthikwithu’s picture

Status: Needs work » Needs review

queueing this for testing.

Xano’s picture

Status: Needs review » Needs work
  1. +++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
    @@ -146,6 +147,7 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
    +      /** @var SelectionInterface $handler */
    

    Missing namespace.

  2. +++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
    @@ -164,6 +166,7 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
    +          /** @var SelectionWithAutocreateInterface $handler */
    

    Missing namespace.

  3. +++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
    @@ -211,6 +214,7 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
    +            /** @var \Drupal\Core\Entity\Entity $entity */
    

    \Drupal\Core\Entity\EntityInterface.

    We integrate with interfaces, not classes, unless no interfaces exist.

  4. +++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
    @@ -233,6 +237,8 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
    +   * @param SelectionInterface $handler
    

    Missing namespace.

  5. +++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
    @@ -246,7 +252,7 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
    +  protected static function matchEntityByTitle(SelectionInterface $handler, $input, &$element, FormStateInterface $form_state, $strict) {
    

    $element needs a type hint.

$element in ::valueCallback() needs a type hint as well.

What is #DrupalBoyScout?

legolasbo’s picture

FileSize
3.02 KB
3.18 KB

Thanks for the review Xano,

I have fixed your suggestions and for future patches like these I will keep in mind that we use full namespace paths for type hints.

$element in ::valueCallback() needs a type hint as well.

I will open a follow up for this because the added type hint requires the parent interface and all it's children to be type hinted as well.

What is #DrupalBoyScout?

The boyscout principle states that you always leave your code a little cleaner than you found it. Since Drupal's code base is full of things that can be a lot cleaner I want to initiate a movement to always clean Drupal's code whenever you encounter a possibility to do so. I therefor introduced this tag to keep track of any issues created with the sole purpose of cleaning up Drupal's codebase.

legolasbo’s picture

Xano’s picture

I will open a follow up for this because the added type hint requires the parent interface and all it's children to be type hinted as well.

Duh. Never mind the follow-up even, as that would be a BC break. Thanks for pointing out!

The boyscout principle states that you always leave your code a little cleaner than you found it. Since Drupal's code base is full of things that can be a lot cleaner I want to initiate a movement to always clean Drupal's code whenever you encounter a possibility to do so. I therefor introduced this tag to keep track of any issues created with the sole purpose of cleaning up Drupal's codebase.

I like the initiative, but let's come up with a gender-inclusive name. We're both from a country where scouts don't distinguish between genders ;-)

legolasbo’s picture

Duh. Never mind the follow-up even, as that would be a BC break. Thanks for pointing out!

Too late, already opened + patch added. Also we could argue (in the other issue) that the BC break is limited and/or not a big deal because everyone is (should be) passing an array and every implementation should break if a non-array is passed.

I like the initiative, but let's come up with a gender-inclusive name. We're both from a country where scouts don't distinguish between genders ;-)

How about DrupalCodeScouts, DrupalCleanCoders, DrupalCodeCleanup or something like that?

Back on-topic. RTBC?

Xano’s picture

Status: Needs review » Reviewed & tested by the community

How about DrupalCodeScouts, DrupalCleanCoders, DrupalCodeCleanup or something like that?

There's clean-up as well.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed
+++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
@@ -246,7 +252,7 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
+  protected static function matchEntityByTitle(SelectionInterface $handler, $input, array &$element, FormStateInterface $form_state, $strict) {
     $entities_by_bundle = $handler->getReferenceableEntities($input, '=', 6);

I pondered if this should be allowed in 8.0.x but given the call to getReferenceableEntities this makes a lot of sense.

Committed 5ce93e8 and pushed to 8.0.x and 8.1.x. Thanks!

  • alexpott committed 999e819 on 8.1.x
    Issue #2637304 by legolasbo, Xano: Add missing typehints in...

  • alexpott committed 5ce93e8 on 8.0.x
    Issue #2637304 by legolasbo, Xano: Add missing typehints in...

Status: Fixed » Closed (fixed)

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