Skip to content

Commit

Permalink
Respect FastAPI aliases in route definitions (#13394)
Browse files Browse the repository at this point in the history
## Summary

Closes #13263
  • Loading branch information
charliermarsh authored Sep 18, 2024
1 parent 4eb849a commit 44d916f
Show file tree
Hide file tree
Showing 3 changed files with 320 additions and 239 deletions.
16 changes: 14 additions & 2 deletions crates/ruff_linter/resources/test/fixtures/fastapi/FAST003.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from fastapi import FastAPI
from typing import Annotated

from fastapi import FastAPI, Path

app = FastAPI()

Expand Down Expand Up @@ -82,6 +84,11 @@ async def read_thing(
return {"query": query}


@app.get("/books/{name}/{title}")
async def read_thing(*, author: Annotated[str, Path(alias="author_name")], title: str):
return {"author": author, "title": title}


# OK
@app.get("/things/{thing_id}")
async def read_thing(thing_id: int, query: str):
Expand Down Expand Up @@ -118,6 +125,11 @@ async def read_thing(*, author: str, title: str):
return {"author": author, "title": title}


@app.get("/books/{name}/{title}")
async def read_thing(*, author: Annotated[str, Path(alias="name")], title: str):
return {"author": author, "title": title}


# Ignored
@app.get("/things/{thing-id}")
async def read_thing(query: str):
Expand All @@ -131,4 +143,4 @@ async def read_thing(query: str):

@app.get("/things/{thing_id=}")
async def read_thing(query: str):
return {"query": query}
return {"query": query}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use ruff_diagnostics::Fix;
use ruff_diagnostics::{Diagnostic, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_semantic::Modules;
use ruff_python_ast::{Expr, Parameter, ParameterWithDefault};
use ruff_python_semantic::{Modules, SemanticModel};
use ruff_python_stdlib::identifiers::is_identifier;
use ruff_text_size::{Ranged, TextSize};

Expand Down Expand Up @@ -141,7 +142,10 @@ pub(crate) fn fastapi_unused_path_parameter(
.args
.iter()
.chain(function_def.parameters.kwonlyargs.iter())
.map(|arg| arg.parameter.name.as_str())
.map(|ParameterWithDefault { parameter, .. }| {
parameter_alias(parameter, checker.semantic())
.unwrap_or_else(|| parameter.name.as_str())
})
.collect();

// Check if any of the path parameters are not in the function signature.
Expand Down Expand Up @@ -190,6 +194,52 @@ pub(crate) fn fastapi_unused_path_parameter(
checker.diagnostics.extend(diagnostics);
}

/// 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> {
let Some(annotation) = &parameter.annotation else {
return None;
};

let Expr::Subscript(subscript) = annotation.as_ref() else {
return None;
};

let Expr::Tuple(tuple) = subscript.slice.as_ref() else {
return None;
};

let Some(Expr::Call(path)) = tuple.elts.get(1) else {
return None;
};

// Find the `alias` keyword argument.
let alias = path
.arguments
.find_keyword("alias")
.map(|alias| &alias.value)?;

// Ensure that it's a literal string.
let Expr::StringLiteral(alias) = alias else {
return None;
};

// Verify that the subscript was a `typing.Annotated`.
if !semantic.match_typing_expr(&subscript.value, "Annotated") {
return None;
}

// Verify that the call was a `fastapi.Path`.
if !semantic
.resolve_qualified_name(&path.func)
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["fastapi", "Path"]))
{
return None;
}

Some(alias.value.to_str())
}

/// An iterator to extract parameters from FastAPI route paths.
///
/// The iterator yields tuples of the parameter name and the range of the parameter in the input,
Expand Down
Loading

0 comments on commit 44d916f

Please sign in to comment.