Skip to content

[FastAPI] Avoid introducing invalid syntax in fix for fast-api-non-annotated-dependency (FAST002) - #13133

Merged
AlexWaygood merged 2 commits into
astral-sh:mainfrom
arkuhn:akuhn/FAST002_autofix
Aug 28, 2024
Merged

AlexWaygood merged 2 commits into
astral-sh:mainfrom
arkuhn:akuhn/FAST002_autofix

Conversation

@arkuhn

@arkuhn arkuhn commented Aug 28, 2024 •

Copy link
Copy Markdown
Contributor

Summary

  • Context: this rule swaps out default function argument values for type annotations
  • Problem: When this swap occurs after a default argument value is already present, the function signature is no longer valid python code
  • Fix: Track default arguments and do not recommend a fix if we hit this condition, making this fix only sometimes possible.

Fixes #12982

Addtl:

  • Breakout helper functions

Test Plan

# 3 new test cases demonstrating default value scenarios
cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/fastapi/FAST002.py  --no-cache --preview --select FAST002

cargo test

  - Context: this rule swaps out default function argument values
for type annotations
  - Problem: When this swap occurs after a default argument value is
    already present, the function signature is no longer valid python
code (astral-sh#12982)
  - Fix: Track default arguments and bail if we hit this condition,
    making this fix only sometimes possible.
@arkuhn
arkuhn force-pushed the akuhn/FAST002_autofix branch from eff5e04 to dc94dd4 Compare August 28, 2024 05:10
@github-actions

github-actions Bot commented Aug 28, 2024 •

Copy link
Copy Markdown
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood self-assigned this Aug 28, 2024

@AlexWaygood AlexWaygood left a comment

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.

Thanks! This seems like a reasonable fix to me

Comment thread crates/ruff_linter/src/rules/fastapi/rules/fastapi_non_annotated_dependency.rs Outdated
@AlexWaygood AlexWaygood changed the title [FAST002] fix - do not generate invalid args [FastAPI] Avoid introducing invalid syntax in fix for fast-api-non-annotated-dependency (FAST002) Aug 28, 2024
@AlexWaygood AlexWaygood added bug An issue describing something that isn't working, or a PR that fixes a bug fixes Related to suggested fixes for violations labels Aug 28, 2024
@AlexWaygood
AlexWaygood enabled auto-merge (squash) August 28, 2024 15:26
@AlexWaygood
AlexWaygood merged commit df694ca into astral-sh:main Aug 28, 2024
@codspeed

codspeed Bot commented Aug 28, 2024

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #13133 will improve performances by 7.61%

Comparing arkuhn:akuhn/FAST002_autofix (8e2aaf3) with main (2e75cfb)

Summary

⚡ 1 improvements
✅ 31 untouched benchmarks

Benchmarks breakdown

Benchmark main arkuhn:akuhn/FAST002_autofix Change
⚡ linter/all-rules[numpy/globals.py] 784.7 µs 729.2 µs +7.61%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue describing something that isn't working, or a PR that fixes a bug fixes Related to suggested fixes for violations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FAST002] FastAPI dependency without Annotated unsafe fix error

2 participants