Problem/Motivation

Once we can use property hooks, we can define entity as a property on EntityReferenceFieldItemList and use property hooks to return the referenced entity.

This will allow us to completely remove the hack added in #3565937: Workaround PHP bug with fibers and __get() because property hooks don't have the same guarding issue.

The test coverage added in there can stay - since that demonstrates that the fiber suspend isn't broken by any PHP bugs.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3566626

Command icon Show commands

Start 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:

About issue forks

Comments

catch created an issue. See original summary.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

godotislate made their first commit to this issue’s fork.

godotislate opened merge request !14588

godotislate’s picture

godotislate
he/him
commented
Status: Active » Needs work

Took a shot with property hooks for entity in both EntityReferenceItem and EntityReferenceItemList. I chose EntityReferenceItem over EntityReferenceItemBase, since entity is defined in EntityReferenceItem::propertyDefinitions and not the code>EntityReferenceItemBase class.

I thought it also made sense to add property hooks for target_id, even though that property isn't involved in the Fiber issue.

But the test PHPCS and PHPStan configuration doesn't like property hooks:
https://git.drupalcode.org/issue/drupal-3566626/-/jobs/8312295

PHP_CodeSniffer version 3.13.5 (stable) by Squiz and PHPCSStandards
$ composer phpcs -- -s --report-full --report-summary --report-\\Micheh\\PhpCodeSniffer\\Report\\Gitlab=$_ARTIFACTS_DIR/phpcs-quality-report.json --cache=core/.phpcscache
FILE: .../core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
--------------------------------------------------------------------------------
FOUND 26 ERRORS AND 39 WARNINGS AFFECTING 56 LINES
--------------------------------------------------------------------------------
   1 | ERROR   | [ ] An error occurred during processing; checking has been
     |         |     aborted. The error message was: Undefined array key "" in
     |         |     /builds/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/TernaryOperatorHelper.php
     |         |     on line 59
     |         |     The error originated in the
     |         |     SlevomatCodingStandard.ControlStructures.RequireShortTernaryOperator
     |         |     sniff on line 47. (Internal.Exception)
  36 | ERROR   | [x] Multi-line function call not indented correctly; expected
     |         |     4 spaces but found 2
     |         |     (PEAR.Functions.FunctionCallSignature.Indent)
  37 | ERROR   | [x] Multi-line function call not indented correctly; expected
     |         |     4 spaces but found 2
     |         |     (PEAR.Functions.FunctionCallSignature.Indent)
  38 | ERROR   | [x] Multi-line function call not indented correctly; expected
     |         |     4 spaces but found 2
     |         |     (PEAR.Functions.FunctionCallSignature.Indent)
  39 | ERROR   | [x] Array value not aligned correctly; expected 16 spaces but
     |         |     found 4 (Squiz.Arrays.ArrayDeclaration.ValueNotAligned)
  40 | ERROR   | [x] Multi-line function call not indented correctly; expected
     |         |     4 spaces but found 2
     |         |     (PEAR.Functions.FunctionCallSignature.Indent)
  40 | ERROR   | [x] Closing parenthesis not aligned correctly; expected 15
     |         |     spaces but found 2
     |         |     (Squiz.Arrays.ArrayDeclaration.CloseBraceNotAligned)
  41 | ERROR   | [x] Multi-line function call not indented correctly; expected
     |         |     4 spaces but found 2
     |         |     (PEAR.Functions.FunctionCallSignature.Indent)
  42 | ERROR   | [x] Multi-line function call not indented correctly; expected
     |         |     4 spaces but found 2
     |         |     (PEAR.Functions.FunctionCallSignature.Indent)
  43 | ERROR   | [x] Multi-line function call not indented correctly; expected
     |         |     4 spaces but found 2
     |         |     (PEAR.Functions.FunctionCallSignature.Indent)
  44 | ERROR   | [x] Multi-line function call not indented correctly; expected
     |         |     4 spaces but found 2
     |         |     (PEAR.Functions.FunctionCallSignature.Indent)
  51 | ERROR   | [ ] There must not be more than one property declared per
     |         |     statement (PSR2.Classes.PropertyDeclaration.Multiple)
  52 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  53 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  53 | ERROR   | [ ] Missing member variable doc comment
     |         |     (Drupal.Commenting.VariableComment.Missing)
  53 | ERROR   | [ ] Visibility must be declared on property "$this"
     |         |     (PSR2.Classes.PropertyDeclaration.ScopeMissing)
  54 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  55 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  55 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  56 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  56 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  57 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  63 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  64 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  64 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  65 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  65 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  66 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  67 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  67 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  68 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  68 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  69 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  75 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  84 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  94 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 142 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 157 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 184 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 191 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 238 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 253 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 297 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 311 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 327 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 341 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 359 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 425 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 440 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 467 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 542 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 558 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 565 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 605 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 615 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 674 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 681 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 688 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 698 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 724 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 734 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 757 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 769 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 778 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 809 | WARNING | [ ] Code after the RETURN statement on line 53 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 22 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------
