Skip to content

Commit c90a5c7

Browse files
committed
fix(FAST003): handle Depends(Class()) instance pattern for __call__ deps
Rework the dependency resolution to correctly distinguish between: - Depends(SomeClass) — class reference, FastAPI calls __init__ - Depends(SomeClass()) — instance, FastAPI calls __call__ Previously the code conflated both cases by checking __init__ OR __call__ in a single find(), which was both incorrect and misleading. Now: - from_dependency_name: handles Name refs, uses __init__ for classes - from_dependency_instance: handles Call exprs, uses __call__ for classes - class_params_from_method: shared helper for both paths Also applies the matches!() nit from review and restores the original 'Skip self parameter' comment.
1 parent bbaeb39 commit c90a5c7

4 files changed

Lines changed: 211 additions & 64 deletions

crates/ruff_linter/resources/test/fixtures/fastapi/FAST003.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,9 @@ async def read_thing_posonly_with_regular(query: str = "", /, x=None): ...
303303

304304
# https://github.com/astral-sh/ruff/issues/23526
305305

306-
# OK: callable class dependency with __call__ method
306+
# Error: `Depends(CallableQuery)` passes the class itself, so FastAPI uses
307+
# `__init__` (which has no params here), not `__call__`. The path parameter
308+
# `thing_id` is unused.
307309
class CallableQuery:
308310
def __call__(self, thing_id: int):
309311
pass
@@ -313,7 +315,14 @@ def __call__(self, thing_id: int):
313315
async def read_thing_callable_dep(query: Annotated[str, Depends(CallableQuery)]): ...
314316

315317

316-
# OK: class with both __init__ and __call__
318+
# OK: `Depends(CallableQuery())` passes an instance, so FastAPI uses `__call__`,
319+
# which declares `thing_id`.
320+
@app.get("/things/{thing_id}")
321+
async def read_thing_callable_dep_instance(query: Annotated[str, Depends(CallableQuery())]): ...
322+
323+
324+
# OK: class with both __init__ and __call__, passed as class reference.
325+
# FastAPI uses `__init__`, which declares `thing_id`.
317326
class InitAndCallQuery:
318327
def __init__(self, thing_id: int):
319328
pass
@@ -326,7 +335,8 @@ def __call__(self, other: str):
326335
async def read_thing_init_and_call_dep(query: Annotated[str, Depends(InitAndCallQuery)]): ...
327336

328337

329-
# Error: callable class dependency where path param is not in __call__
338+
# Error: `Depends(CallableQueryOther)` — class reference, uses `__init__` (no
339+
# params). `thing_id` is unused.
330340
class CallableQueryOther:
331341
def __call__(self, other: str):
332342
pass
@@ -336,6 +346,12 @@ def __call__(self, other: str):
336346
async def read_thing_callable_dep_missing(query: Annotated[str, Depends(CallableQueryOther)]): ...
337347

338348

349+
# Error: `Depends(InitAndCallQuery())` passes an instance, so FastAPI uses
350+
# `__call__`, which has `other` — not `thing_id`.
351+
@app.get("/things/{thing_id}")
352+
async def read_thing_init_and_call_instance(query: Annotated[str, Depends(InitAndCallQuery())]): ...
353+
354+
339355
# OK: class with no __init__ and no __call__ (unknown dependency, no diagnostic)
340356
class EmptyClass:
341357
pass

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

Lines changed: 82 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -307,13 +307,27 @@ impl<'a> Dependency<'a> {
307307
}
308308

309309
fn from_depends_call(arguments: &'a Arguments, semantic: &SemanticModel<'a>) -> Option<Self> {
310-
let Some(Expr::Name(name)) = arguments.find_argument_value("dependency", 0) else {
311-
return None;
312-
};
313-
314-
Self::from_dependency_name(name, semantic)
310+
let dep_arg = arguments.find_argument_value("dependency", 0)?;
311+
312+
match dep_arg {
313+
// `Depends(some_callable)` — a name reference (function or class).
314+
Expr::Name(name) => Self::from_dependency_name(name, semantic),
315+
// `Depends(SomeClass(...))` — a call expression. If the callee is a
316+
// class, FastAPI will invoke `__call__` on the resulting instance.
317+
Expr::Call(call) => {
318+
let Expr::Name(name) = call.func.as_ref() else {
319+
return None;
320+
};
321+
Self::from_dependency_instance(name, semantic)
322+
}
323+
_ => None,
324+
}
315325
}
316326

