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
FormStateInterface::cleanValues() method doc specifies @return $this, but FormState::cleanValues() doesn't return $this.
Proposed resolution
Fix it.
Remaining tasks
Reviews
Commit
User interface changes
None
API changes
None
Beta phase evaluation
| Issue category | Bug because that wasn't the intent |
|---|---|
| Issue priority | Normal because it only happens when you try something like $form_state->cleanValues()->unsetValue($blah);. Your IDE says you can do it, but the code fails. This could be minor, but bugs can't be minor. |
| Unfrozen changes | Not Unfrozen |
| Disruption | Disruption is zero to none. |
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | form_state_clean_values-2388349-5.patch | 1.01 KB | almaudoh |
| #5 | form_state_clean_values-2388349-5-TEST-ONLY.patch | 676 bytes | almaudoh |
| form_state_clean_values.patch | 359 bytes | almaudoh | |
Comments
Comment #2
almaudoh CreditAttribution: almaudoh commentedMarking quickfix to get some attention.
Comment #3
TR CreditAttribution: TR commentedThis bit me several times.
A common use for this function would be
$form_state->cleanValues()->getValues()to return an array of form state values with the Form API tokens etc. removed. As documented, that should work. But it does not because the implementation is faulty.I checked the source and all the other method implementation in the FormState class return the proper documented types.
All methods declared by FormStateInterface are designed to be chainable, if appropriate. Setters, for example, always return $this, whereas getters obviously can't. cleanValues() simply modifies $form_state, so returning this is both appropriate and desired.
It's clear to me that FormStateInterface::cleanValues() was *intended* to return $this, and that the documentation that says it returns $this is not a mistake. Rather, the FormState implementation class is the place where the fault lies.
Comment #4
alexpottLet's test this.
Comment #5
almaudoh CreditAttribution: almaudoh commentedTests added.
Comment #6
tim.plunkettLooks good, thanks!
Comment #8
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 89e6ec9 and pushed to 8.0.x. Thanks!