Problem/Motivation
Deprecated dynamic property in TermWeightWidgetOrderProcessor::sortResults() in PHP 8.3
In PHP 8.2+, the creation of dynamic properties is deprecated, and this triggers a deprecation notice in TermWeightWidgetOrderProcessor::sortResults() when the $termWeight property is assigned dynamically to a Result object. This will cause issues in PHP 8.3 and beyond, as dynamic properties are not allowed by default.
The issue occurs in the Facets module when sorting results using term weight, causing the following deprecation warning:
Deprecated function: Creation of dynamic property Drupal\facets\Result\Result::$termWeight is deprecated in Drupal\facets\Plugin\facets\processor\TermWeightWidgetOrderProcessor->sortResults() (line 91 of modules/contrib/facets/src/Plugin/facets/processor/TermWeightWidgetOrderProcessor.php).
Steps to reproduce
- Install the Facets module and set up a facet that uses the Term Weight Widget Order processor.
- Use PHP 8.2 or later.
- Perform a search or view a page where the facet is displayed.
- Observe the deprecation warning in the log or on the page.
Proposed resolution
To resolve the issue and prevent the use of dynamic properties, the termWeight property should be explicitly declared in the Result class and ResultInterface. Additionally, use proper setter and getter methods to manage the property.
Update the TermWeightWidgetOrderProcessor::sortResults() method to use the getter and setter instead of directly assigning or accessing the dynamic property
Remaining tasks
User interface changes
None
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | facets_3482015_279.patch | 3.56 KB | impol |
| #3 | 3482015-3.patch | 3.22 KB | mohammad-fayoumi |
Issue fork facets-3482015
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:
Comments
Comment #3
mohammad-fayoumiCreating a static patch file for the latest stable version 3.0.0-beta1
Comment #4
mohammad-fayoumiComment #5
joseph.olstadPHPUnit is failing on this change.
Comment #6
joseph.olstadSee also: #3469997: Use of dynamic properties is deprecated in PHP 8.2+
Comment #9
herved commentedSorry for the noise, I didn't know if I could rebase the existing MR, so I created a new one.
Tests are passing, the branch only needed to be rebased.
All credits to mohammad-fayoumi
Comment #10
joseph.olstadok thanks for that.
There's still many others listed if you're ambitious. Most of the ones excluding dependency injection should be done sooner than later.
https://git.drupalcode.org/issue/facets-3482015/-/jobs/4157965
Comment #11
joseph.olstadWith that said, baby steps, suggest making a new issue for the others.
Comment #12
herved commentedSure, I can create an issue for phpstan.
Why excluding DI ? as it breaks backwards compatibility ?
Comment #13
joseph.olstad@herved , feel free to work on the DI also, it's just that a large amount of code changes will require rerolled patches everywhere, this is fine, I think these phpstan errors are a good thing to fix.
Comment #14
herved commented@joseph.olstad see #3502957: Fix PHPStan issues reported on CI/gitlab and #3502922: Fix PHPCS issues reported on CI/gitlab, thanks
Comment #15
impol commentedAdding a static patch.
Comment #16
liam morlandComment #18
strykaizer