[fastapi] Handle parameters with Depends correctly (FAST003)

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

@@ -1,6 +1,6 @@
 from typing import Annotated
 
-from fastapi import FastAPI, Path
+from fastapi import Depends, FastAPI, Path
 
 app = FastAPI()
 
@@ -144,3 +144,35 @@ async def read_thing(query: str):
 @app.get("/things/{thing_id=}")
 async def read_thing(query: str):
     return {"query": query}
+
+
+# https://github.com/astral-sh/ruff/issues/13657
+def takes_thing_id(thing_id): ...
+def something_else(lorem): ...
+
+from foo import unknown_imported
+unknown_not_function = unknown_imported()
+
+
+### Errors
+@app.get("/things/{thing_id}")
+async def single(other: Annotated[str, Depends(something_else)]): ...
+@app.get("/things/{thing_id}")
+async def double(other: Annotated[str, Depends(something_else), Depends(takes_thing_id)]): ...
+@app.get("/things/{thing_id}")
+async def default(other: str = Depends(something_else)): ...
+
+
+### No errors
+@app.get("/things/{thing_id}")
+async def single(other: Annotated[str, Depends(takes_thing_id)]): ...
+@app.get("/things/{thing_id}")
+async def double(other: Annotated[str, Depends(takes_thing_id), Depends(something_else)]): ...
+@app.get("/things/{thing_id}")
+async def default(other: str = Depends(takes_thing_id)): ...
+@app.get("/things/{thing_id}")
+async def unknown_1(other: str = Depends(unknown_unresolved)): ...
+@app.get("/things/{thing_id}")
+async def unknown_2(other: str = Depends(unknown_not_function)): ...
+@app.get("/things/{thing_id}")
+async def unknown_3(other: str = Depends(unknown_imported)): ...

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

