Fixed #4592: Made CheckboxSelectMultiple more like RadioSelect

I refactored RadioSelect and CheckboxSelectMultiple to
make them inherit from a base class, allowing them to share
the behavior of being able to iterate over their subwidgets.

Thanks to Matt McClanahan for the initial patch and to
Claude Paroz for the review.

4 files changed, 106 insertions, 58 deletions

diff --git a/django/forms/widgets.py b/django/forms/widgets.py
index b0e355eeb6..e72ab014b8 100644
--- a/django/forms/widgets.py
+++ b/django/forms/widgets.py
@@ -11,6 +11,7 @@ try:
     from urllib.parse import urljoin
 except ImportError:     # Python 2
     from urlparse import urljoin
+import warnings
 
 from django.conf import settings
 from django.forms.util import flatatt, to_current_timezone
@@ -585,14 +586,16 @@ class SelectMultiple(Select):
 
 
 @python_2_unicode_compatible
-class RadioInput(SubWidget):
+class ChoiceInput(SubWidget):
     """
-    An object used by RadioFieldRenderer that represents a single
-    <input type='radio'>.
+    An object used by ChoiceFieldRenderer that represents a single
+    <input type='$input_type'>.
     """
+    input_type = None  # Subclasses must define this
 
     def __init__(self, name, value, attrs, choice, index):
-        self.name, self.value = name, value
+        self.name = name
+        self.value = value
         self.attrs = attrs
         self.choice_value = force_text(choice[0])
         self.choice_label = force_text(choice[1])
@@ -609,8 +612,7 @@ class RadioInput(SubWidget):
             label_for = format_html(' for="{0}_{1}"', self.attrs['id'], self.index)
         else:
             label_for = ''
-        choice_label = force_text(self.choice_label)
-        return format_html('<label{0}>{1} {2}</label>', label_for, self.tag(), choice_label)
+        return format_html('<label{0}>{1} {2}</label>', label_for, self.tag(), self.choice_label)
 
     def is_checked(self):
         return self.value == self.choice_value
@@ -618,34 +620,69 @@ class RadioInput(SubWidget):
     def tag(self):
         if 'id' in self.attrs:
             self.attrs['id'] = '%s_%s' % (self.attrs['id'], self.index)
-        final_attrs = dict(self.attrs, type='radio', name=self.name, value=self.choice_value)
+        final_attrs = dict(self.attrs, type=self.input_type, name=self.name, value=self.choice_value)
         if self.is_checked():
             final_attrs['checked'] = 'checked'
         return format_html('<input{0} />', flatatt(final_attrs))
 
+
+class RadioChoiceInput(ChoiceInput):
+    input_type = 'radio'
+
+    def __init__(self, *args, **kwargs):
+        super(RadioChoiceInput, self).__init__(*args, **kwargs)
+        self.value = force_text(self.value)
+
+
+class RadioInput(RadioChoiceInput):
+    def __init__(self, *args, **kwargs):
+        msg = "RadioInput has been deprecated. Use RadioChoiceInput instead."
+        warnings.warn(msg, PendingDeprecationWarning, stacklevel=2)
+        super(RadioInput, self).__init__(*args, **kwargs)
+
+
+class CheckboxChoiceInput(ChoiceInput):
+    input_type = 'checkbox'
+
+    def __init__(self, *args, **kwargs):
+        super(CheckboxChoiceInput, self).__init__(*args, **kwargs)
+        self.value = set(force_text(v) for v in self.value)
+
+    def is_checked(self):
+        return self.choice_value in self.value
+
+
 @python_2_unicode_compatible
-class RadioFieldRenderer(object):
+class ChoiceFieldRenderer(object):
     """
     An object used by RadioSelect to enable customization of radio widgets.
     """
 
+    choice_input_class = None
+
     def __init__(self, name, value, attrs, choices):
-        self.name, self.value, self.attrs = name, value, attrs
+        self.name = name
+        self.value = value
+        self.attrs = attrs
         self.choices = choices
 
     def __iter__(self):
         for i, choice in enumerate(self.choices):
-            yield RadioInput(self.name, self.value, self.attrs.copy(), choice, i)
+            yield self.choice_input_class(self.name, self.value, self.attrs.copy(), choice, i)
 
     def __getitem__(self, idx):
         choice = self.choices[idx] # Let the IndexError propogate
