Support PHP 8.2 with ctools

RTBC patch #9

We need this change for PHP 8.2 compatibility, it's the same change that's been used in Drupal 10 core, in other drupal 7 contrib modules such as webform and i18n and others.

The test fails are unrelated and this change will not cause a regression.

💬 D7 ctools head tests affected by D10 deprecations - See upstream issues Fixed

Ok new patch

This fix is ready for maintainers eyes.

Need a second pair of eyes from the community to RTBC.
There are no failing tests that this patch resolves (that I could see).
No steps to reproduce the errors though it's marked as a feature request the title is nebulous about the testbot composer issues.
This needs a better title and a clear set of instructions to test so we know the changes are not just for change sake.

@joseph.olstad Sorry not meant to derail your progress. Thanks for taking the time to get things ready for PHP 8.2 and fixing the color issue that has plagued me in other issues too!

To reproduce the errors, run the automated tests against HEAD with PHP 8.2

Here's a no_change patch to prove failures.

The above no_change.patch fails PHP 8.2 testing.

Please review/apply/commit/tag/release with patch 16 to resolve the ctools PHP 8.2 compatibility issues.

@Joel Pittet, to give you a bit more background information, I've written PHP 8.2 compatibility patches for the following modules and they have been committed and released:

entity_translation
#3327347-37: Fully Support PHP 8.2 →
file_entity
#3327347: Fully Support PHP 8.2 →
date module
📌 PHP 8.2 - Fix deprecated dynamic properties Fixed
media
✨ Media - PHP 8.2 compatibility Fixed
i18n
#3327343: Fully Support PHP 8.2 →
entityreference
🐛 PHP 8.2 compatibility fix Fixed
webform
🐛 PHP 8.2 compatibility fix Fixed

and a few others.

These are all very similar fixes to patch 16.

This is a very simple fix that we need to move forward on in order to keep fixing the modules that depend on ctools. Please consider fast tracking patch 16. There's a lot of other projects I'd like to continue fixing and this one is holding things up.

@joel pittet, in PHP 8.2 Dynamic Properties are deprecated in the sense that they are now disabled by default on all classes. This is to improve performance and reduce memory consumption. If a class doesn't need dynamic properties support PHP 8.2 does not load it unless explicitly asked on a class by class basis.

Patch 16 above is very simple, adding two missing properties to one class and explicitly enabling Dynamic properties on the other.

The upgrade to PHP 8.2 is a significant performance improvement and once again lowers memory footprint.

Patch 16 resolves 4 HEAD failures as illustrated above. Patch 16 is very similar to the rest of the PHP 8.2 fixes that have already gone into core and many contrib modules and dependencies.

The webform module needed a fix due to someone misspelling a property , previous versions of php just added the property dynamically but PHP 8.2 will not do this unless the class has a Dynamic Properties directive.

Does this help ?

Oh the illustration with the failed tests really helps and the detailed explaination, thanks again @joseph.olstad

I committed this to D9 and wonder if it's relevant (will test tomorrow) but since you are doing some 8.2 support I'll ask your take?

📌 PHP 8.2: ${var} in strings is deprecated Fixed

D7 findings

❯ ag '\$\{'
includes/export.inc
283:    if (empty(${$export['identifier']})) {
291:    $item = ${$export['identifier']};
1237:    $output .= "  \${$export['identifier']}s = array();\n\n";
1240:      $output .= "  \${$export['identifier']}s['" . check_plain($object->{$export['key']}) . "'] = \${$export['identifier']};\n\n";
1242:    $output .= "  return \${$export['identifier']}s;\n";

page_manager/page_manager.module
1202:      $code .= "  \${$export['identifier']}s['" . check_plain($object->{$export['key']}) . "'] = \${$export['identifier']};\n\n";
1204:    $code .= "  return \${$export['identifier']}s;\n";

I noticed a few more while enabling a bunch ctools modules:

Deprecated function: Creation of dynamic property view::$export_module is deprecated in _ctools_export_get_defaults() (line 689 of /var/www/html/public/sites/all/modules/contrib/ctools/includes/export.inc).
Deprecated function: Creation of dynamic property view::$export_type is deprecated in ctools_export_load_object() (line 538 of /var/www/html/public/sites/all/modules/contrib/ctools/includes/export.inc).
Deprecated function: Creation of dynamic property view::$in_code_only is deprecated in ctools_export_load_object() (line 539 of /var/www/html/public/sites/all/modules/contrib/ctools/includes/export.inc).
Deprecated function: Creation of dynamic property view::$table is deprecated in ctools_export_load_object() (line 540 of /var/www/html/public/sites/all/modules/contrib/ctools/includes/export.inc).
Deprecated function: Creation of dynamic property view::$type is deprecated in ctools_export_load_object() (line 537 of /var/www/html/public/sites/all/modules/contrib/ctools/includes/export.inc).

I just realized #29 is all the view class from views which has been fixed already in dev with #[\AllowDynamicProperties]

#28 I'm still investigating but we can create a follow-up

Just going to trigger a no change test because ci on main project returns error today

Just looking at comment 28 no change patch, expecting pass on all tests

Ok 21 is passing, @Joel Pittet could you please retrigger the project tests for ctools? Why would the no change patch work and the project tests on commit , not work?
Perhaps retrigger those and we can review that. Required maintainer prerms to do this

I'll retrigger them thanks. I don't think the stuff I mentioned in #28 above have test coverage.

Deprecated function: Creation of dynamic property ctools_context::$arg_length is deprecated in ctools_plugin_example_context_create_simplecontext() (line 80 of /var/www/html/public/sites/all/modules/contrib/ctools/ctools_plugin_example/plugins/contexts/simplecontext.inc).

One more

Maybe ctools_context should also have #[\AllowDynamicProperties], that might help other ecosystem modules that add stuff to context?

And same for ctools_context_required

Deprecated function: Creation of dynamic property ctools_context_required::$restrictions is deprecated in ctools_context_required->__construct() (line 275 of /var/www/html/public/sites/all/modules/contrib/ctools/includes/context.inc).
Deprecated function: Creation of dynamic property ctools_context_required::$restrictions is deprecated in DrupalDatabaseCache->prepareItem() (line 449 of /var/www/html/public/includes/cache.inc).

I did some testing and exporting and #28 is not an issue.

Thoughts on this patch in relation to the comments #38 and #40?

Changing parent to the release plan 🌱 Plan for CTools 7.x-1.21 release Fixed

Patch looks good, put it in as a compatibility insurance policy IMHO good to go in.

I'll release this soon, just need to get the branch to pass the tests

weird that the project tests are outputing such a strange output about being hundred+ commits behind and what's this about a production branch?

I've added a comment here:

#3332436-13: Refactor & correct list of removed core modules →

I asked around and hestenet mentioned this was the issue #3315509: phpcs endless loop with "Drupal" rules due to missing @endcode → so I did those fixes here 🐛 phpcs endless loop with "Drupal" rules due to missing @endcode Fixed

@joelpittet, and @hestenet , thanks for that!

Automatically closed - issue fixed for 2 weeks with no activity.