@@ -6,8 +6,8 @@ use ruff_diagnostics::Fix;
 use ruff_diagnostics::{Diagnostic, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_ast as ast;
-use ruff_python_ast::{Expr, Parameter, ParameterWithDefault};
-use ruff_python_semantic::{Modules, SemanticModel};
+use ruff_python_ast::{Expr, ExprCall, ExprSubscript, Parameter, ParameterWithDefault};
+use ruff_python_semantic::{BindingKind, Modules, ScopeKind, SemanticModel};
 use ruff_python_stdlib::identifiers::is_identifier;
 use ruff_text_size::{Ranged, TextSize};
 
@@ -137,16 +137,29 @@ pub(crate) fn fastapi_unused_path_parameter(
     let path_params = PathParamIterator::new(path.to_str());
 
     // Extract the arguments from the function signature
-    let named_args: Vec<_> = function_def
-        .parameters
-        .args
-        .iter()
-        .chain(&function_def.parameters.kwonlyargs)
-        .map(|ParameterWithDefault { parameter, .. }| {
-            parameter_alias(parameter, checker.semantic())
-                .unwrap_or_else(|| parameter.name.as_str())
-        })
-        .collect();
+    let mut named_args = vec![];
+
+    for parameter_with_default in non_posonly_non_variadic_parameters(function_def) {
+        let ParameterWithDefault { parameter, .. } = parameter_with_default;
+
+        if let Some(alias) = parameter_alias(parameter, checker.semantic()) {
+            named_args.push(alias.to_string());
+        } else {
+            named_args.push(parameter.name.to_string());
+        }
+
+        let Some(dependency) =
+            Dependency::from_parameter(parameter_with_default, checker.semantic())
+        else {
+            continue;
+        };
+
+        if let Some(parameter_names) = dependency.parameter_names() {
+            named_args.extend(parameter_names);
+        } else {
+            return;
+        }
+    }
 
     // Check if any of the path parameters are not in the function signature.
     let mut diagnostics = vec![];
@@ -156,8 +169,9 @@ pub(crate) fn fastapi_unused_path_parameter(
             continue;
         }
 
-        // If the path parameter is already in the function signature, we don't need to do anything.
-        if named_args.contains(&path_param) {
+        // If the path parameter is already in the function or the dependency signature,
+        // we don't need to do anything.
+        if named_args.contains(&path_param.to_string()) {
             continue;
         }
 
@@ -194,6 +208,126 @@ pub(crate) fn fastapi_unused_path_parameter(
     checker.diagnostics.extend(diagnostics);
 }
 
+fn non_posonly_non_variadic_parameters(
+    function_def: &ast::StmtFunctionDef,
+) -> impl Iterator<Item = &ParameterWithDefault> {
+    function_def
+        .parameters
+        .args
+        .iter()
+        .chain(&function_def.parameters.kwonlyargs)
+}
+
+#[derive(Debug, is_macro::Is)]
+enum Dependency {
+    /// Not defined in the same file, or otherwise cannot be determined to be a function.
+    Unknown,
+    /// A function defined in the same file, whose parameter names are as given.
+    Function(Vec<String>),
+}
+
+impl Dependency {
+    fn parameter_names(self) -> Option<Vec<String>> {
+        match self {
+            Self::Unknown => None,
+            Self::Function(parameter_names) => Some(parameter_names),
+        }
+    }
+}
+
+impl Dependency {
+    /// Return `foo` in the first metadata `Depends(foo)`,
+    /// or that of the default value:
+    ///
+    /// ```python
+    /// def _(
+    ///     a: Annotated[str, Depends(foo), Depends(bar)],
+    ///     #                 ^^^^^^^^^^^^
+    ///     a: str = Depends(foo),
+    ///     #        ^^^^^^^^^^^^
+    /// ): ...
+    /// ```
+    fn from_parameter(
+        parameter_with_default: &ParameterWithDefault,
+        semantic: &SemanticModel,
+    ) -> Option<Self> {
+        let ParameterWithDefault {
+            parameter, default, ..
+        } = parameter_with_default;
+
+        if let Some(default) = default {
+            if let Some(dependency) = Self::from_call(default.as_ref(), semantic) {
+                return Some(dependency);
+            };
+        }
+
+        let Expr::Subscript(ExprSubscript { value, slice, .. }) =
+            &parameter.annotation.as_deref()?
+        else {
+            return None;
+        };
+
+        if !semantic.match_typing_expr(value, "Annotated") {
+            return None;
+        }
+
+        let Expr::Tuple(tuple) = slice.as_ref() else {
+            return None;
+        };
+
+        tuple
+            .elts
+            .iter()
+            .skip(1)
+            .find_map(|metadata| Self::from_call(metadata, semantic))
+    }
+
+    fn from_call(expr: &Expr, semantic: &SemanticModel) -> Option<Self> {
+        let Expr::Call(ExprCall {
+            func, arguments, ..
+        }) = expr
+        else {
+            return None;
+        };
+
+        if !is_fastapi_depends(func.as_ref(), semantic) {
+            return None;
+        }
+
+        let Some(Expr::Name(name)) = arguments.find_argument_value("dependency", 0) else {
+            return None;
+        };
+
+        let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
+            return Some(Self::Unknown);
+        };
+
+        let BindingKind::FunctionDefinition(scope_id) = binding.kind else {
+            return Some(Self::Unknown);
+        };
+
+        let scope = &semantic.scopes[scope_id];
+
+        let ScopeKind::Function(function_def) = scope.kind else {
+            return Some(Self::Unknown);
+        };
+
+        let parameter_names = non_posonly_non_variadic_parameters(function_def)
+            .map(|ParameterWithDefault { parameter, .. }| parameter.name.to_string())
+            .collect::<Vec<_>>();
+
+        Some(Self::Function(parameter_names))
+    }
+}
+
+fn is_fastapi_depends(expr: &Expr, semantic: &SemanticModel) -> bool {
+    let Some(qualified_name) = semantic.resolve_qualified_name(expr) else {
+        return false;
+    };
+
+    matches!(qualified_name.segments(), ["fastapi", "Depends"])
+}
+
 /// Extract the expected in-route name for a given parameter, if it has an alias.
 /// For example, given `document_id: Annotated[str, Path(alias="documentId")]`, returns `"documentId"`.
 fn parameter_alias<'a>(parameter: &'a Parameter, semantic: &SemanticModel) -> Option<&'a str> {

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

@@ -340,4 +340,65 @@ FAST003.py:87:18: FAST003 [*] Parameter `name` appears in route path, but not in
    88 |+async def read_thing(name, *, author: Annotated[str, Path(alias="author_name")], title: str):
 89 89 |     return {"author": author, "title": title}
 90 90 | 
-91 91 |
+91 91 | 
+
+FAST003.py:158:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `single` signature
+    |
+157 | ### Errors
+158 | @app.get("/things/{thing_id}")
+    |                   ^^^^^^^^^^ FAST003
+159 | async def single(other: Annotated[str, Depends(something_else)]): ...
+160 | @app.get("/things/{thing_id}")
+    |
+    = help: Add `thing_id` to function signature
+
+ℹ Unsafe fix
+156 156 | 
+157 157 | ### Errors
+158 158 | @app.get("/things/{thing_id}")
+159     |-async def single(other: Annotated[str, Depends(something_else)]): ...
+    159 |+async def single(other: Annotated[str, Depends(something_else)], thing_id): ...
+160 160 | @app.get("/things/{thing_id}")
+161 161 | async def double(other: Annotated[str, Depends(something_else), Depends(takes_thing_id)]): ...
+162 162 | @app.get("/things/{thing_id}")
+
+FAST003.py:160:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `double` signature
+    |
+158 | @app.get("/things/{thing_id}")
+159 | async def single(other: Annotated[str, Depends(something_else)]): ...
+160 | @app.get("/things/{thing_id}")
+    |                   ^^^^^^^^^^ FAST003
+161 | async def double(other: Annotated[str, Depends(something_else), Depends(takes_thing_id)]): ...
+162 | @app.get("/things/{thing_id}")
+    |
+    = help: Add `thing_id` to function signature
+
+ℹ Unsafe fix
+158 158 | @app.get("/things/{thing_id}")
+159 159 | async def single(other: Annotated[str, Depends(something_else)]): ...
+160 160 | @app.get("/things/{thing_id}")
+161     |-async def double(other: Annotated[str, Depends(something_else), Depends(takes_thing_id)]): ...
+    161 |+async def double(other: Annotated[str, Depends(something_else), Depends(takes_thing_id)], thing_id): ...
+162 162 | @app.get("/things/{thing_id}")
+163 163 | async def default(other: str = Depends(something_else)): ...
+164 164 | 
+
+FAST003.py:162:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `default` signature
+    |
+160 | @app.get("/things/{thing_id}")
+161 | async def double(other: Annotated[str, Depends(something_else), Depends(takes_thing_id)]): ...
+162 | @app.get("/things/{thing_id}")
+    |                   ^^^^^^^^^^ FAST003
+163 | async def default(other: str = Depends(something_else)): ...
+    |
+    = help: Add `thing_id` to function signature
+
+ℹ Unsafe fix
+160 160 | @app.get("/things/{thing_id}")
+161 161 | async def double(other: Annotated[str, Depends(something_else), Depends(takes_thing_id)]): ...
+162 162 | @app.get("/things/{thing_id}")
+163     |-async def default(other: str = Depends(something_else)): ...
+    163 |+async def default(thing_id, other: str = Depends(something_else)): ...
+164 164 | 
+165 165 | 
+166 166 | ### No errors