From 6ea159be44de78cf3c5f886a8f6427abc790eef8 Mon Sep 17 00:00:00 2001 From: David Szotten Date: Sat, 3 Jul 2021 08:41:19 +0000 Subject: [PATCH] Use `Sequence` instead of `List` for `path` param Unlike `List`, which is invariant, `Sequence` is covariant, which lets `path` accept lists of subsets of the `Union` as well. I believe this is safe, as django doesn't mutate this input. I found [this comment](https://github.com/python/mypy/issues/3351#issuecomment-300447832) helpful --- django-stubs/conf/urls/__init__.pyi | 6 +++--- django-stubs/urls/conf.pyi | 8 ++++---- tests/typecheck/urls/test_conf.yml | 10 ++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/django-stubs/conf/urls/__init__.pyi b/django-stubs/conf/urls/__init__.pyi index 384ab9e09..2ea5668c7 100644 --- a/django-stubs/conf/urls/__init__.pyi +++ b/django-stubs/conf/urls/__init__.pyi @@ -1,5 +1,5 @@ # Stubs for django.conf.urls (Python 3.5) -from typing import Any, Callable, Dict, List, Optional, overload, Tuple, Union +from typing import Any, Callable, Dict, Optional, overload, Sequence, Tuple, Union from django.http.response import HttpResponse, HttpResponseBase @@ -10,7 +10,7 @@ handler403: Union[str, Callable[..., HttpResponse]] = ... handler404: Union[str, Callable[..., HttpResponse]] = ... handler500: Union[str, Callable[..., HttpResponse]] = ... -IncludedURLConf = Tuple[List[Union[URLResolver, URLPattern]], Optional[str], Optional[str]] +IncludedURLConf = Tuple[Sequence[Union[URLResolver, URLPattern]], Optional[str], Optional[str]] def include(arg: Any, namespace: str = ..., app_name: str = ...) -> IncludedURLConf: ... @overload @@ -21,5 +21,5 @@ def url( def url(regex: str, view: IncludedURLConf, kwargs: Dict[str, Any] = ..., name: str = ...) -> URLResolver: ... @overload def url( - regex: str, view: List[Union[URLResolver, str]], kwargs: Dict[str, Any] = ..., name: str = ... + regex: str, view: Sequence[Union[URLResolver, str]], kwargs: Dict[str, Any] = ..., name: str = ... ) -> URLResolver: ... diff --git a/django-stubs/urls/conf.pyi b/django-stubs/urls/conf.pyi index 638447f84..d8251ba92 100644 --- a/django-stubs/urls/conf.pyi +++ b/django-stubs/urls/conf.pyi @@ -1,4 +1,4 @@ -from typing import Any, List, Optional, Tuple, overload, Callable, Dict, Union +from typing import Any, Optional, Sequence, Tuple, overload, Callable, Dict, Union from .resolvers import URLResolver, URLPattern from ..conf.urls import IncludedURLConf @@ -6,7 +6,7 @@ from ..http.response import HttpResponseBase def include( arg: Any, namespace: Optional[str] = ... -) -> Tuple[List[Union[URLResolver, URLPattern]], Optional[str], Optional[str]]: ... +) -> Tuple[Sequence[Union[URLResolver, URLPattern]], Optional[str], Optional[str]]: ... # path() @overload @@ -17,7 +17,7 @@ def path( def path(route: str, view: IncludedURLConf, kwargs: Dict[str, Any] = ..., name: str = ...) -> URLResolver: ... @overload def path( - route: str, view: List[Union[URLResolver, str]], kwargs: Dict[str, Any] = ..., name: str = ... + route: str, view: Sequence[Union[URLResolver, str]], kwargs: Dict[str, Any] = ..., name: str = ... ) -> URLResolver: ... # re_path() @@ -29,5 +29,5 @@ def re_path( def re_path(route: str, view: IncludedURLConf, kwargs: Dict[str, Any] = ..., name: str = ...) -> URLResolver: ... @overload def re_path( - route: str, view: List[Union[URLResolver, str]], kwargs: Dict[str, Any] = ..., name: str = ... + route: str, view: Sequence[Union[URLResolver, str]], kwargs: Dict[str, Any] = ..., name: str = ... ) -> URLResolver: ... diff --git a/tests/typecheck/urls/test_conf.yml b/tests/typecheck/urls/test_conf.yml index f03341dda..047264dd1 100644 --- a/tests/typecheck/urls/test_conf.yml +++ b/tests/typecheck/urls/test_conf.yml @@ -6,3 +6,13 @@ def include() -> Tuple[List[Union[URLPattern, URLResolver]], None, None]: ... path('test/', include()) + + +- case: test_path_accepts_pattern_resolver_union_subset + main: | + from typing import List, Tuple + from django.urls import path, URLPattern + + def include() -> Tuple[List[URLPattern], None, None]: ... + + path('test/', include())