FILE: /builds/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php
--------------------------------------------------------------------------------
FOUND 16 ERRORS AND 11 WARNINGS AFFECTING 19 LINES
--------------------------------------------------------------------------------
   1 | ERROR   | [ ] An error occurred during processing; checking has been
     |         |     aborted. The error message was: Undefined array key "" in
     |         |     /builds/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/TernaryOperatorHelper.php
     |         |     on line 59
     |         |     The error originated in the
     |         |     SlevomatCodingStandard.ControlStructures.RequireShortTernaryOperator
     |         |     sniff on line 47. (Internal.Exception)
  17 | ERROR   | [ ] There must not be more than one property declared per
     |         |     statement (PSR2.Classes.PropertyDeclaration.Multiple)
  18 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  19 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  19 | ERROR   | [ ] Missing member variable doc comment
     |         |     (Drupal.Commenting.VariableComment.Missing)
  19 | ERROR   | [ ] Visibility must be declared on property "$this"
     |         |     (PSR2.Classes.PropertyDeclaration.ScopeMissing)
  20 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  21 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  21 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  22 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  22 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  23 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  29 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  30 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  30 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  31 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  31 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  32 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  33 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  33 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  34 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  34 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 6
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  35 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
     |         |     (Drupal.WhiteSpace.ScopeIndent.IncorrectExact)
  41 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  51 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
  87 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
 132 | WARNING | [ ] Code after the RETURN statement on line 19 cannot be
     |         |     executed (Squiz.PHP.NonExecutableCode.Unreachable)
--------------------------------------------------------------------------------

https://git.drupalcode.org/issue/drupal-3566626/-/jobs/8312294

-- --------------------------------------------------------------------------- 
     Error                                                                      
 -- --------------------------------------------------------------------------- 
     Internal error: Method                                                     
     Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem::$entity::ge  
     t() does not exist while analysing file                                    
     /builds/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReference  
     Item.php                                                                   
                                                                                
     Run PHPStan with -v option and post the stack trace to:                    
     https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml     
                                                                                
     Internal error: Method                                                     
     Drupal\Core\Field\EntityReferenceFieldItemList::$target_id::get() does     
     not exist while analysing file                                             
     /builds/core/lib/Drupal/Core/Field/EntityReferenceFieldItemList.php        
                                                                                
     Run PHPStan with -v option and post the stack trace to:                    
     https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml     
                                                                                
 -- ---------------------------------------------------------------------------

I ignored these temporarily, and otherwise tests are passing.

godotislate’s picture

godotislate
he/him
commented

Not sure about PHPStan, but looks like PHPCS support for property hooks is still in progress: https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/734

mondrake’s picture

mondrake
Location 🇮🇹
commented

Got similar PHPStan error while playing with property hooks in the image system:

 -- ------------------------------------------------------------------------------------------------------------------------------------------------- 
     Error                                                                                                                                            
 -- ------------------------------------------------------------------------------------------------------------------------------------------------- 
     Internal error: Method Drupal\system\Plugin\ImageToolkit\Operation\gd\Scale::$needsToolkitInjection::get() does not exist while analysing file   
     /Users/xxxxx/Dev/drupal/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Scale.php                                             
                                                                                                                                                      
     Run PHPStan with -v option and post the stack trace to:                                                                                          
     https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml                                                                           
                                                                                                                                                      
     Internal error: Method Drupal\system\Plugin\ImageToolkit\Operation\gd\Resize::$needsToolkitInjection::get() does not exist while analysing file  
     /Users/xxxxx/Dev/drupal/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Resize.php                                            
                                                                                                                                                      
     Run PHPStan with -v option and post the stack trace to:                                                                                          
     https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml                                                                           
                                                                                                                                                      
 -- -------------------------------------------------------------------------------------------------------------------------------------------------

if I composer remove mglaman/phpstan-drupal, the error disappears. Not necessarily a phpstan-drupal issue, anyway - could also be related to phpstan-deprecation-rules.

Looks like we're having a general problem with property hooks atm.

mondrake’s picture

mondrake
Location 🇮🇹
commented

#7 the issue is in \mglaman\PHPStanDrupal\DeprecatedScope\IgnoreDeprecationsScope. That method tries to reflect property hook g/setter as if they were normal methods which they aren't apparently.

mondrake’s picture

mondrake
Location 🇮🇹
commented

I proposed a PR with a fix upstream, https://github.com/mglaman/phpstan-drupal/pull/935

godotislate’s picture

godotislate
he/him
commented

Thanks for investigating!
I think the bigger challenge might be PHPCS, but one step at a time.