Magic properties for FieldItemListInterface not detected

[kasperg]

In my code I have something that boils down to the following statement:

/** @var \Drupal\node\NodeInterface $node */
$node->get('field_name')->value;

This results in the following error: Access to an undefined property Drupal\Core\Field\FieldItemListInterface::$value.

I find debugging this quite difficult but when I dig into it it seems as if the problem is caused by the usage of $reflection->implementsInterface() in EntityFieldsViaMagicReflectionExtension. In my tests this returns false for instances of the actual object type like what is returned by EntityFieldReflection.

Unfortunately I cannot post the full source code. I would also expect the problem to be picked up by the current testing but it is not.

I am running the analysis using Drupal Check installed as a project dependency using Composer with the following versions:

mglaman/drupal-check                            1.1.3
mglaman/phpstan-drupal                          0.12.6
phpstan/phpstan                                 0.12.42

When I analyze the same code with a standalone version of Drupal Check using dependencies in their locked versions the error does not occur. Downgrading phpstan/phpstan to 0.12.32 also makes the problem go away. Perhaps the situation is caused by a problem upstream.

[mglaman]

FieldItemListPropertyReflection is supposed to handle this. Maybe it stopped. I noticed this as well.

[dench0]

I have the same issue.

mglaman/drupal-check                       1.1.6
mglaman/phpstan-drupal                     0.12.7 
phpstan/phpstan                            0.12.64          
phpstan/phpstan-deprecation-rules          0.12.6 

[andrewbelcher]

I think I've spotted the issue here. I'm guessing it is a change/break upstream, but the issue is that if the class being reflected is actually the interface itself:

$reflection->implementsInterface('Drupal\Core\Field\FieldItemListInterface') === FALSE

This will be the case as that is what is returned by EntityFieldReflection::getReadableType...

Short of upstream getting fixed, the alternative is to do (in both hasProperty and getProperty):

$reflection->getName() === 'Drupal\Core\Field\FieldItemListInterface' || $reflection->implementsInterface('Drupal\Core\Field\FieldItemListInterface')

This is not PHP's native ReflectionClass which does work, PHPStan is using Roave/BetterReflection.

For reference:

mglaman/phpstan-drupal         0.12.7
phpstan/phpstan                0.12.67

[andrewbelcher]

(Possibly more suitable for a separate issue) FieldItemListPropertyReflection has the following:

// @todo use the class reflection and be more specific about handled properties.
// Currently \PHPStan\Reflection\EntityFieldReflection::getType always passes FieldItemListInterface.

Do you have an idea of the direction to resolve that? Whilst the above fixes it for the hardcoded list of properties (entity, value & target_id), it would be amazing if it was able to detect the appropriate field types and get the properties from there.

I imagine we'd want to break it up into steps, perhaps something like (though maybe some are too difficult to be worth it):

1. Gather all field types and allow all field properties across the board
2. Get the entity type and look up the base/bundle field storage for the types and use specific properties for the type
3. Check implementations of hook_entity_field_storage_info and include those types
4. Analyse module's config and site wide config directories for config field storage
5. Try and handle dependencies so only the relevant hooks/config are included

My hesitation currently is that adding PHPStan to CI is going to create quite a bit of noise for developers. Whilst it is a really valuable check, I can't see how it could be enabled until it handles additional field types...

[PeterSletten]

Is there a solution for this in the pipeline?

[mglaman]

There isn't. Currently the project only receives maintenance development. I don't have spare cycles for active feature development right now. I am available for paid feature development or adding time via GitHub Sponsors.

[mglaman]

I think this may be fixed. But I'm not sure. We need a specific test case. This may be fixed by updating the FieldItemListInterface stub and overriding \Drupal\Core\TypedData\ListInterface::first and such to specify it returns FieldItemInterface not the generic TypedDataInterface.

Maybe we could handle it with generics somehow, more easily.

/**
 * @template T of FieldItemInterface
 * @extends ListInterface<T>
 * @property mixed $value
 */

So first would return T, I think.

EDIT: ListInterface already does this. So I think this is fixed.

[mglaman]

I think #433 fixed this by stubbing ListInterface. I'm going to close and we can open a new issue if not.