From d89053585e11e869efcc9debb1c311b47b5e20ea Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Fri, 19 Jul 2019 13:55:32 -0700 Subject: [PATCH] Improved error message when index in __getitem__() is invalid. --- django/core/paginator.py | 5 ++++- django/db/models/query.py | 5 ++++- django/forms/boundfield.py | 5 ++++- tests/forms_tests/tests/test_forms.py | 9 +++++++++ tests/pagination/tests.py | 3 ++- tests/queries/tests.py | 5 +++++ 6 files changed, 28 insertions(+), 4 deletions(-) diff --git a/django/core/paginator.py b/django/core/paginator.py index d484c31ed5..73dfcd679f 100644 --- a/django/core/paginator.py +++ b/django/core/paginator.py @@ -151,7 +151,10 @@ class Page(collections.abc.Sequence): def __getitem__(self, index): if not isinstance(index, (int, slice)): - raise TypeError + raise TypeError( + 'Page indices must be integers or slices, not %s.' + % type(index).__name__ + ) # The object_list is converted to a list so that if it was a QuerySet # it won't be a database hit per __getitem__. if not isinstance(self.object_list, list): diff --git a/django/db/models/query.py b/django/db/models/query.py index 1544beb613..a62947d9d0 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -283,7 +283,10 @@ class QuerySet: def __getitem__(self, k): """Retrieve an item or slice from the set of results.""" if not isinstance(k, (int, slice)): - raise TypeError + raise TypeError( + 'QuerySet indices must be integers or slices, not %s.' + % type(k).__name__ + ) assert ((not isinstance(k, slice) and (k >= 0)) or (isinstance(k, slice) and (k.start is None or k.start >= 0) and (k.stop is None or k.stop >= 0))), \ diff --git a/django/forms/boundfield.py b/django/forms/boundfield.py index 7f40fbbbb4..8832169021 100644 --- a/django/forms/boundfield.py +++ b/django/forms/boundfield.py @@ -63,7 +63,10 @@ class BoundField: # Prevent unnecessary reevaluation when accessing BoundField's attrs # from templates. if not isinstance(idx, (int, slice)): - raise TypeError + raise TypeError( + 'BoundField indices must be integers or slices, not %s.' + % type(idx).__name__ + ) return self.subwidgets[idx] @property diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py index 18fb4a94de..45f5405fee 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -746,6 +746,15 @@ Java [str(bf[1]), str(bf[2]), str(bf[3])], ) + def test_boundfield_invalid_index(self): + class TestForm(Form): + name = ChoiceField(choices=[]) + + field = TestForm()['name'] + msg = 'BoundField indices must be integers or slices, not str.' + with self.assertRaisesMessage(TypeError, msg): + field['foo'] + def test_boundfield_bool(self): """BoundField without any choices (subwidgets) evaluates to True.""" class TestForm(Form): diff --git a/tests/pagination/tests.py b/tests/pagination/tests.py index ef5108e42b..95310d26bd 100644 --- a/tests/pagination/tests.py +++ b/tests/pagination/tests.py @@ -366,7 +366,8 @@ class ModelPaginationTests(TestCase): # Make sure object_list queryset is not evaluated by an invalid __getitem__ call. # (this happens from the template engine when using eg: {% page_obj.has_previous %}) self.assertIsNone(p.object_list._result_cache) - with self.assertRaises(TypeError): + msg = 'Page indices must be integers or slices, not str.' + with self.assertRaisesMessage(TypeError, msg): p['has_previous'] self.assertIsNone(p.object_list._result_cache) self.assertNotIsInstance(p.object_list, list) diff --git a/tests/queries/tests.py b/tests/queries/tests.py index c49a0231ac..6d3dcbdb1f 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2436,6 +2436,11 @@ class QuerySetSupportsPythonIdioms(TestCase): with self.assertRaisesMessage(AssertionError, "Negative indexing is not supported."): Article.objects.all()[0:-5] + def test_invalid_index(self): + msg = 'QuerySet indices must be integers or slices, not str.' + with self.assertRaisesMessage(TypeError, msg): + Article.objects.all()['foo'] + def test_can_get_number_of_items_in_queryset_using_standard_len(self): self.assertEqual(len(Article.objects.filter(name__exact='Article 1')), 1)