astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.41k stars 1.08k forks source link

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

Open tiangolo opened 2 months ago

tiangolo commented 2 months ago

In short

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

and hopefully autofix them.

Depends and Security are already supported.

This would be related to/continuation of https://github.com/astral-sh/ruff/pull/11579

Description

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

This is very similar to the rule for Depends and Security implemented here: https://github.com/astral-sh/ruff/pull/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
amirsoroush commented 2 months ago

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 commented 2 months ago

Right! I just updated the expected result. :nerd_face:

kiran-4444 commented 2 months ago

I’d love to take this up!

RevKraft commented 2 months ago

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

MichaReiser commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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! :scream: That's amazing! I assumed it didn't work, but it actually does! :rocket:

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 *.