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.
Problem/Motivation
Noticed the parameter $form_status
was incorrectly named and not used.
Proposed resolution
Remove it
Comment | File | Size | Author |
---|---|---|---|
#6 | 2778655-6.patch | 489 bytes | joelpittet |
| |||
#2 | remove_unused-2778655-2.patch | 469 bytes | joelpittet |
|
Comments
Comment #2
joelpittetComment #3
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedI think that parameter should have been called
$form_state
instead of$form_status
. I think it is not unusual to have that parameter defined on form callbacks even if you do not make use of it?The coding standards aren't clear about whether or not
$form_state
is required or should be removed when not used. According to the examples, it seems like it is common to have it always defined.https://www.drupal.org/coding-standards/docs#forms
Comment #4
joelpittet@MegaChriz yes that is correct. And there is no way to modify that unused variable inside that function so defining it is fairly pointless. The
$form_state
is initialized indrupal_get_form()
and at that point only contains$form_state['build_info']['args'] = $args;
If you aren't passing it to an alter or modifying it, it's not needed.Comment #5
twistor CreditAttribution: twistor as a volunteer commentedLest's rename the variable to $form_state, add the array type hinting, and call it a day. All form functions take a $form and a $form_state. Whether we use the $form_state isn't important. If there was a performance win here, I would still say no, but there isn't :) The signature for forms is ($form, $form_state). This is even required for all of our class-based forms.
Comment #6
joelpittet@twistor, ok it's your call. I just noticed PHP Storm saying it's not used so decided to remove it.
Comment #8
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedCommitted #6. Thanks for spotting the wrong named parameter, joelpittet!
I agree that in most cases it is better to remove unused parameters, but in this case I agree with twistor here. I think the code is more consistent if
$form_state
is defined, because nearly all form constructors have it even not used.