Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typing.Annotated should be considered a dangerous default #10093

Open
lattwood opened this issue Nov 22, 2024 · 1 comment
Open

typing.Annotated should be considered a dangerous default #10093

lattwood opened this issue Nov 22, 2024 · 1 comment
Labels
Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. Proposal 📨

Comments

@lattwood
Copy link

lattwood commented Nov 22, 2024

Current problem

If a programmer accidentally uses typing.Annotated as a default value (not as a default type), pylint doesn't complain.

Ex, you're converting some FastAPI code from plain Depends to Annotated and make a typo, like below:

Original:

def some_method(
    session: AsyncSession = Depends(get_session_read),
) -> None:
    ...

Brainfart:

def some_method(
    session= typing.Annotated[AsyncSession, Depends(get_session_read)],
) -> None:
    ...

What you're actually supposed to do:

def some_method(
    session: typing.Annotated[AsyncSession, Depends(get_session_read)],
) -> None:
    ...

Desired solution

PyLint should make me feel bad about myself for making this typo. /s

@lattwood lattwood added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Nov 22, 2024
@Pierre-Sassoulas Pierre-Sassoulas added Proposal 📨 Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Nov 23, 2024
@Pierre-Sassoulas
Copy link
Member

Thank you for opening the issue. I'm not sure we can easily create a checker for this as it's possible to use typing classes as default value for function argument voluntarily (meta-programming or advanced use cases?).

I.e. something like this:

def some_meta_method(
    typing_type: Type[typing.Annotated] = typing.Annotated[AsyncSession, Depends(get_session_read)],
) -> None:
    ...

How would you deal with that ? Raise only if there's no defined type for typing_type ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. Proposal 📨
Projects
None yet
Development

No branches or pull requests

2 participants