Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Replace function_exists() with is_callable() in form.inc as part of #1002080: [Policy, no patch] Normalize on usage of is_callable() instead of function_exists().
Comment | File | Size | Author |
---|---|---|---|
#3 | drupal_2040559_3.patch | 2 KB | Xano |
#3 | interdiff.txt | 3.48 KB | Xano |
#1 | drupal_2040559_1.patch | 3.44 KB | Xano |
Comments
Comment #1
XanoComment #2
tstoecklerI think these should stay as function_exists() because that's exactly what's being checked here: "Is there a function called $foo_submit() that I can auto-magically call as a submit handler?" There's no chance that this is going to be an object, because you can hardly append '_submit' to an object.
This change looks great, but the direct function call should be replaced by call_user_func() in order to get object support. This would need a test, as well.
Comment #3
XanoThanks for the review :) Find and replace wasn't such a good idea after all.
Comment #4
tstoecklerFrom the interdiff:
Ohh, nice. I didn't catch that.
Patch looks great now, but I think we need some tests.
Comment #5
XanoHow do you suggest we test this? The only way is to create a custom element in system_test.module that uses class methods, and then the class with the methods, but that may be a little overkill.
Comment #6
tstoecklerI don't think that is overkill. Look at the existing form_test.module for how this could look.
Comment #7
XanoThe #value_callback method callbacks are tested in #2041367: Element values retrieved from top to bottom instead of bottom to top.
Comment #8
XanoWe apparently already have a change record that says this should be possible, so this issue technically fixes a bug.
Comment #9
tim.plunkettI think you need call_user_func_array for the ones taken by reference.
Comment #10
XanoAre you referring to the batch API redirect_callback?
I'm still not sure. It's an extra 150 lines of test code just to make sure that two calls also support classes.
Comment #11
Xano#3: drupal_2040559_3.patch queued for re-testing.
Comment #12.0
(not verified) CreditAttribution: commentedAdded parent issue.
Comment #13
Xanoform.inc is now \Drupal\Core\Form\FormBuilder
Comment #14
tim.plunkettWas done in #2072995: Move FAPI callbacks for file/image widgets in classes, the only remaining function_exists in FormBuilder are when concatenating a string:
elseif (function_exists($form_id . '_submit')) {