Skip to content

Commit a802d7a

Browse files
[fastapi] Handle ellipsis defaults in FAST002 autofix (FAST002) (#20810)
## Summary Implement handling of ellipsis (`...`) defaults in the `FAST002` autofix to correctly differentiate between required and optional parameters in FastAPI route definitions. Previously, the autofix did not properly handle cases where parameters used `...` as a default value (to indicate required parameters). This could lead to incorrect transformations when applying the autofix. This change updates the `FAST002` autofix logic to: - Correctly recognize `...` as a valid FastAPI required default. - Preserve the semantics of required parameters while still applying other autofix improvements. - Avoid incorrectly substituting or removing ellipsis defaults. Fixes #20800 ## Test Plan Added a new test fixture at: ```crates/ruff_linter/resources/test/fixtures/fastapi/FAST002_2.py```
1 parent 692b7d7 commit a802d7a

File tree

5 files changed

+725
-7
lines changed

5 files changed

+725
-7
lines changed
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
"""Test FAST002 ellipsis handling."""
2+
3+
from fastapi import Body, Cookie, FastAPI, Header, Query
4+
5+
app = FastAPI()
6+
7+
8+
# Cases that should be fixed - ellipsis should be removed
9+
10+
11+
@app.get("/test1")
12+
async def test_ellipsis_query(
13+
# This should become: param: Annotated[str, Query(description="Test param")]
14+
param: str = Query(..., description="Test param"),
15+
) -> str:
16+
return param
17+
18+
19+
@app.get("/test2")
20+
async def test_ellipsis_header(
21+
# This should become: auth: Annotated[str, Header(description="Auth header")]
22+
auth: str = Header(..., description="Auth header"),
23+
) -> str:
24+
return auth
25+
26+
27+
@app.post("/test3")
28+
async def test_ellipsis_body(
29+
# This should become: data: Annotated[dict, Body(description="Request body")]
30+
data: dict = Body(..., description="Request body"),
31+
) -> dict:
32+
return data
33+
34+
35+
@app.get("/test4")
36+
async def test_ellipsis_cookie(
37+
# This should become: session: Annotated[str, Cookie(description="Session ID")]
38+
session: str = Cookie(..., description="Session ID"),
39+
) -> str:
40+
return session
41+
42+
43+
@app.get("/test5")
44+
async def test_simple_ellipsis(
45+
# This should become: id: Annotated[str, Query()]
46+
id: str = Query(...),
47+
) -> str:
48+
return id
49+
50+
51+
@app.get("/test6")
52+
async def test_multiple_kwargs_with_ellipsis(
53+
# This should become: param: Annotated[str, Query(description="Test", min_length=1, max_length=10)]
54+
param: str = Query(..., description="Test", min_length=1, max_length=10),
55+
) -> str:
56+
return param
57+
58+
59+
# Cases with actual default values - these should preserve the default
60+
61+
62+
@app.get("/test7")
63+
async def test_with_default_value(
64+
# This should become: param: Annotated[str, Query(description="Test")] = "default"
65+
param: str = Query("default", description="Test"),
66+
) -> str:
67+
return param
68+
69+
70+
@app.get("/test8")
71+
async def test_with_default_none(
72+
# This should become: param: Annotated[str | None, Query(description="Test")] = None
73+
param: str | None = Query(None, description="Test"),
74+
) -> str:
75+
return param or "empty"
76+
77+
78+
@app.get("/test9")
79+
async def test_mixed_parameters(
80+
# First param should be fixed with default preserved
81+
optional_param: str = Query("default", description="Optional"),
82+
# Second param should not be fixed because of the preceding default
83+
required_param: str = Query(..., description="Required"),
84+
# Third param should be fixed with default preserved
85+
another_optional_param: int = Query(42, description="Another optional"),
86+
) -> str:
87+
return f"{required_param}-{optional_param}-{another_optional_param}"

crates/ruff_linter/src/rules/fastapi/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ mod tests {
1818
#[test_case(Rule::FastApiRedundantResponseModel, Path::new("FAST001.py"))]
1919
#[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_0.py"))]
2020
#[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_1.py"))]
21+
#[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_2.py"))]
2122
#[test_case(Rule::FastApiUnusedPathParameter, Path::new("FAST003.py"))]
2223
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
2324
let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
@@ -56,6 +57,7 @@ mod tests {
5657
// since `typing.Annotated` was added in Python 3.9
5758
#[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_0.py"))]
5859
#[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_1.py"))]
60+
#[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_2.py"))]
5961
fn rules_py38(rule_code: Rule, path: &Path) -> Result<()> {
6062
let snapshot = format!("{}_{}_py38", rule_code.name(), path.to_string_lossy());
6163
let diagnostics = test_path(

crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -275,19 +275,42 @@ fn create_diagnostic(
275275
.collect::<Vec<_>>()
276276
.join(", ");
277277

278-
seen_default = true;
279-
format!(
280-
"{parameter_name}: {binding}[{annotation}, {default_}({kwarg_list})] \
281-
= {default_value}",
278+
// Check if the default argument is ellipsis (...), which in FastAPI means "required"
279+
let is_default_argument_ellipsis = matches!(
280+
dependency_call.default_argument.value(),
281+
ast::Expr::EllipsisLiteral(_)
282+
);
283+
284+
if is_default_argument_ellipsis && seen_default {
285+
// For ellipsis after a parameter with default, can't remove the default
286+
return Ok(None);
287+
}
288+
289+
if !is_default_argument_ellipsis {
290+
// For actual default values, mark that we've seen a default
291+
seen_default = true;
292+
}
293+
294+
let base_format = format!(
295+
"{parameter_name}: {binding}[{annotation}, {default_}({kwarg_list})]",
282296
parameter_name = parameter.name,
283297
annotation = checker.locator().slice(parameter.annotation.range()),
284298
default_ = checker
285299
.locator()
286300
.slice(map_callable(parameter.default).range()),
287-
default_value = checker
301+
);
302+
303+
if is_default_argument_ellipsis {
304+
// For ellipsis, don't add a default value since the parameter
305+
// should remain required after conversion to Annotated
306+
base_format
307+
} else {
308+
// For actual default values, preserve them
309+
let default_value = checker
288310
.locator()
289-
.slice(dependency_call.default_argument.value().range()),
290-
)
311+
.slice(dependency_call.default_argument.value().range());
312+
format!("{base_format} = {default_value}")
313+
}
291314
}
292315
_ => {
293316
if seen_default {

0 commit comments

Comments
 (0)