Skip to content

Commit

Permalink
Fix false-positive in RUF027
Browse files Browse the repository at this point in the history
  • Loading branch information
robincaloudis committed Feb 25, 2024
1 parent 6fe15e7 commit f75fb21
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 6 deletions.
3 changes: 3 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ def negative_cases():
json3 = "{ 'positive': 'false' }"
alternative_formatter("{a}", a=5)
formatted = "{a}".fmt(a=7)
partial = "partial sentence"
a = _("formatting of {partial} in a translation string is bad practice")
_("formatting of {partial} in a translation string is bad practice")
print(do_nothing("{a}".format(a=3)))
print(do_nothing(alternative_formatter("{a}", a=5)))
print(format(do_nothing("{a}"), a=5))
Expand Down
47 changes: 41 additions & 6 deletions crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,28 @@ pub(crate) fn missing_fstring_syntax(
locator: &Locator,
semantic: &SemanticModel,
) {
// we want to avoid statement expressions that are just a string literal.
// there's no reason to have standalone f-strings and this lets us avoid docstrings too
if let ast::Stmt::Expr(ast::StmtExpr { value, .. }) = semantic.current_statement() {
match value.as_ref() {
ast::Expr::StringLiteral(_) | ast::Expr::FString(_) => return,
_ => {}
match semantic.current_statement() {
ast::Stmt::Expr(ast::StmtExpr { value, .. }) => {
match value.as_ref() {
// We want to avoid statement expressions that are just a string literal.
// There's no reason to have standalone f-strings and this lets us avoid docstrings too.
ast::Expr::StringLiteral(_) | ast::Expr::FString(_) => return,
ast::Expr::Call(ast::ExprCall { func, .. }) => {
if is_translation_string(func) {
return;
}
}
_ => {}
}
}
ast::Stmt::Assign(ast::StmtAssign { value, .. }) => {
if let ast::Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() {
if is_translation_string(func) {
return;
}
}
}
_ => {}
}

if should_be_fstring(literal, locator, semantic) {
Expand All @@ -77,6 +92,26 @@ pub(crate) fn missing_fstring_syntax(
}
}

// It is a convention to use the `_()` alias for `gettext()`. We want to avoid
// statement expressions and assignments related to aliases of the gettext API.
// See https://docs.python.org/3/library/gettext.html for details. When one
// uses `_() to mark a string for translation, the tools look for these markers
// and replace the original string with its translated counterpart. If the
// string contains variable placeholders or formatting, it can complicate the
// translation process, lead to errors or incorrect translations.
fn is_translation_string(func: &Box<ast::Expr>) -> bool {
match func.as_ref() {
ast::Expr::Name(ast::ExprName { id, .. }) => {
if id == "_" {
true
} else {
false
}
}
_ => false,
}
}

/// Returns `true` if `literal` is likely an f-string with a missing `f` prefix.
/// See [`MissingFStringSyntax`] for the validation criteria.
fn should_be_fstring(
Expand Down

0 comments on commit f75fb21

Please sign in to comment.