327+
/// Resolve a dependency that is a name reference (e.g. `Depends(Query)`).
328+
///
329+
/// For classes, FastAPI calls the class constructor, so the parameters come
330+
/// from `__init__`.
317331
fn from_dependency_name(name: &'a ast::ExprName, semantic: &SemanticModel<'a>) -> Option<Self> {
318332
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
319333
return Some(Self::Unknown);
@@ -334,55 +348,75 @@ impl<'a> Dependency<'a> {
334348
Some(Self::Function(parameter_names))
335349
}
336350
BindingKind::ClassDefinition(scope_id) => {
337-
let scope = &semantic.scopes[scope_id];
338-
339-
let ScopeKind::Class(class_def) = scope.kind else {
340-
return Some(Self::Unknown);
341-
};
351+
Self::class_params_from_method(semantic, scope_id, "__init__")
352+
}
353+
_ => Some(Self::Unknown),
354+
}
355+
}
342356

343-
let parameter_names = if class_def
344-
.bases()
345-
.iter()
346-
.any(|expr| is_pydantic_base_model(expr, semantic))
347-
{
348-
class_def
349-
.body
350-
.iter()
351-
.filter_map(|stmt| {
352-
stmt.as_ann_assign_stmt()
353-
.and_then(|ann_assign| ann_assign.target.as_name_expr())
354-
.map(|name| name.id.as_str())
355-
})
356-
.collect()
357-
} else if let Some(method_def) = class_def
358-
.body
359-
.iter()
360-
.filter_map(|stmt| stmt.as_function_def_stmt())
361-
.find(|func_def| {
362-
func_def.name.as_str() == "__init__"
363-
|| func_def.name.as_str() == "__call__"
364-
})
365-
{
366-
// Skip `self` parameter.
367-
//
368-
// For classes used as FastAPI dependencies, the parameters
369-
// can come from either `__init__` (when the class itself is
370-
// passed to `Depends`) or `__call__` (when an instance of
371-
// the class is passed to `Depends`). We check `__init__`
372-
// first, falling back to `__call__`.
373-
non_posonly_non_variadic_parameters(method_def)
374-
.skip(1)
375-
.map(|param| param.name().as_str())
376-
.collect()
377-
} else {
378-
return Some(Self::Unknown);
379-
};
357+
/// Resolve a dependency that is a class instance (e.g. `Depends(Query())`).
358+
///
359+
/// FastAPI calls the instance, so the parameters come from `__call__`.
360+
fn from_dependency_instance(
361+
name: &'a ast::ExprName,
362+
semantic: &SemanticModel<'a>,
363+
) -> Option<Self> {
364+
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
365+
return Some(Self::Unknown);
366+
};
380367

381-
Some(Self::Class(parameter_names))
368+
match binding.kind {
369+
BindingKind::ClassDefinition(scope_id) => {
370+
Self::class_params_from_method(semantic, scope_id, "__call__")
382371
}
383372
_ => Some(Self::Unknown),
384373
}
385374
}
375+
376+
/// Extract parameters from a specific method (`__init__` or `__call__`) of a class.
377+
fn class_params_from_method(
378+
semantic: &SemanticModel<'a>,
379+
scope_id: ruff_python_semantic::ScopeId,
380+
method_name: &str,
381+
) -> Option<Self> {
382+
let scope = &semantic.scopes[scope_id];
383+
384+
let ScopeKind::Class(class_def) = scope.kind else {
385+
return Some(Self::Unknown);
386+
};
387+
388+
let parameter_names = if class_def
389+
.bases()
390+
.iter()
391+
.any(|expr| is_pydantic_base_model(expr, semantic))
392+
{
393+
class_def
394+
.body
395+
.iter()
396+
.filter_map(|stmt| {
397+
stmt.as_ann_assign_stmt()
398+
.and_then(|ann_assign| ann_assign.target.as_name_expr())
399+
.map(|name| name.id.as_str())
400+
})
401+
.collect()
402+
} else if let Some(method_def) = class_def
403+
.body
404+
.iter()
405+
.filter_map(|stmt| stmt.as_function_def_stmt())
406+
.find(|func_def| func_def.name.as_str() == method_name)
407+
{
408+
// Skip `self` parameter
409+
non_posonly_non_variadic_parameters(method_def)
410+
.skip(1)
411+
.map(|param| param.name().as_str())
412+
.collect()
413+
} else {
414+
// No matching method found — treat as unknown to suppress false positives.
415+
return Some(Self::Unknown);
416+
};
417+
418+
Some(Self::Class(parameter_names))
419+
}
386420
}
387421

