Fixed #30995 -- Allowed converter.to_url() to raise ValueError to indicate no match.

5 files changed, 57 insertions, 10 deletions

diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py
index 120e0396af..6e3f2443c9 100644
--- a/django/urls/resolvers.py
+++ b/django/urls/resolvers.py
@@ -632,11 +632,18 @@ class URLResolver:
                     candidate_subs = kwargs
                 # Convert the candidate subs to text using Converter.to_url().
                 text_candidate_subs = {}
+                match = True
                 for k, v in candidate_subs.items():
                     if k in converters:
-                        text_candidate_subs[k] = converters[k].to_url(v)
+                        try:
+                            text_candidate_subs[k] = converters[k].to_url(v)
+                        except ValueError:
+                            match = False
+                            break
                     else:
                         text_candidate_subs[k] = str(v)
+                if not match:
+                    continue
                 # WSGI provides decoded URLs, without %xx escapes, and the URL
                 # resolver operates on such URLs. First substitute arguments
                 # without quoting to build a decoded URL and look for a match.
diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt
index cdacbd71cd..3def8920cc 100644
--- a/docs/releases/3.1.txt
+++ b/docs/releases/3.1.txt
@@ -294,7 +294,8 @@ Tests
 URLs
 ~~~~
 
-* ...
+* :ref:`Path converters <registering-custom-path-converters>` can now raise
+  ``ValueError`` in ``to_url()`` to indicate no match when reversing URLs.
 
 Utilities
 ~~~~~~~~~
diff --git a/docs/topics/http/urls.txt b/docs/topics/http/urls.txt
index ef9d4c007c..efe61d041c 100644
--- a/docs/topics/http/urls.txt
+++ b/docs/topics/http/urls.txt
@@ -156,7 +156,14 @@ A converter is a class that includes the following:
   user unless another URL pattern matches.
 
 * A ``to_url(self, value)`` method, which handles converting the Python type
-  into a string to be used in the URL.
+  into a string to be used in the URL. It should raise ``ValueError`` if it
+  can't convert the given value. A ``ValueError`` is interpreted as no match
+  and as a consequence :func:`~django.urls.reverse` will raise
+  :class:`~django.urls.NoReverseMatch` unless another URL pattern matches.
+
+  .. versionchanged:: 3.1
+
+    Support for raising ``ValueError`` to indicate no match was added.
 
 For example::
 
@@ -666,7 +673,9 @@ included at all).
 
 You may also use the same name for multiple URL patterns if they differ in
 their arguments. In addition to the URL name, :func:`~django.urls.reverse()`
-matches the number of arguments and the names of the keyword arguments.
+matches the number of arguments and the names of the keyword arguments. Path
+converters can also raise ``ValueError`` to indicate no match, see
+:ref:`registering-custom-path-converters` for details.
 
 .. _topics-http-defining-url-namespaces:
 
diff --git a/tests/urlpatterns/path_same_name_urls.py b/tests/urlpatterns/path_same_name_urls.py
index 8eee949316..7b4fd2e627 100644
--- a/tests/urlpatterns/path_same_name_urls.py
+++ b/tests/urlpatterns/path_same_name_urls.py
@@ -1,6 +1,8 @@
-from django.urls import path, re_path
+from django.urls import path, re_path, register_converter
 
-from . import views
+from . import converters, views
+
+register_converter(converters.DynamicConverter, 'to_url_value_error')
 
 urlpatterns = [
     # Different number of arguments.
@@ -18,4 +20,15 @@ urlpatterns = [
     # Different regular expressions.
     re_path(r'^regex/uppercase/([A-Z]+)/', views.empty_view, name='regex'),
     re_path(r'^regex/lowercase/([a-z]+)/', views.empty_view, name='regex'),
+    # converter.to_url() raises ValueError (no match).
+    path(
+        'converter_to_url/int/<value>/',
+        views.empty_view,
+        name='converter_to_url',
+    ),
+    path(
+        'converter_to_url/tiny_int/<to_url_value_error:value>/',
+        views.empty_view,
+        name='converter_to_url',
+    ),
 ]
diff --git a/tests/urlpatterns/tests.py b/tests/urlpatterns/tests.py
index 95bea58339..0de0f6f05d 100644
--- a/tests/urlpatterns/tests.py
+++ b/tests/urlpatterns/tests.py
@@ -3,7 +3,7 @@ import uuid
 from django.core.exceptions import ImproperlyConfigured
 from django.test import SimpleTestCase
 from django.test.utils import override_settings
-from django.urls import Resolver404, path, resolve, reverse
+from django.urls import NoReverseMatch, Resolver404, path, resolve, reverse
 
 from .converters import DynamicConverter
 from .views import empty_view
@@ -203,6 +203,12 @@ class ConverterTests(SimpleTestCase):
 @override_settings(ROOT_URLCONF='urlpatterns.path_same_name_urls')
 class SameNameTests(SimpleTestCase):
     def test_matching_urls_same_name(self):
+        @DynamicConverter.register_to_url
+        def requires_tiny_int(value):
+            if value > 5:
+                raise ValueError
+            return value
+
         tests = [
             ('number_of_args', [
                 ([], {}, '0/'),
@@ -227,6 +233,10 @@ class SameNameTests(SimpleTestCase):
                 (['ABC'], {}, 'uppercase/ABC/'),
                 (['abc'], {}, 'lowercase/abc/'),
             ]),
+            ('converter_to_url', [
+                ([6], {}, 'int/6/'),
+                ([1], {}, 'tiny_int/1/'),
+            ]),
         ]
         for url_name, cases in tests:
             for args, kwargs, url_suffix in cases:
@@ -272,9 +282,16 @@ class ConversionExceptionTests(SimpleTestCase):
         with self.assertRaisesMessage(TypeError, 'This type error propagates.'):
             resolve('/dynamic/abc/')
 
-    def test_reverse_value_error_propagates(self):
+    def test_reverse_value_error_means_no_match(self):
         @DynamicConverter.register_to_url
         def raises_value_error(value):
-            raise ValueError('This value error propagates.')
-        with self.assertRaisesMessage(ValueError, 'This value error propagates.'):
+            raise ValueError
+        with self.assertRaises(NoReverseMatch):
+            reverse('dynamic', kwargs={'value': object()})
+
+    def test_reverse_type_error_propagates(self):
+        @DynamicConverter.register_to_url
+        def raises_type_error(value):
+            raise TypeError('This type error propagates.')
+        with self.assertRaisesMessage(TypeError, 'This type error propagates.'):
             reverse('dynamic', kwargs={'value': object()})