Fix LanguageNegotiationContentEntity::getLangcode() errors thrown

1. +++ b/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationContentEntity.php
   @@ -97,6 +97,10 @@ public static function create(ContainerInterface $container, array $configuratio
      public function getLangcode(Request $request = NULL) {
   +    if ($request === NULL || $this->languageManager === NULL) {
   +      return NULL;
   +    }
   

   Code above is from the patch

   
      * @return string
      *   A valid language code or FALSE if the negotiation was unsuccessful.
      */
     public function getLangcode(Request $request = NULL);
   
   

   a) Based on the comments of \Drupal\language\LanguageNegotiationMethodInterface::getLangcode() above, returning NULL is inapplicable. And the type hinting of the interface should be updated to string|false or string|bool, not sure which one is applicable, is false a type? but found usages in core.

   b) Or update the comment to say string or NULL, and change @return string to @return string|null

2. And the whole method might need to be rewritten

       if ($request === NULL || $this->languageManager === NULL) {
         return NULL;
       }
   
       $langcode = $request->query->get(static::QUERY_PARAMETER);
   
       $language_enabled = array_key_exists($langcode, $this->languageManager->getLanguages());
       return $language_enabled ? $langcode : NULL; 
   

   a) $langcode = $request->query->get(static::QUERY_PARAMETER); is possible to return NULL.
   b) the const name QUERY_PARAMETER is too generic, it's not obvious relevant to langcode or language_content_entity -- the real value. But it's out of the scope here.

Is this still an issue?

+++ b/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationContentEntity.php
@@ -97,6 +97,10 @@ public static function create(ContainerInterface $container, array $configuratio
+    if ($request === NULL || $this->languageManager === NULL) {
+      return NULL;

According to the API, this should return a string or FALSE, not NULL - so I think we should update this.

As this is a bug, we need a failing test demonstrating the issue. And whilst \Drupal\Tests\language\Functional\LanguageNegotiationContentEntityTest might seem a logical place for this, I think we can test it in a Unit test to reproduce the reported issue here.

Rerolled the patch #4 in Drupal 9.5.x and changes comment #14.

Couple of minor comments on the MR

This looks good to me, thanks @stefanos.petrakis 🙌

I don't think we should be changing the return value here. I think we should open a separate issue to discuss the fact the interface docs and the implementations (at least all the ones I've checked) do different things. Personally I think we should always err on the side of changing the docs rather code.

Thanks @stefanos.petrakis for creating the separate issue and following this one through

This looks good to me

Committed and pushed 5738144ad9 to 10.1.x and 681c999fea to 10.0.x. Thanks!

Not backporting to 9.5.x because the test needs changing because it has property typehints and we still support PHP 7.3 there and this is a minor bug.