Skip to content

Commit

Permalink
Use TypeChecker for detecting fastapi routes (#15093)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomerBin authored Dec 21, 2024
1 parent fd4bea5 commit 2fb6b32
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 11 deletions.
15 changes: 15 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/fastapi/FAST001.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,18 @@ async def create_item(item: Item) -> Dict[str, str]:
@app.post("/items/", response_model=Item)
async def create_item(item: Item) -> Item:
return item


# Routes might be defined inside functions


def setup_app(app_arg: FastAPI, non_app: str) -> None:
# Error
@app_arg.get("/", response_model=str)
async def get_root() -> str:
return "Hello World!"

# Ok
@non_app.get("/", response_model=str)
async def get_root() -> str:
return "Hello World!"
11 changes: 10 additions & 1 deletion crates/ruff_linter/resources/test/fixtures/ruff/RUF029.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,20 @@ async def test() -> str:
return ",".join(vals)


# FastApi routes can be async without actually using await

from fastapi import FastAPI

app = FastAPI()


@app.post("/count")
async def fastapi_route(): # Ok: FastApi routes can be async without actually using await
async def fastapi_route():
return 1


def setup_app(app_arg: FastAPI, non_app: str) -> None:
@app_arg.get("/")
async def get_root() -> str:
return "Hello World!"

16 changes: 8 additions & 8 deletions crates/ruff_linter/src/rules/fastapi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod fastapi_redundant_response_model;
mod fastapi_unused_path_parameter;

use ruff_python_ast as ast;
use ruff_python_semantic::analyze::typing::resolve_assignment;
use ruff_python_semantic::analyze::typing;
use ruff_python_semantic::SemanticModel;

/// Returns `true` if the function is a FastAPI route.
Expand Down Expand Up @@ -41,11 +41,11 @@ pub(crate) fn is_fastapi_route_call(call_expr: &ast::ExprCall, semantic: &Semant
) {
return false;
}

resolve_assignment(value, semantic).is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["fastapi", "FastAPI" | "APIRouter"]
)
})
let Some(name) = value.as_name_expr() else {
return false;
};
let Some(binding_id) = semantic.resolve_name(name) else {
return false;
};
typing::is_fastapi_route(semantic.binding(binding_id), semantic)
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/ruff_linter/src/rules/fastapi/mod.rs
snapshot_kind: text
---
FAST001.py:17:22: FAST001 [*] FastAPI route with redundant `response_model` argument
|
Expand Down Expand Up @@ -172,4 +171,25 @@ FAST001.py:53:24: FAST001 [*] FastAPI route with redundant `response_model` argu
53 |+@router.get("/items/")
54 54 | async def create_item(item: Item) -> Item:
55 55 | return item
56 56 |
56 56 |

FAST001.py:118:23: FAST001 [*] FastAPI route with redundant `response_model` argument
|
116 | def setup_app(app_arg: FastAPI, non_app: str) -> None:
117 | # Error
118 | @app_arg.get("/", response_model=str)
| ^^^^^^^^^^^^^^^^^^ FAST001
119 | async def get_root() -> str:
120 | return "Hello World!"
|
= help: Remove argument

Unsafe fix
115 115 |
116 116 | def setup_app(app_arg: FastAPI, non_app: str) -> None:
117 117 | # Error
118 |- @app_arg.get("/", response_model=str)
118 |+ @app_arg.get("/")
119 119 | async def get_root() -> str:
120 120 | return "Hello World!"
121 121 |
33 changes: 33 additions & 0 deletions crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,35 @@ impl TypeChecker for PathlibPathChecker {
}
}

pub struct FastApiRouteChecker;

impl FastApiRouteChecker {
fn is_fastapi_route_constructor(semantic: &SemanticModel, expr: &Expr) -> bool {
let Some(qualified_name) = semantic.resolve_qualified_name(expr) else {
return false;
};

matches!(
qualified_name.segments(),
["fastapi", "FastAPI" | "APIRouter"]
)
}
}

impl TypeChecker for FastApiRouteChecker {
fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool {
Self::is_fastapi_route_constructor(semantic, annotation)
}

fn match_initializer(initializer: &Expr, semantic: &SemanticModel) -> bool {
let Expr::Call(ast::ExprCall { func, .. }) = initializer else {
return false;
};

Self::is_fastapi_route_constructor(semantic, func)
}
}

pub struct TypeVarLikeChecker;

impl TypeVarLikeChecker {
Expand Down Expand Up @@ -914,6 +943,10 @@ pub fn is_pathlib_path(binding: &Binding, semantic: &SemanticModel) -> bool {
check_type::<PathlibPathChecker>(binding, semantic)
}

pub fn is_fastapi_route(binding: &Binding, semantic: &SemanticModel) -> bool {
check_type::<FastApiRouteChecker>(binding, semantic)
}

/// Test whether the given binding is for an old-style `TypeVar`, `TypeVarTuple` or a `ParamSpec`.
pub fn is_type_var_like(binding: &Binding, semantic: &SemanticModel) -> bool {
check_type::<TypeVarLikeChecker>(binding, semantic)
Expand Down

0 comments on commit 2fb6b32

Please sign in to comment.