Skip to content

[FastAPI] Add sub-diagnostic explaining why a fix was unavailable (FAST002) #22565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ntBre merged 6 commits into from
Jan 27, 2026
Merged

[FastAPI] Add sub-diagnostic explaining why a fix was unavailable (FAST002) #22565

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
49ca9db
add autofix skipped reason as diagnostic info
11happy Jan 14, 2026
04adb03
refactor: use better wording for info messages
11happy Jan 14, 2026
6137ed8
update snaps
11happy Jan 14, 2026
6fc5a41
update message
11happy Jan 21, 2026
0678602
review snaps
11happy Jan 21, 2026
6b5118d
update subdiagnostic info
11happy Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 36 additions & 9 deletions crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,27 @@ impl Violation for FastApiNonAnnotatedDependency {
}
}

#[derive(Debug)]
enum AutofixSkipped {
EllipsisAfterDefault,
RequiredAfterOptional,
ImportError,
}

impl AutofixSkipped {
fn message(&self) -> &'static str {
match self {
Self::EllipsisAfterDefault => {
"Fix unavailable: Cannot convert required parameter (ellipsis) after optional parameter"
}
Self::RequiredAfterOptional => {
"Fix unavailable: Cannot convert required parameter after optional parameter"
}
Self::ImportError => "Fix unavailable: Could not resolve import for `Annotated`",
}
}
}

/// FAST002
pub(crate) fn fastapi_non_annotated_dependency(
checker: &Checker,
Expand Down Expand Up @@ -251,8 +272,10 @@ fn create_diagnostic(
parameter.range,
);

let try_generate_fix = || {
let (import_edit, binding) = importer.import(parameter.range.start())?;
let try_generate_fix = || -> Result<Fix, AutofixSkipped> {
let (import_edit, binding) = importer
.import(parameter.range.start())
.map_err(|_| AutofixSkipped::ImportError)?;
Comment thread
ntBre marked this conversation as resolved.
Outdated

// Each of these classes takes a single, optional default
// argument, followed by kw-only arguments
Expand Down Expand Up @@ -284,7 +307,7 @@ fn create_diagnostic(

if is_default_argument_ellipsis && seen_default {
// For ellipsis after a parameter with default, can't remove the default
return Ok(None);
return Err(AutofixSkipped::EllipsisAfterDefault);
}

if !is_default_argument_ellipsis {
Expand Down Expand Up @@ -315,7 +338,7 @@ fn create_diagnostic(
}
_ => {
if seen_default {
return Ok(None);
return Err(AutofixSkipped::RequiredAfterOptional);
}
format!(
"{parameter_name}: {binding}[{annotation}, {default_}]",
Expand All @@ -326,17 +349,21 @@ fn create_diagnostic(
}
};
let parameter_edit = Edit::range_replacement(content, parameter.range);
Ok(Some(Fix::unsafe_edits(import_edit, [parameter_edit])))
Ok(Fix::unsafe_edits(import_edit, [parameter_edit]))
};

// make sure we set `seen_default` if we bail out of `try_generate_fix` early. we could
// `match` on the result directly, but still calling `try_set_optional_fix` avoids
// duplicating the debug logging here
let fix: anyhow::Result<Option<Fix>> = try_generate_fix();
if fix.is_err() {
seen_default = true;
match try_generate_fix() {
Ok(fix) => {
diagnostic.set_fix(fix);
}
Err(reason) => {
diagnostic.info(reason.message());
seen_default = true;
}
}
diagnostic.try_set_optional_fix(|| fix);

seen_default
}
1 change: 1 addition & 0 deletions ...s/ruff_linter__rules__fastapi__tests__fast-api-non-annotated-dependency_FAST002_0.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,3 +358,4 @@ FAST002 FastAPI dependency without `Annotated`
72 | pass
|
help: Replace with `typing.Annotated`
info: Fix unavailable: Cannot convert required parameter after optional parameter
1 change: 1 addition & 0 deletions ...f_linter__rules__fastapi__tests__fast-api-non-annotated-dependency_FAST002_0.py_py38.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,3 +358,4 @@ FAST002 FastAPI dependency without `Annotated`
72 | pass
|
help: Replace with `typing_extensions.Annotated`
info: Fix unavailable: Cannot convert required parameter after optional parameter

@MichaReiser MichaReiser Jan 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the messaging here a bit confusing. It almost sounds as if there's no way to fix this violation (I'm also not fully convinced that we should add a sub-diagnostic like this, or that we should only show it when running ruff with -v).

Maybe something like: info: automatic fix is not available because

@11happy 11happy Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I agree these can be worded better, let me update.

@11happy 11happy Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, changed the messages to be a little bit more verbose & user friendly.

@11happy 11happy Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also leaning towards making these subdiagnostics available in case of verbose mode(-v)

@ntBre ntBre Jan 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yeah maybe this wasn't a great suggestion for a sub-diagnostic, sorry! We could also consider just adding a Fix availability note to the documentation instead.

Alternatively, it might be more helpful to phrase the sub-diagnostic as a suggestion like "Reorder the arguments to enable an automatic fix" (but probably more specific than that).

Just a couple of ideas, I'm curious what y'all think.

@MichaReiser MichaReiser Jan 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that users face the same issue as the fix -- they have to reorder the arguments or assign a default): Should we add a message making users aware that they have to be mindful of the default argument?

This makes the message useful beyond just saying that the auto-fix isn't available.

@11happy 11happy Jan 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, worded the message a little more clear with suggestion to reorder the arguments.

1 change: 1 addition & 0 deletions ...s/ruff_linter__rules__fastapi__tests__fast-api-non-annotated-dependency_FAST002_2.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ FAST002 FastAPI dependency without `Annotated`
85 | another_optional_param: int = Query(42, description="Another optional"),
|
help: Replace with `typing.Annotated`
info: Fix unavailable: Cannot convert required parameter (ellipsis) after optional parameter

FAST002 [*] FastAPI dependency without `Annotated`
--> FAST002_2.py:85:5
Expand Down
1 change: 1 addition & 0 deletions ...f_linter__rules__fastapi__tests__fast-api-non-annotated-dependency_FAST002_2.py_py38.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ FAST002 FastAPI dependency without `Annotated`
85 | another_optional_param: int = Query(42, description="Another optional"),
|
help: Replace with `typing_extensions.Annotated`
info: Fix unavailable: Cannot convert required parameter (ellipsis) after optional parameter

FAST002 [*] FastAPI dependency without `Annotated`
--> FAST002_2.py:85:5
Expand Down
Loading