Skip to content

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

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

[FastAPI] Avoid introducing invalid syntax in fix for fast-api-non-annotated-dependency (FAST002)#13133
AlexWaygood merged 2 commits intoastral-sh:mainfrom
arkuhn:akuhn/FAST002_autofix

Conversation

@arkuhn
Copy link
Copy Markdown
Contributor

@arkuhn arkuhn commented Aug 28, 2024 •
edited by AlexWaygood
Loading

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

@arkuhn
[FAST002] fix - do not generate invalid args
dc94dd4 
  - 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
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 28, 2024 •
edited
Loading

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 approved these changes Aug 28, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@AlexWaygood AlexWaygood left a comment

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

@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 Something isn't working 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-hq
Copy link
Copy Markdown

codspeed-hq bot commented Aug 28, 2024

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%

@BrewTestBot BrewTestBot mentioned this pull request Aug 29, 2024
Merged
@TomerBin TomerBin mentioned this pull request Nov 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

Assignees

@AlexWaygood AlexWaygood

Labels

bug Something isn't working fixes Related to suggested fixes for violations

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FAST002] FastAPI dependency without Annotated unsafe fix error

2 participants

@arkuhn @AlexWaygood