Skip to content

Commit

Permalink
feat: Warn about ambiguous default values in parameter specifications (
Browse files Browse the repository at this point in the history
…#3283)

* Warn about annotated with default

* Fix test
  • Loading branch information
provinzkraut committed Mar 30, 2024
1 parent 4ed6018 commit 604e9e9
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 3 deletions.
24 changes: 22 additions & 2 deletions litestar/typing.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import warnings
from collections import abc, deque
from copy import deepcopy
from dataclasses import dataclass, is_dataclass, replace
Expand All @@ -21,7 +22,7 @@
from msgspec import UnsetType
from typing_extensions import NotRequired, Required, Self, get_args, get_origin, get_type_hints, is_typeddict

from litestar.exceptions import ImproperlyConfiguredException
from litestar.exceptions import ImproperlyConfiguredException, LitestarWarning
from litestar.openapi.spec import Example
from litestar.params import BodyKwarg, DependencyKwarg, KwargDefinition, ParameterKwarg
from litestar.types import Empty
Expand Down Expand Up @@ -512,12 +513,31 @@ def from_annotation(cls, annotation: Any, **kwargs: Any) -> FieldDefinition:
if isinstance(kwargs.get("default"), (KwargDefinition, DependencyKwarg)):
kwargs["kwarg_definition"] = kwargs.pop("default")
elif any(isinstance(v, (KwargDefinition, DependencyKwarg)) for v in metadata):
kwargs["kwarg_definition"] = next( # pragma: no cover
kwarg_definition = kwargs["kwarg_definition"] = next( # pragma: no cover
# see https://github.com/nedbat/coveragepy/issues/475
v
for v in metadata
if isinstance(v, (KwargDefinition, DependencyKwarg))
)
if kwarg_definition.default is not Empty:
warnings.warn(
f"Deprecated default value specification for annotation '{annotation}'. Setting defaults "
f"inside 'typing.Annotated' is discouraged and support for this will be removed in a future "
f"version. Defaults should be set with regular parameter default values. Use "
"'param: Annotated[<type>, Parameter(...)] = <default>' instead of "
"'param: Annotated[<type>, Parameter(..., default=<default>)].",
category=DeprecationWarning,
stacklevel=2,
)
if "default" in kwargs and kwarg_definition.default != kwargs["default"]:
warnings.warn(
f"Ambiguous default values for annotation '{annotation}'. The default value "
f"'{kwarg_definition.default!r}' set inside the parameter annotation differs from the "
f"parameter default value '{kwargs['default']!r}'",
category=LitestarWarning,
stacklevel=2,
)

metadata = tuple(v for v in metadata if not isinstance(v, (KwargDefinition, DependencyKwarg)))
elif (extra := kwargs.get("extra", {})) and "kwarg_definition" in extra:
kwargs["kwarg_definition"] = extra.pop("kwarg_definition")
Expand Down
13 changes: 12 additions & 1 deletion tests/unit/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
import pytest
from typing_extensions import Annotated, NotRequired, Required, TypedDict, get_type_hints

from litestar.params import DependencyKwarg, KwargDefinition, ParameterKwarg
from litestar.exceptions import LitestarWarning
from litestar.params import DependencyKwarg, KwargDefinition, Parameter, ParameterKwarg
from litestar.typing import FieldDefinition, _unpack_predicate

from .test_utils.test_signature import T, _check_field_definition, field_definition_int, test_type_hints
Expand Down Expand Up @@ -450,3 +451,13 @@ def test_field_definition_get_type_hints_dont_resolve_generics(
)
def test_unpack_predicate(predicate: Any, expected_meta: dict[str, Any]) -> None:
assert _unpack_predicate(predicate) == expected_meta


def test_warn_ambiguous_default_values() -> None:
with pytest.warns(LitestarWarning, match="Ambiguous default values"):
FieldDefinition.from_annotation(Annotated[int, Parameter(default=1)], default=2)


def test_warn_defaults_inside_parameter_definition() -> None:
with pytest.warns(DeprecationWarning, match="Deprecated default value specification"):
FieldDefinition.from_annotation(Annotated[int, Parameter(default=1)], default=1)

0 comments on commit 604e9e9

Please sign in to comment.