A method declaration is incompatible with the parent class's method. Parent class method has 2 arguments, declared method has 1. A fatal error is generated.
@ds/src/Plugin/views/Entity/Render/TranslationLanguageRenderer.php:61
public function getLangcode(ResultRow $row) {
return isset($row->{$this->langcodeAlias}) ? $row->{$this->langcodeAlias} : $this->languageManager->getDefaultLanguage()->getId();
}
is incompatible with the parent class method:
@ds/src/Plugin/views/Entity/Render/DefaultLanguageRenderer.php:21
public function getLangcode(ResultRow $row, $relationship = NULL) {
return $row->_entity->getUntranslated()->language()->getId();
}
Proposing patch that declares the method with 2 arguments.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | ds-method-declaration-incompatible-2779243-17.patch | 746 bytes | peacog |
| #4 | declaration_parent_compatibility_2.patch | 1.79 KB | efpapado |
Comments
Comment #2
aspilicious commentedComment #3
szeidlerThis issue is a result of an early adopted patch from the core views issue queue @ https://www.drupal.org/node/2457999#comment-11101825
I guess we can postpone the issue for DS here, keeping an eye for the final decision in the related views issue.
But good to have this patch here as a backreference for anyone already applied the views patch and for possible future usage.
Comment #4
efpapado commentedHmm if I'm not wrong the mother class, whose method has the 2 arguments, is also a DS class. DS has already adopted the new views signature on its dev version:
@ds/src/Plugin/views/Entity/Render/DefaultLanguageRenderer.php:21
already declares:
which, as you said, (early) adopts the core views' patch. So this patch is "internal" for DS.
Actually the proposed patch is incomplete, as there are 2 more classes into DS that extend the
\Drupal\ds\Plugin\views\Entity\Render\DefaultLanguageRenderer
and declare the method with only 1 argument:
\Drupal\ds\Plugin\views\Entity\Render\TranslationLanguageRenderer::getLangcode (proposed patch)
\Drupal\ds\Plugin\views\Entity\Render\CurrentLanguageRenderer::getLangcode
\Drupal\ds\Plugin\views\Entity\Render\ConfigurableLanguageRenderer::getLangcode
Attaching a new one.
Comment #5
szeidlerYou're right, it's not dependent on the views patch. I can confirm that the patch applies successfully and fixes the Fatal error I got, too.
Comment #6
aspilicious commentedWe need a test for this.
Someone up for a challenge?
There is a already a test for views but I need some more, it needs an export of each "language type".
It should fatal at the moment.
Comment #7
swentel commentedClosing in favor of #2779243: Method declaration incompatibility - but in the end we just need to wait on core.
Comment #8
replicaobscuraI'm a bit confused. Multiple people confirmed this doesn't need to wait on core, the patch only resolves an issue internal to the ds module. This issue still exists in the ds module today, in 3.x as well.
I don't see a reason this shouldn't be fixed in ds right away--there's no external dependency that I can tell.
Additionally, the linked issue you mentioned you are closing this in favor of is in fact this same issue. Is there really another issue where this is fixed?
Finally, this patch still applies cleanly, including on 3.x.
Comment #9
efpapado commentedThe comment #7 is closing setting the issue as "duplicate" and the reference issue is itself. Maybe swentel was confused or pasted the wrong url?
Setting it as needs review again, and I also agree with bmcclure that it doesn't have to do with core at all.
Comment #10
efpapado commentedComment #11
swentel commentedHmm, you are absolutely right. I have no idea anymore to which issue I was pointing so I will leave this open for sure, apologies! :)
Comment #12
replicaobscuraCool, thank you!
Also, since this patch fixes it for 3.x as well, should the target version be changed, or is there some other way this could be applied easily to both branches?
I've tested this successfully on 3.x, but as I don't use 2.x I didn't want to mark RTBC yet.
Comment #13
replicaobscuraOne more piece of information about this issue just so it's recorded here: This issue did not cause errors in my Drupal installation when I was running on PHP 7.0, but it started causing serious errors in my Drupal installation as soon as I upgraded to PHP 7.2 until I applied this patch.
Comment #14
peacog commentedI've just encountered this after upgrading to PHP 7.2. The fatal PHP error results in a white screen of death. This patch fixes it (thanks!)
With the release of Dupal 8.5, PHP 7.2 is now the recommended PHP version so I'm changing this to critical and RTBC.
Comment #15
swentel commentedI know I might be a pain in the ass, but wouldn't it make more sense by removing $relationship = NULL in DefaultLanguageRenderer ?
I don't even understand why that param is there to be fair, but abstract method in EntityTranslationRendererBase doesn't have it either and it's not even used, so I think it's safe to remove it no ?
Comment #16
peacog commented@swentel Yes I think you're right. $relationship isn't used at all. I think it should be removed
Comment #17
peacog commentedHere's a patch that removes the unused $relationship parameter from getLangcode
Comment #18
peacog commentedTests are failing but not because of this patch. There's an issue dealing with failing tests here #2947362: Running phpunit test results in FieldGroupTestTrait not found error
Comment #19
bryandenijsThanks for the patch! Works for me.
Comment #21
swentel commentedcommitted and pushed, thanks!
Comment #23
pieterdcAlthough it was committed before version 3.1 was packaged, it's not in 3.1
But it is in 3.x