diff --git a/crates/ruff_linter/resources/test/fixtures/fastapi/FAST002.py b/crates/ruff_linter/resources/test/fixtures/fastapi/FAST002.py index 3473df3cac0fd..4f1d19463b5ca 100644 --- a/crates/ruff_linter/resources/test/fixtures/fastapi/FAST002.py +++ b/crates/ruff_linter/resources/test/fixtures/fastapi/FAST002.py @@ -17,7 +17,7 @@ router = APIRouter() -# Errors +# Fixable errors @app.get("/items/") def get_items( @@ -40,6 +40,34 @@ def do_stuff( # do stuff pass +@app.get("/users/") +def get_users( + skip: int, + limit: int, + current_user: User = Depends(get_current_user), +): + pass + +@app.get("/users/") +def get_users( + current_user: User = Depends(get_current_user), + skip: int = 0, + limit: int = 10, +): + pass + + + +# Non fixable errors + +@app.get("/users/") +def get_users( + skip: int = 0, + limit: int = 10, + current_user: User = Depends(get_current_user), +): + pass + # Unchanged diff --git a/crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs b/crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs index 8c4691451f9e9..4ae14b02f9083 100644 --- a/crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs +++ b/crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; use ruff_python_ast::helpers::map_callable; @@ -59,14 +59,16 @@ use crate::settings::types::PythonVersion; #[violation] pub struct FastApiNonAnnotatedDependency; -impl AlwaysFixableViolation for FastApiNonAnnotatedDependency { +impl Violation for FastApiNonAnnotatedDependency { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { format!("FastAPI dependency without `Annotated`") } - fn fix_title(&self) -> String { - "Replace with `Annotated`".to_string() + fn fix_title(&self) -> Option { + Some("Replace with `Annotated`".to_string()) } } @@ -75,64 +77,95 @@ pub(crate) fn fastapi_non_annotated_dependency( checker: &mut Checker, function_def: &ast::StmtFunctionDef, ) { - if !checker.semantic().seen_module(Modules::FASTAPI) { - return; - } - if !is_fastapi_route(function_def, checker.semantic()) { + if !checker.semantic().seen_module(Modules::FASTAPI) + || !is_fastapi_route(function_def, checker.semantic()) + { return; } + + let mut updatable_count = 0; + let mut has_non_updatable_default = false; + let total_params = function_def.parameters.args.len(); + for parameter in &function_def.parameters.args { + let needs_update = matches!( + (¶meter.parameter.annotation, ¶meter.default), + (Some(_annotation), Some(default)) if is_fastapi_dependency(checker, default) + ); + + if needs_update { + updatable_count += 1; + // Determine if it's safe to update this parameter: + // - if all parameters are updatable its safe. + // - if we've encountered a non-updatable parameter with a default value, its no longer + // safe. (https://github.com/astral-sh/ruff/issues/12982) + let safe_to_update = updatable_count == total_params || !has_non_updatable_default; + create_diagnostic(checker, parameter, safe_to_update); + } else if parameter.default.is_some() { + has_non_updatable_default = true; + } + } +} + +fn is_fastapi_dependency(checker: &Checker, expr: &ast::Expr) -> bool { + checker + .semantic() + .resolve_qualified_name(map_callable(expr)) + .is_some_and(|qualified_name| { + matches!( + qualified_name.segments(), + [ + "fastapi", + "Query" + | "Path" + | "Body" + | "Cookie" + | "Header" + | "File" + | "Form" + | "Depends" + | "Security" + ] + ) + }) +} + +fn create_diagnostic( + checker: &mut Checker, + parameter: &ast::ParameterWithDefault, + safe_to_update: bool, +) { + let mut diagnostic = Diagnostic::new(FastApiNonAnnotatedDependency, parameter.range); + + if safe_to_update { if let (Some(annotation), Some(default)) = (¶meter.parameter.annotation, ¶meter.default) { - if checker - .semantic() - .resolve_qualified_name(map_callable(default)) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - [ - "fastapi", - "Query" - | "Path" - | "Body" - | "Cookie" - | "Header" - | "File" - | "Form" - | "Depends" - | "Security" - ] - ) - }) - { - let mut diagnostic = - Diagnostic::new(FastApiNonAnnotatedDependency, parameter.range); - - diagnostic.try_set_fix(|| { - let module = if checker.settings.target_version >= PythonVersion::Py39 { - "typing" - } else { - "typing_extensions" - }; - let (import_edit, binding) = checker.importer().get_or_import_symbol( - &ImportRequest::import_from(module, "Annotated"), - function_def.start(), - checker.semantic(), - )?; - let content = format!( - "{}: {}[{}, {}]", - parameter.parameter.name.id, - binding, - checker.locator().slice(annotation.range()), - checker.locator().slice(default.range()) - ); - let parameter_edit = Edit::range_replacement(content, parameter.range()); - Ok(Fix::unsafe_edits(import_edit, [parameter_edit])) - }); - - checker.diagnostics.push(diagnostic); - } + diagnostic.try_set_fix(|| { + let module = if checker.settings.target_version >= PythonVersion::Py39 { + "typing" + } else { + "typing_extensions" + }; + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import_from(module, "Annotated"), + parameter.range.start(), + checker.semantic(), + )?; + let content = format!( + "{}: {}[{}, {}]", + parameter.parameter.name.id, + binding, + checker.locator().slice(annotation.range()), + checker.locator().slice(default.range()) + ); + let parameter_edit = Edit::range_replacement(content, parameter.range); + Ok(Fix::unsafe_edits(import_edit, [parameter_edit])) + }); } + } else { + diagnostic.fix = None; } + + checker.diagnostics.push(diagnostic); }