388422
fn depends_arguments<'a>(expr: &'a Expr, semantic: &SemanticModel) -> Option<&'a Arguments> {

crates/ruff_linter/src/rules/fastapi/snapshots/ruff_linter__rules__fastapi__tests__deferred_annotations_diff_fast-api-unused-path-parameter_FAST003.py.snap

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
---
22
source: crates/ruff_linter/src/rules/fastapi/mod.rs
3-
assertion_line: 41
43
---
54
--- Linter settings ---
65
-linter.unresolved_target_version = 3.13
@@ -75,20 +74,22 @@ help: Add `id` to function signature
7574
note: This is an unsafe fix and may change runtime behavior
7675

7776

78-
FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing_callable_dep_missing` signature
79-
--> FAST003.py:335:19
77+
FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing_init_and_call_instance` signature
78+
--> FAST003.py:351:19
8079
|
81-
335 | @app.get("/things/{thing_id}")
80+
349 | # Error: `Depends(InitAndCallQuery())` passes an instance, so FastAPI uses
81+
350 | # `__call__`, which has `other` — not `thing_id`.
82+
351 | @app.get("/things/{thing_id}")
8283
| ^^^^^^^^^^
83-
336 | async def read_thing_callable_dep_missing(query: Annotated[str, Depends(CallableQueryOther)]): ...
84+
352 | async def read_thing_init_and_call_instance(query: Annotated[str, Depends(InitAndCallQuery())]): ...
8485
|
8586
help: Add `thing_id` to function signature
86-
333 |
87-
334 |
88-
335 | @app.get("/things/{thing_id}")
89-
- async def read_thing_callable_dep_missing(query: Annotated[str, Depends(CallableQueryOther)]): ...
90-
336 + async def read_thing_callable_dep_missing(query: Annotated[str, Depends(CallableQueryOther)], thing_id): ...
91-
337 |
92-
338 |
93-
339 | # OK: class with no __init__ and no __call__ (unknown dependency, no diagnostic)
87+
349 | # Error: `Depends(InitAndCallQuery())` passes an instance, so FastAPI uses
88+
350 | # `__call__`, which has `other` — not `thing_id`.
89+
351 | @app.get("/things/{thing_id}")
90+
- async def read_thing_init_and_call_instance(query: Annotated[str, Depends(InitAndCallQuery())]): ...
91+
352 + async def read_thing_init_and_call_instance(query: Annotated[str, Depends(InitAndCallQuery())], thing_id): ...
92+
353 |
93+
354 |
94+
355 | # OK: class with no __init__ and no __call__ (unknown dependency, no diagnostic)
9495
note: This is an unsafe fix and may change runtime behavior
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
---
2+
source: crates/ruff_linter/src/rules/fastapi/mod.rs
3+
assertion_line: 41
4+
---
5+
--- Linter settings ---
6+
-linter.unresolved_target_version = 3.13
7+
+linter.unresolved_target_version = 3.14
8+
9+
--- Summary ---
10+
Removed: 4
11+
Added: 0
12+
13+
--- Removed ---
14+
FAST003 [*] Parameter `thing_id` appears in route path, but not in `single` signature
15+
--> FAST003.py:158:19
16+
|
17+
157 | ### Errors
18+
158 | @app.get("/things/{thing_id}")
19+
| ^^^^^^^^^^
20+
159 | async def single(other: Annotated[str, Depends(something_else)]): ...
21+
160 | @app.get("/things/{thing_id}")
22+
|
23+
help: Add `thing_id` to function signature
24+
156 |
25+
157 | ### Errors
26+
158 | @app.get("/things/{thing_id}")
27+
- async def single(other: Annotated[str, Depends(something_else)]): ...
28+
159 + async def single(other: Annotated[str, Depends(something_else)], thing_id): ...
29+
160 | @app.get("/things/{thing_id}")
30+
161 | async def default(other: str = Depends(something_else)): ...
31+
162 |
32+
note: This is an unsafe fix and may change runtime behavior
33+
34+
35+
FAST003 [*] Parameter `id` appears in route path, but not in `get_id_pydantic_full` signature
36+
--> FAST003.py:197:12
37+
|
38+
196 | # Errors
39+
197 | @app.get("/{id}")
40+
| ^^^^
41+
198 | async def get_id_pydantic_full(
42+
199 | params: Annotated[PydanticParams, Depends(PydanticParams)],
43+
|
44+
help: Add `id` to function signature
45+
196 | # Errors
46+
197 | @app.get("/{id}")
47+
198 | async def get_id_pydantic_full(
48+
- params: Annotated[PydanticParams, Depends(PydanticParams)],
49+
199 + params: Annotated[PydanticParams, Depends(PydanticParams)], id,
50+
200 | ): ...
51+
201 | @app.get("/{id}")
52+
202 | async def get_id_pydantic_short(params: Annotated[PydanticParams, Depends()]): ...
53+
note: This is an unsafe fix and may change runtime behavior
54+
55+
56+
FAST003 [*] Parameter `id` appears in route path, but not in `get_id_pydantic_short` signature
57+
--> FAST003.py:201:12
58+
|
59+
199 | params: Annotated[PydanticParams, Depends(PydanticParams)],
60+
200 | ): ...
61+
201 | @app.get("/{id}")
62+
| ^^^^
63+
202 | async def get_id_pydantic_short(params: Annotated[PydanticParams, Depends()]): ...
64+
203 | @app.get("/{id}")
65+
|
66+
help: Add `id` to function signature
67+
199 | params: Annotated[PydanticParams, Depends(PydanticParams)],
68+
200 | ): ...
69+
201 | @app.get("/{id}")
70+
- async def get_id_pydantic_short(params: Annotated[PydanticParams, Depends()]): ...
71+
202 + async def get_id_pydantic_short(params: Annotated[PydanticParams, Depends()], id): ...
72+
203 | @app.get("/{id}")
73+
204 | async def get_id_init_not_annotated(params = Depends(InitParams)): ...
74+
205 |
75+
note: This is an unsafe fix and may change runtime behavior
76+
77+
78+
FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing_init_and_call_instance` signature
79+
--> FAST003.py:353:19
80+
|
81+
351 | # (the instance was already constructed).
82+
352 | # Actually this should be an error since __call__ has `other`, not `thing_id`.
83+
353 | @app.get("/things/{thing_id}")
84+
| ^^^^^^^^^^
85+
354 | async def read_thing_init_and_call_instance(query: Annotated[str, Depends(InitAndCallQuery())]): ...
86+
|
87+
help: Add `thing_id` to function signature
88+
351 | # (the instance was already constructed).
89+
352 | # Actually this should be an error since __call__ has `other`, not `thing_id`.
90+
353 | @app.get("/things/{thing_id}")
91+
- async def read_thing_init_and_call_instance(query: Annotated[str, Depends(InitAndCallQuery())]): ...
92+
354 + async def read_thing_init_and_call_instance(query: Annotated[str, Depends(InitAndCallQuery())], thing_id): ...
93+
355 |
94+
356 |
95+
357 | # OK: class with no __init__ and no __call__ (unknown dependency, no diagnostic)
96+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)