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

Define strict return value typing for Depends and Security #11255

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JaniM
Copy link

@JaniM JaniM commented Mar 7, 2024

Depends can be used with a direct form, which although not recommended, is still supported. The following code is problematic wrt. type checking.

def dependency() -> str:
    return "dependency"

@router.get("/")
def endpoint(value: int = Depends(dependency)):
    pass

Currently, Depends(dependency) returns Any, which then happily gets assigned to the int. This patch changes it to return a type matching what the dependency returns, making the above code fail type checking with mypy as it should.

@JaniM JaniM force-pushed the push-ukwmvrqmqpyo branch 3 times, most recently from e365b6e to a910d87 Compare March 7, 2024 11:52
Copy link

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good fix!

I would only change the title and description to make things clearer.
In fact, this fix does not cause FastAPI to show any errors if the returned dependency type does not match the parameter type. But this fix allows you to find this error using mypy.

With this fix, 'mypy' will show error: Incompatible default for argument "value" (default has type "str", argument has type "int") [assignment].

UPD: why don't you add the same to the Security as well?

@JaniM
Copy link
Author

JaniM commented Mar 7, 2024

I didn't touch Security, as I have never used it. Wasn't sure if it's used the same way or has different requirements.

Edit: Made the change.

@JaniM JaniM changed the title Define strict return value typing for Depends Define strict return value typing for Depends and Security Mar 7, 2024
@JaniM JaniM requested a review from YuriiMotov March 7, 2024 15:52
@JaniM
Copy link
Author

JaniM commented Mar 8, 2024

The checks seem to be stuck, can someone give them a nudge?

fastapi/param_functions.py Outdated Show resolved Hide resolved
Depends can be used with a direct form, which although not recommended, is
still supported. The following code is problematic wrt. type checking.

    def dependency() -> str:
        return "dependency"

    @router.get("/")
    def endpoint(value: int = Depends(dependency)):
      pass

Currently, `Depends(dependency)` returns Any, which then happily gets assigned
to the int. This patch changes it to return a type matching what the dependency
returns, making the above code fail type checking with mypy as it should.
Copy link

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good useful fix!

@jsimonlane
Copy link

Good fix! Hoping this gets merged soon...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants