Fix
i$ignoreErrors[] = [
'message' => '#^Method Drupal\\\\Core\\\\Entity\\\\ContentEntityBase\\:\\:getTranslatedField\\(\\) return type with generic interface Drupal\\\\Core\\\\Field\\\\FieldItemListInterface does not specify its types\\: T$#',
'identifier' => 'missingType.generics',
'count' => 1,
'path' => __DIR__ . '/lib/Drupal/Core/Entity/ContentEntityBase.php',
];
and similar.
grep "FieldItemListInterface does not specify its types" -B1 -A4 core/.phpstan-baseline.php
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !10730
Comments
Comment #2
mondrakeComment #4
mondrakephpstan-drupal has a stub that defines
FieldItemListInterfaceas a generic, so we need to pass the type in the PHPDocs here.See https://github.com/mglaman/phpstan-drupal/blob/main/stubs/Drupal/Core/Fi...
Comment #5
mondrakeComment #6
mondrakeTBH I think it’s nonsense to have core lacking appropriate PHPDoc, for drupal-phpstan to write a stub gapfill, and then that gapfill to force back core implementations’ PHPDocs to be adjusted…
I would suggest to move the definitions in the stub from drupal-php to core as part of this issue.
Comment #7
mglamanphpstan-drupal uses a stub provider, so we can fix here and back port anything to phpstan-drupal and use a version check on Drupal to decide if the phpstan-drupal stub should be loaded or not
Comment #8
mondrakeDone #7 for the Drupal core part (I think).
Comment #9
smustgrave commentedLeft a comment on the MR but leaving in review for someone more familiar
Comment #10
smustgrave commentedThanks for the response @mglaman. Moving to NW for rebase.
If you are another contributor eager to jump in, please allow the original poster @mglaman or @mondrake at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!
Comment #11
mondrakeRebased, only merge conflicts resolved.
Comment #12
smustgrave commentedRebase seems good.
Comment #13
nod_merge conflict
Comment #14
mondrakemerged with 11.x, no conflicts. Dunno why GitLab still reports a merge error.
Comment #15
catchI'm really not sure about using templates like this. I realise I'm in the minority using vim (and without whatever vim support would take advantage of this, there's probably something somewhere), but don't think I'm the only one. There's also reading docs on api.drupal.org or gitlab which people sometimes do.
Comment #16
smustgrave commentedappears to need a rebase
If you are another contributor eager to jump in, please allow the previous poster at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!
Comment #17
mondrakeThe weirdness here is that this MR is practically just a c/p of (some of) the stubs that are defined in mglaman/phpstan-drupal. The difference is the use of FQCN as per Drupal code standards vs class names and
useimports.If we don't do this, Drupal core is not compatible with its own PHPStan tooling. Therefore we should remove the stubs from mglaman/phpstan-drupal if we aim at clearing the baseline... fun.
Comment #18
mondrakeComment #19
smustgrave commentedAppears some feedback was added to the MR.
Comment #20
gselderslaghs commented#9 works as expected, @mglaman thanks for saving me a headache on this one
Comment #21
mondrakeRebased and addressed feedback.
Comment #22
mstrelan commentedAdded steps to reproduce, can confirm all 9 instances in the baseline are fixed in this MR.
I'm guessing this is specifically about ListInterface, which is the only class that uses the templates outside of where they are defined. This concern is understandable in that looking at that method in isolation doesn't tell you what
Tis. I think longwaves suggestion in #15 to use@phpstan-returnand leaving the original@returnfor these addresses that concern, so I think we can make this RTBC. Please set back if you're not satisfied with that.We could of course have a coding standards discussion for use of templates, e.g. use
@phpstan-params when referring to templates, but I don't think we should block this on that issue.Comment #23
longwaveBeen sitting on this one a while but given we agree that PHPStan helps more than it hinders, I think this is the only way forward. @mglaman's
phpstan-drupalhas already implemented this and fields are such a key part of Drupal that improving static analysis here will help more people. I think that @catch's concerns are resolved by keeping@returnfor humans and using@phpstan-returnfor machines to read.Committed and pushed 41ee2804c3f to 11.x and 567f20edeaa to 11.3.x. Thanks!