From 43b574007ef5edfe11f3b899fbe0a6fc05c43774 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Sat, 3 Jun 2017 16:49:01 +0200 Subject: [PATCH] Fixed #28192 -- Required passing optional form field args as keyword args. --- django/contrib/postgres/forms/array.py | 6 +- django/forms/fields.py | 80 +++++++++---------- django/forms/models.py | 18 ++--- docs/ref/forms/fields.txt | 4 +- docs/releases/2.0.txt | 13 +++ .../forms_tests/field_tests/test_charfield.py | 3 +- .../field_tests/test_multivaluefield.py | 4 +- 7 files changed, 66 insertions(+), 62 deletions(-) diff --git a/django/contrib/postgres/forms/array.py b/django/contrib/postgres/forms/array.py index 97e4346f60..6b3b217810 100644 --- a/django/contrib/postgres/forms/array.py +++ b/django/contrib/postgres/forms/array.py @@ -16,10 +16,10 @@ class SimpleArrayField(forms.CharField): 'item_invalid': _('Item %(nth)s in the array did not validate: '), } - def __init__(self, base_field, delimiter=',', max_length=None, min_length=None, *args, **kwargs): + def __init__(self, base_field, *, delimiter=',', max_length=None, min_length=None, **kwargs): self.base_field = base_field self.delimiter = delimiter - super().__init__(*args, **kwargs) + super().__init__(**kwargs) if min_length is not None: self.min_length = min_length self.validators.append(ArrayMinLengthValidator(int(min_length))) @@ -156,7 +156,7 @@ class SplitArrayField(forms.Field): 'item_invalid': _('Item %(nth)s in the array did not validate: '), } - def __init__(self, base_field, size, remove_trailing_nulls=False, **kwargs): + def __init__(self, base_field, size, *, remove_trailing_nulls=False, **kwargs): self.base_field = base_field self.size = size self.remove_trailing_nulls = remove_trailing_nulls diff --git a/django/forms/fields.py b/django/forms/fields.py index f621aa65da..1bc6953561 100644 --- a/django/forms/fields.py +++ b/django/forms/fields.py @@ -53,7 +53,7 @@ class Field: } empty_values = list(validators.EMPTY_VALUES) - def __init__(self, required=True, widget=None, label=None, initial=None, + def __init__(self, *, required=True, widget=None, label=None, initial=None, help_text='', error_messages=None, show_hidden_initial=False, validators=(), localize=False, disabled=False, label_suffix=None): # required -- Boolean that specifies whether the field is required. @@ -205,12 +205,12 @@ class Field: class CharField(Field): - def __init__(self, max_length=None, min_length=None, strip=True, empty_value='', *args, **kwargs): + def __init__(self, *, max_length=None, min_length=None, strip=True, empty_value='', **kwargs): self.max_length = max_length self.min_length = min_length self.strip = strip self.empty_value = empty_value - super().__init__(*args, **kwargs) + super().__init__(**kwargs) if min_length is not None: self.validators.append(validators.MinLengthValidator(int(min_length))) if max_length is not None: @@ -243,12 +243,12 @@ class IntegerField(Field): } re_decimal = re.compile(r'\.0*\s*$') - def __init__(self, max_value=None, min_value=None, *args, **kwargs): + def __init__(self, *, max_value=None, min_value=None, **kwargs): self.max_value, self.min_value = max_value, min_value if kwargs.get('localize') and self.widget == NumberInput: # Localized number input is not well supported on most browsers kwargs.setdefault('widget', super().widget) - super().__init__(*args, **kwargs) + super().__init__(**kwargs) if max_value is not None: self.validators.append(validators.MaxValueValidator(max_value)) @@ -324,9 +324,9 @@ class DecimalField(IntegerField): 'invalid': _('Enter a number.'), } - def __init__(self, max_value=None, min_value=None, max_digits=None, decimal_places=None, *args, **kwargs): + def __init__(self, *, max_value=None, min_value=None, max_digits=None, decimal_places=None, **kwargs): self.max_digits, self.decimal_places = max_digits, decimal_places - super().__init__(max_value, min_value, *args, **kwargs) + super().__init__(max_value=max_value, min_value=min_value, **kwargs) self.validators.append(validators.DecimalValidator(max_digits, decimal_places)) def to_python(self, value): @@ -372,8 +372,8 @@ class DecimalField(IntegerField): class BaseTemporalField(Field): - def __init__(self, input_formats=None, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, *, input_formats=None, **kwargs): + super().__init__(**kwargs) if input_formats is not None: self.input_formats = input_formats @@ -490,12 +490,12 @@ class DurationField(Field): class RegexField(CharField): - def __init__(self, regex, max_length=None, min_length=None, *args, **kwargs): + def __init__(self, regex, **kwargs): """ regex can be either a string or a compiled regular expression object. """ kwargs.setdefault('strip', False) - super().__init__(max_length, min_length, *args, **kwargs) + super().__init__(**kwargs) self._set_regex(regex) def _get_regex(self): @@ -517,8 +517,8 @@ class EmailField(CharField): widget = EmailInput default_validators = [validators.validate_email] - def __init__(self, *args, **kwargs): - super().__init__(*args, strip=True, **kwargs) + def __init__(self, **kwargs): + super().__init__(strip=True, **kwargs) class FileField(Field): @@ -534,10 +534,10 @@ class FileField(Field): 'contradiction': _('Please either submit a file or check the clear checkbox, not both.') } - def __init__(self, *args, max_length=None, allow_empty_file=False, **kwargs): + def __init__(self, *, max_length=None, allow_empty_file=False, **kwargs): self.max_length = max_length self.allow_empty_file = allow_empty_file - super().__init__(*args, **kwargs) + super().__init__(**kwargs) def to_python(self, data): if data in self.empty_values: @@ -650,8 +650,8 @@ class URLField(CharField): } default_validators = [validators.URLValidator()] - def __init__(self, *args, **kwargs): - super().__init__(*args, strip=True, **kwargs) + def __init__(self, **kwargs): + super().__init__(strip=True, **kwargs) def to_python(self, value): @@ -751,12 +751,8 @@ class ChoiceField(Field): 'invalid_choice': _('Select a valid choice. %(value)s is not one of the available choices.'), } - def __init__(self, choices=(), required=True, widget=None, label=None, - initial=None, help_text='', *args, **kwargs): - super().__init__( - required=required, widget=widget, label=label, initial=initial, - help_text=help_text, *args, **kwargs - ) + def __init__(self, *, choices=(), **kwargs): + super().__init__(**kwargs) self.choices = choices def __deepcopy__(self, memo): @@ -812,10 +808,10 @@ class ChoiceField(Field): class TypedChoiceField(ChoiceField): - def __init__(self, *args, coerce=lambda val: val, empty_value='', **kwargs): + def __init__(self, *, coerce=lambda val: val, empty_value='', **kwargs): self.coerce = coerce self.empty_value = empty_value - super().__init__(*args, **kwargs) + super().__init__(**kwargs) def _coerce(self, value): """ @@ -879,10 +875,10 @@ class MultipleChoiceField(ChoiceField): class TypedMultipleChoiceField(MultipleChoiceField): - def __init__(self, *args, coerce=lambda val: val, **kwargs): + def __init__(self, *, coerce=lambda val: val, **kwargs): self.coerce = coerce self.empty_value = kwargs.pop('empty_value', []) - super().__init__(*args, **kwargs) + super().__init__(**kwargs) def _coerce(self, value): """ @@ -918,8 +914,8 @@ class ComboField(Field): """ A Field whose clean() method calls multiple Field clean() methods. """ - def __init__(self, fields, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, fields, **kwargs): + super().__init__(**kwargs) # Set 'required' to False on the individual fields, because the # required validation will be handled by ComboField, not by those # individual fields. @@ -960,9 +956,9 @@ class MultiValueField(Field): 'incomplete': _('Enter a complete value.'), } - def __init__(self, fields, *args, require_all_fields=True, **kwargs): + def __init__(self, fields, *, require_all_fields=True, **kwargs): self.require_all_fields = require_all_fields - super().__init__(*args, **kwargs) + super().__init__(**kwargs) for f in fields: f.error_messages.setdefault('incomplete', self.error_messages['incomplete']) @@ -1061,15 +1057,11 @@ class MultiValueField(Field): class FilePathField(ChoiceField): - def __init__(self, path, match=None, recursive=False, allow_files=True, - allow_folders=False, required=True, widget=None, label=None, - initial=None, help_text='', *args, **kwargs): + def __init__(self, path, *, match=None, recursive=False, allow_files=True, + allow_folders=False, **kwargs): self.path, self.match, self.recursive = path, match, recursive self.allow_files, self.allow_folders = allow_files, allow_folders - super().__init__( - choices=(), required=required, widget=widget, label=label, - initial=initial, help_text=help_text, *args, **kwargs - ) + super().__init__(choices=(), **kwargs) if self.required: self.choices = [] @@ -1117,7 +1109,7 @@ class SplitDateTimeField(MultiValueField): 'invalid_time': _('Enter a valid time.'), } - def __init__(self, input_date_formats=None, input_time_formats=None, *args, **kwargs): + def __init__(self, *, input_date_formats=None, input_time_formats=None, **kwargs): errors = self.default_error_messages.copy() if 'error_messages' in kwargs: errors.update(kwargs['error_messages']) @@ -1130,7 +1122,7 @@ class SplitDateTimeField(MultiValueField): error_messages={'invalid': errors['invalid_time']}, localize=localize), ) - super().__init__(fields, *args, **kwargs) + super().__init__(fields, **kwargs) def compress(self, data_list): if data_list: @@ -1146,10 +1138,10 @@ class SplitDateTimeField(MultiValueField): class GenericIPAddressField(CharField): - def __init__(self, protocol='both', unpack_ipv4=False, *args, **kwargs): + def __init__(self, *, protocol='both', unpack_ipv4=False, **kwargs): self.unpack_ipv4 = unpack_ipv4 self.default_validators = validators.ip_address_validators(protocol, unpack_ipv4)[0] - super().__init__(*args, **kwargs) + super().__init__(**kwargs) def to_python(self, value): if value in self.empty_values: @@ -1163,11 +1155,11 @@ class GenericIPAddressField(CharField): class SlugField(CharField): default_validators = [validators.validate_slug] - def __init__(self, *args, allow_unicode=False, **kwargs): + def __init__(self, *, allow_unicode=False, **kwargs): self.allow_unicode = allow_unicode if self.allow_unicode: self.default_validators = [validators.validate_unicode_slug] - super().__init__(*args, **kwargs) + super().__init__(**kwargs) class UUIDField(CharField): diff --git a/django/forms/models.py b/django/forms/models.py index 98b56392d0..3b5e1300e0 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -1128,10 +1128,10 @@ class ModelChoiceField(ChoiceField): } iterator = ModelChoiceIterator - def __init__(self, queryset, empty_label="---------", + def __init__(self, queryset, *, empty_label="---------", required=True, widget=None, label=None, initial=None, help_text='', to_field_name=None, limit_choices_to=None, - *args, **kwargs): + **kwargs): if required and (initial is not None): self.empty_label = None else: @@ -1139,8 +1139,10 @@ class ModelChoiceField(ChoiceField): # Call Field instead of ChoiceField __init__() because we don't need # ChoiceField.__init__(). - Field.__init__(self, required, widget, label, initial, help_text, - *args, **kwargs) + Field.__init__( + self, required=required, widget=widget, label=label, + initial=initial, help_text=help_text, **kwargs + ) self.queryset = queryset self.limit_choices_to = limit_choices_to # limit the queryset later. self.to_field_name = to_field_name @@ -1236,12 +1238,8 @@ class ModelMultipleChoiceField(ModelChoiceField): 'invalid_pk_value': _('"%(pk)s" is not a valid value.') } - def __init__(self, queryset, required=True, widget=None, label=None, - initial=None, help_text='', *args, **kwargs): - super().__init__( - queryset, None, required, widget, label, initial, help_text, - *args, **kwargs - ) + def __init__(self, queryset, **kwargs): + super().__init__(queryset, empty_label=None, **kwargs) def to_python(self, value): if not value: diff --git a/docs/ref/forms/fields.txt b/docs/ref/forms/fields.txt index 442ec58da9..473ce3d4e0 100644 --- a/docs/ref/forms/fields.txt +++ b/docs/ref/forms/fields.txt @@ -1006,7 +1006,7 @@ Slightly complex built-in ``Field`` classes from django.core.validators import RegexValidator class PhoneField(MultiValueField): - def __init__(self, *args, **kwargs): + def __init__(self, **kwargs): # Define one message for all fields. error_messages = { 'incomplete': 'Enter a country calling code and a phone number.', @@ -1030,7 +1030,7 @@ Slightly complex built-in ``Field`` classes ) super().__init__( error_messages=error_messages, fields=fields, - require_all_fields=False, *args, **kwargs + require_all_fields=False, **kwargs ) .. attribute:: MultiValueField.widget diff --git a/docs/releases/2.0.txt b/docs/releases/2.0.txt index d1ea0cef48..9c0f960d35 100644 --- a/docs/releases/2.0.txt +++ b/docs/releases/2.0.txt @@ -351,6 +351,19 @@ now prohibited, e.g.:: ... TypeError: Cannot reverse a query once a slice has been taken. +Form fields no longer accept optional arguments as positional arguments +----------------------------------------------------------------------- + +To help prevent runtime errors due to incorrect ordering of form field +arguments, optional arguments of built-in form fields are no longer accepted +as positional arguments. For example:: + + forms.IntegerField(25, 10) + +raises an exception and should be replaced with:: + + forms.IntegerField(max_value=25, min_value=10) + Miscellaneous ------------- diff --git a/tests/forms_tests/field_tests/test_charfield.py b/tests/forms_tests/field_tests/test_charfield.py index a59291e33a..d51f9d8007 100644 --- a/tests/forms_tests/field_tests/test_charfield.py +++ b/tests/forms_tests/field_tests/test_charfield.py @@ -73,7 +73,8 @@ class CharFieldTest(FormFieldAssertionsMixin, SimpleTestCase): CharField(min_length='a') with self.assertRaises(ValueError): CharField(max_length='a') - with self.assertRaises(ValueError): + msg = '__init__() takes 1 positional argument but 2 were given' + with self.assertRaisesMessage(TypeError, msg): CharField('a') def test_charfield_widget_attrs(self): diff --git a/tests/forms_tests/field_tests/test_multivaluefield.py b/tests/forms_tests/field_tests/test_multivaluefield.py index a644e4a278..82d51ac657 100644 --- a/tests/forms_tests/field_tests/test_multivaluefield.py +++ b/tests/forms_tests/field_tests/test_multivaluefield.py @@ -31,13 +31,13 @@ class ComplexMultiWidget(MultiWidget): class ComplexField(MultiValueField): - def __init__(self, required=True, widget=None, label=None, initial=None): + def __init__(self, **kwargs): fields = ( CharField(), MultipleChoiceField(choices=beatles), SplitDateTimeField(), ) - super().__init__(fields, required, widget, label, initial) + super().__init__(fields, **kwargs) def compress(self, data_list): if data_list: