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

[feature request] FastAPI rule: prefer Annotated for Query, Path, Body, Cookie #12913

Closed
tiangolo opened this issue Aug 16, 2024 · 9 comments · Fixed by #15462
Closed

[feature request] FastAPI rule: prefer Annotated for Query, Path, Body, Cookie #12913

tiangolo opened this issue Aug 16, 2024 · 9 comments · Fixed by #15462
Labels
rule Implementing or modifying a lint rule

Comments

@tiangolo
Copy link

tiangolo commented Aug 16, 2024

In short

I would like to have a FastAPI rule to prefer Annotated for:

  • Body
  • Cookie
  • File
  • Form
  • Header
  • Path
  • Query

and hopefully autofix them.

Depends and Security are already supported.

This would be related to/continuation of #11579

Description

I would like to have Annotated as the recommended style/syntax for FastAPI params that support it:

  • Body
  • Cookie
  • File
  • Form
  • Header
  • Path
  • Query

This is very similar to the rule for Depends and Security implemented here: #11579

The only caveat is that if one of these parameters has a default argument (or the first argument), as in Query(default="foo") then that would become the new default value. For example:

@app.post("/stuff/")
def do_stuff(
    some_query_param: str | None = Query(default=None),
    some_path_param: str = Path(),
    some_body_param: str = Body("foo"),
    some_cookie_param: str = Cookie(),
    some_header_param: int = Header(default=5),
    some_file_param: UploadFile = File(),
    some_form_param: str = Form(),
):
    # do stuff
    pass

would ideally be transformed into:

@app.post("/stuff/")
def do_stuff(
    some_path_param: Annotated[str, Path()],
    some_cookie_param: Annotated[str, Cookie()],
    some_file_param: Annotated[UploadFile, File()],
    some_form_param: Annotated[str, Form()],
    some_query_param: Annotated[str | None, Query()] = None,
    some_body_param: Annotated[str, Body()] = "foo",
    some_header_param: Annotated[int, Header()] = 5,
):
    # do stuff
    pass

or:

@app.post("/stuff/")
def do_stuff(
   *,
    some_query_param: Annotated[str | None, Query()] = None,
    some_path_param: Annotated[str, Path()],
    some_body_param: Annotated[str, Body()] = "foo",
    some_cookie_param: Annotated[str, Cookie()],
    some_header_param: Annotated[int, Header()] = 5,
    some_file_param: Annotated[UploadFile, File()],
    some_form_param: Annotated[str, Form()],
):
    # do stuff
    pass
@charliermarsh charliermarsh added rule Implementing or modifying a lint rule help wanted Contributions especially welcome labels Aug 16, 2024
@amirsoroush
Copy link

amirsoroush commented Aug 16, 2024

Have a question though:
The ideal form currently has invalid syntax as some non-default parameters follow the default parameters. Does that mean Ruff is also going to change the order of the parameters to fix it? In that case the dependencies are going to be resolved in different order.
I usually tend to put the (mostly custom one with Depends not Query and Body etc.) dependencies that would fail fast at the beginning.
What would be the behavior of Ruff? Is that something I should concern about at all?
Thanks.

@tiangolo
Copy link
Author

Right! I just updated the expected result. 🤓

@kiran-4444
Copy link
Contributor

I’d love to take this up!

@RevKraft
Copy link

I can give some help on that and team up with other people

@MichaReiser
Copy link
Member

Thanks for suggesting this rule. Is there some resource that explains why using Annotated is preferred? It's probably a silly question but for someone who never used fastapi (or Python) a lot, it primarily looks more verbose ;)

@TomerBin
Copy link
Contributor

Thanks for suggesting this rule. Is there some resource that explains why using Annotated is preferred? It's probably a silly question but for someone who never used fastapi (or Python) a lot, it primarily looks more verbose ;)

Here it is https://fastapi.tiangolo.com/tutorial/dependencies/#share-annotated-dependencies

@MichaReiser
Copy link
Member

Thanks, I think this is the important section (we should link it in the rule) https://fastapi.tiangolo.com/tutorial/query-params-str-validations/#advantages-of-annotated

@TomerBin
Copy link
Contributor

We should already support all of the mentioned params (Query, File, etc..). Can you please attach your code sample where it doesn't work as expected? I put your example in the Ruff playground (with the needed imports) and it seems to work.

Regarding the default value handling - I really liked @tiangolo suggestion of adding * when needed.

@tiangolo
Copy link
Author

We should already support all of the mentioned params (Query, File, etc..). Can you please attach your code sample where it doesn't work as expected? I put your example in the Ruff playground (with the needed imports) and it seems to work.

Indeed! 😱 That's amazing! I assumed it didn't work, but it actually does! 🚀

From:

@app.post("/stuff/")
def do_stuff(
    some_query_param: str | None = Query(default=None),
    some_path_param: str = Path(),
    some_body_param: str = Body("foo"),
    some_cookie_param: str = Cookie(),
    some_header_param: int = Header(default=5),
    some_file_param: UploadFile = File(),
    some_form_param: str = Form(),
):
    # do stuff
    pass

to:

@app.post("/stuff/")
def do_stuff(
    some_query_param: Annotated[str | None, Query(default=None)],
    some_path_param: Annotated[str, Path()],
    some_body_param: Annotated[str, Body("foo")],
    some_cookie_param: Annotated[str, Cookie()],
    some_header_param: Annotated[int, Header(default=5)],
    some_file_param: Annotated[UploadFile, File()],
    some_form_param: Annotated[str, Form()],
):
    # do stuff
    pass

The only piece missing would be to move the default outside of Annotated. Maybe with *.

@MichaReiser MichaReiser removed the help wanted Contributions especially welcome label Dec 2, 2024
@ntBre ntBre closed this as completed in 1a77a75 Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants