-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implement RUF027: Missing F-String Syntax
lint
#9728
Conversation
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
e566318
to
5525b24
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent.
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good. I mainly left a few nit comments and some suggestions for more tests (I fear, some of them will be painful)
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
.../ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF027_RUF027.py.snap
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Outdated
Show resolved
Hide resolved
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
RUF027 | 91 | 91 | 0 | 0 | 0 |
CodSpeed Performance ReportMerging #9728 will not alter performanceComparing Summary
|
Really weird false positive They're basically doing |
Here's another false positive They're doing |
This false positive is no good e.g. The vast majority of the false positives in the ecosystem are this. I think we should also not flag |
This false positive is due to some inline JavaScript written as a string I think that's okay... |
I think this ecosystem check might be outdated, this should be fixed. |
Sweet. It'll update when the build succeeds. |
This is probably the best approach, honestly. |
…he literal parts of f-strings
…erals with f-string parts
9d4336e
to
664be28
Compare
Summary
Fixes #8151
This PR implements a new rule,
RUF027
.What it does
Checks for strings that contain f-string syntax but are not f-strings.
Why is this bad?
An f-string missing an
f
at the beginning won't format anything, and instead treat the interpolation syntax as literal.Example
It should instead be:
Heuristics
Since there are many possible string literals which contain syntax similar to f-strings yet are not intended to be,
this lint will disqualify any literal that satisfies any of the following conditions:
format("Message: {value}", value = "Hello World")
)"{value}".format(...)
){...}
expression sections, or uses invalid f-string syntax.Test Plan
I created a new test file,
RUF027.py
, which is both an example of what the lint should catch and a way to test edge cases that may trigger false positives.