Skip to content
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.

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

Consider raw source code for W605 #10480

Merged
merged 1 commit into from Mar 19, 2024
Merged

Consider raw source code for W605 #10480

merged 1 commit into from Mar 19, 2024

Conversation

dhruvmanila
Copy link
Member

Summary

This PR fixes a panic in the linter for W605.

Consider the following f-string:

f"{{}}ab"

The FStringMiddle token would contain {}ab. Notice that the escaped braces have reduced the string. This means we cannot use the text value from the token to determine the location of the escape sequence but need to extract it from the source code.

fixes: #10434

Test Plan

Add new test cases and update the snapshots.

@dhruvmanila dhruvmanila added the bug Something isn't working label Mar 19, 2024
Comment on lines -74 to +75
let Some(range) = indexer.fstring_ranges().innermost(token_range.start()) else {
let Some(f_string_range) = indexer.fstring_ranges().innermost(token_range.start())
else {
Copy link
Member Author

Choose a reason for hiding this comment

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

Just to be explicit that this is the entire f-string range.

Copy link

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@dhruvmanila dhruvmanila merged commit 42d4216 into main Mar 19, 2024
17 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/fix-w605 branch March 19, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule W605 cause panic
2 participants