Improve docs on fix safety for FAST002 #14484
Labels
documentation
Improvements or additions to documentation
Comments
|
At the time it was merged, we weren't handling defaults correctly. If I remember correctly, an improvement was deployed later so we need to check if it's still the case. |
ntBre
added a commit
that referenced
this issue
Jan 15, 2025
## Summary The initial purpose was to fix #15043, where code like this: ```python from fastapi import FastAPI, Query app = FastAPI() @app.get("/test") def handler(echo: str = Query("")): return echo ``` was being fixed to the invalid code below: ```python from typing import Annotated from fastapi import FastAPI, Query app = FastAPI() @app.get("/test") def handler(echo: Annotated[str, Query("")]): # changed return echo ``` As @MichaReiser pointed out, the correct fix is: ```python from typing import Annotated from fastapi import FastAPI, Query app = FastAPI() @app.get("/test") def handler(echo: Annotated[str, Query()] = ""): # changed return echo ``` After fixing the issue for `Query`, I realized that other classes like `Path`, `Body`, `Cookie`, `Header`, `File`, and `Form` also looked susceptible to this issue. The last few commits should handle these too, which I think means this will also close #12913. I had to reorder the arguments to the `do_stuff` test case because the new fix removes some default argument values (eg for `Path`: `some_path_param: str = Path()` becomes `some_path_param: Annotated[str, Path()]`). There's also #14484 related to this rule. I'm happy to take a stab at that here or in a follow up PR too. ## Test Plan `cargo test` I also checked the fixed output with `uv run --with fastapi FAST002_0.py`, but it required making a bunch of additional changes to the test file that I wasn't sure we wanted in this PR. --------- Co-authored-by: Micha Reiser <micha@reiser.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The fix for FAST002 is marked as always unsafe, but there isn't a
## Fix safetysection in the documentation explaining why it is marked as unsafe.I can think of an obvious reason why the fix might be unsafe on Python 3.8 and lower: the fix adds imports of
typing_extensionson Python 3.8 and lower, while the user's code might not havetyping_extensionslisted as a dependency. However, it's not immediately obvious to me (as a non-expert when it comes tofastAPI) what the exact reasons are for the fix being marked as unsafe on Python 3.9+.It would be great to add better docs here. Cc. @TomerBin, who implemented the rule in #11579 and @charliermarsh, who merged it.
The text was updated successfully, but these errors were encountered: