Add Field._clean_bound_field() to remove isinstance check in BaseForm._clean_fields()

[Chris Jerdonek]

This is a follow-up to ticket #32920.

Currently, BaseForm._clean_fields() does an instance check for each field to special-case FileField:
​https://github.com/django/django/blob/f5669fd7b568cf8a3eda1e65c1c6fb583c7b177d/django/forms/forms.py#L396-L400
I noticed that if Field grows a _clean_bound_field(bf) method, then the special-casing could be handled by overriding _clean_bound_field() in FileField, and _clean_fields() would become simpler.

[Nick Pope]

Accepting in principle. The overhead of the extra method call is probably offset by the removal of the isinstance() check.

(I'm basing the following on top of your changes in your ​PR for #32920.)

If I understand this correctly, then the idea is that we end up with changing this:

# django/forms/forms.py

class BaseForm:
    ...
    def _clean_fields(self):
        ...
            value = bf.initial if field.disabled else bf.data
            ...
                if isinstance(field, FileField):
                    value = field.clean(value, bf.initial)
                else:
                    value = field.clean(value)
                self.cleaned_data[name] = value
                ...

To this:

# django/forms/fields.py

class Field:
    ...
    def _clean_bound_field(self, bf):
        value = bf.initial if self.disabled else bf.data
        return self.clean(value)
    ...

class FileField(Field):
    ...
    def _clean_bound_field(self, bf):
        value = bf.initial if self.disabled else bf.data
        return self.clean(value, bf.initial)
    ...

# django/forms/forms.py

class BaseForm:
    ...
    def _clean_fields(self):
        ...
                self.cleaned_data[name] = field._clean_bound_field(bf)
                ...

I have a few thoughts:

• Would we want to use value = field.bound_data(bf.data, bf.initial) in _clean_bound_field()?
  Field.bound_data() implements the same logic, but is specialized in FileField.bound_data() and JSONField.bound_data().
  (I'm not sure whether not using the specialized logic is something we want for the value passed to .clean() or not...)
• What about adding initial=None to the signature of Field.clean()?
  The advantage is that it takes away an extra layer of function calls, the signature of Field.clean() becomes unified for all fields, and we achieve the same simplification.
  The disadvantage is that we'd need to have a deprecation period using inspect.signature() as we'd be changing public API.
• Is there a way we can do this without having to pass initial into FileField.clean() instead? It feels like this was hacked in for a niche edge case.

[syed waheed]

PR - ​https://github.com/django/django/pull/17747
Could you please provide a review for the pull request
thank you

[GitHub <noreply@…>]

In d9b91e38:

Fixed #32923 -- Refactored out Field._clean_bound_field().