-        return RadioInput(self.name, self.value, self.attrs.copy(), choice, idx)
+        return self.choice_input_class(self.name, self.value, self.attrs.copy(), choice, idx)
 
     def __str__(self):
         return self.render()
 
     def render(self):
-        """Outputs a <ul> for this set of radio fields."""
+        """
+        Outputs a <ul> for this set of choice fields.
+        If an id was given to the field, it is applied to the <ul> (each
+        item in the list will get an id of `$id_$i`).
+        """
         id_ = self.attrs.get('id', None)
         start_tag = format_html('<ul id="{0}">', id_) if id_ else '<ul>'
         output = [start_tag]
@@ -654,15 +691,25 @@ class RadioFieldRenderer(object):
         output.append('</ul>')
         return mark_safe('\n'.join(output))
 
-class RadioSelect(Select):
-    renderer = RadioFieldRenderer
+
+class RadioFieldRenderer(ChoiceFieldRenderer):
+    choice_input_class = RadioChoiceInput
+
+
+class CheckboxFieldRenderer(ChoiceFieldRenderer):
+    choice_input_class = CheckboxChoiceInput
+
+
+class RendererMixin(object):
+    renderer = None  # subclasses must define this
+    _empty_value = None
 
     def __init__(self, *args, **kwargs):
         # Override the default renderer if we were passed one.
         renderer = kwargs.pop('renderer', None)
         if renderer:
             self.renderer = renderer
-        super(RadioSelect, self).__init__(*args, **kwargs)
+        super(RendererMixin, self).__init__(*args, **kwargs)
 
     def subwidgets(self, name, value, attrs=None, choices=()):
         for widget in self.get_renderer(name, value, attrs, choices):
@@ -670,56 +717,35 @@ class RadioSelect(Select):
 
     def get_renderer(self, name, value, attrs=None, choices=()):
         """Returns an instance of the renderer."""
-        if value is None: value = ''
-        str_value = force_text(value) # Normalize to string.
+        if value is None:
+            value = self._empty_value
         final_attrs = self.build_attrs(attrs)
         choices = list(chain(self.choices, choices))
-        return self.renderer(name, str_value, final_attrs, choices)
+        return self.renderer(name, value, final_attrs, choices)
 
     def render(self, name, value, attrs=None, choices=()):
         return self.get_renderer(name, value, attrs, choices).render()
 
     def id_for_label(self, id_):
-        # RadioSelect is represented by multiple <input type="radio"> fields,
-        # each of which has a distinct ID. The IDs are made distinct by a "_X"
-        # suffix, where X is the zero-based index of the radio field. Thus,
-        # the label for a RadioSelect should reference the first one ('_0').
+        # Widgets using this RendererMixin are made of a collection of
+        # subwidgets, each with their own <label>, and distinct ID.
+        # The IDs are made distinct by y "_X" suffix, where X is the zero-based
+        # index of the choice field. Thus, the label for the main widget should
+        # reference the first subwidget, hence the "_0" suffix.
         if id_:
             id_ += '_0'
         return id_
 
-class CheckboxSelectMultiple(SelectMultiple):
-    def render(self, name, value, attrs=None, choices=()):
-        if value is None: value = []
-        final_attrs = self.build_attrs(attrs, name=name)
-        id_ = final_attrs.get('id', None)
-        start_tag = format_html('<ul id="{0}">', id_) if id_ else '<ul>'
-        output = [start_tag]
-        # Normalize to strings
-        str_values = set([force_text(v) for v in value])
-        for i, (option_value, option_label) in enumerate(chain(self.choices, choices)):
-            # If an ID attribute was given, add a numeric index as a suffix,
-            # so that the checkboxes don't all have the same ID attribute.
-            if id_:
-                final_attrs = dict(final_attrs, id='%s_%s' % (id_, i))
-                label_for = format_html(' for="{0}_{1}"', id_, i)
-            else:
-                label_for = ''
 
-            cb = CheckboxInput(final_attrs, check_test=lambda value: value in str_values)
-            option_value = force_text(option_value)
-            rendered_cb = cb.render(name, option_value)
-            option_label = force_text(option_label)
-            output.append(format_html('<li><label{0}>{1} {2}</label></li>',
-                                      label_for, rendered_cb, option_label))
-        output.append('</ul>')
-        return mark_safe('\n'.join(output))
+class RadioSelect(RendererMixin, Select):
+    renderer = RadioFieldRenderer
+    _empty_value = ''
+
+
+class CheckboxSelectMultiple(RendererMixin, SelectMultiple):
+    renderer = CheckboxFieldRenderer
+    _empty_value = []
 
-    def id_for_label(self, id_):
-        # See the comment for RadioSelect.id_for_label()
-        if id_:
-            id_ += '_0'
-        return id_
 
 class MultiWidget(Widget):
     """
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index bf1a323489..1533e25dc8 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -372,6 +372,9 @@ these changes.
   - ``django.db.transaction.is_managed()``
   - ``django.db.transaction.managed()``
 
+* ``django.forms.widgets.RadioInput`` will be removed in favor of
+  ``django.forms.widgets.RadioChoiceInput``.
+
 2.0
 ---
 
diff --git a/docs/ref/forms/widgets.txt b/docs/ref/forms/widgets.txt
index 836389125a..514e8b3dc0 100644
--- a/docs/ref/forms/widgets.txt
+++ b/docs/ref/forms/widgets.txt
@@ -658,6 +658,9 @@ the widget.
 The outer ``<ul>`` container will now receive the ``id`` attribute defined on
 the widget.
 
+Like :class:`RadioSelect`, you can now loop over the individual checkboxes making
+up the lists. See the documentation of :class:`RadioSelect` for more details.
+
 .. _file-upload-widgets:
 
 File upload widgets
diff --git a/tests/forms_tests/tests/test_widgets.py b/tests/forms_tests/tests/test_widgets.py
index 3c782713f3..4664553aa7 100644
--- a/tests/forms_tests/tests/test_widgets.py
+++ b/tests/forms_tests/tests/test_widgets.py
@@ -15,7 +15,7 @@ from django.utils import six
 from django.utils.translation import activate, deactivate
 from django.test import TestCase
 from django.test.utils import override_settings
-from django.utils.encoding import python_2_unicode_compatible
+from django.utils.encoding import python_2_unicode_compatible, force_text
 
 from ..models import Article
 
@@ -656,7 +656,7 @@ beatle J R Ringo False""")
 <label><input checked="checked" type="radio" name="beatle" value="G" /> George</label><br />
 <label><input type="radio" name="beatle" value="R" /> Ringo</label>""")
 
-        # A RadioFieldRenderer object also allows index access to individual RadioInput
+        # A RadioFieldRenderer object also allows index access to individual RadioChoiceInput
         w = RadioSelect()
         r = w.get_renderer('beatle', 'J', choices=(('J', 'John'), ('P', 'Paul'), ('G', 'George'), ('R', 'Ringo')))
         self.assertHTMLEqual(str(r[1]), '<label><input type="radio" name="beatle" value="P" /> Paul</label>')
@@ -665,11 +665,8 @@ beatle J R Ringo False""")
         self.assertFalse(r[1].is_checked())
         self.assertEqual((r[1].name, r[1].value, r[1].choice_value, r[1].choice_label), ('beatle', 'J', 'P', 'Paul'))
 
-        try:
+        with self.assertRaises(IndexError):
             r[10]
-            self.fail("This offset should not exist.")
-        except IndexError:
-            pass
 
         # Choices are escaped correctly
         w = RadioSelect()
@@ -817,6 +814,25 @@ beatle J R Ringo False""")
 <li><label for="abc_2"><input checked="checked" type="checkbox" name="letters" value="c" id="abc_2" /> C</label></li>
 </ul>""")
 
+        w = CheckboxSelectMultiple()
+        r = w.get_renderer('abc', 'b', choices=[(c, c.upper()) for c in 'abc'])
+        # You can iterate over the CheckboxFieldRenderer to get individual elements
+        expected = [
+            '<label><input type="checkbox" name="abc" value="a" /> A</label>',
+            '<label><input checked="checked" type="checkbox" name="abc" value="b" /> B</label>',
+            '<label><input type="checkbox" name="abc" value="c" /> C</label>',
+        ]
+        for output, expected in zip(r, expected):
+            self.assertHTMLEqual(force_text(output), expected)
+
+        # You can access individual elements
+        self.assertHTMLEqual(force_text(r[1]),
+            '<label><input checked="checked" type="checkbox" name="abc" value="b" /> B</label>')
+
+        # Out-of-range errors are propagated
+        with self.assertRaises(IndexError):
+            r[42]
+
     def test_multi(self):
         class MyMultiWidget(MultiWidget):
             def decompress(self, value):