From 17693ee8c3dc9974a1ba2c13581d254ff93acab9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 25 Feb 2024 18:02:21 -0500 Subject: [PATCH] Detect along path --- .../resources/test/fixtures/ruff/RUF027_1.py | 1 + .../ruff/rules/missing_fstring_syntax.rs | 62 +++++++++---------- 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py index ac8fcf11a0227..964147300ba11 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py @@ -28,6 +28,7 @@ def negative_cases(): 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(_("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)) diff --git a/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs b/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs index 4f461ee4a43e8..583a51947398d 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs @@ -61,28 +61,18 @@ pub(crate) fn missing_fstring_syntax( locator: &Locator, semantic: &SemanticModel, ) { - 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; - } - } + // 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, + _ => {} } - _ => {} + } + + // We also want to avoid expressions that are intended to be translated. + if semantic.current_expressions().any(is_gettext) { + return; } if should_be_fstring(literal, locator, semantic) { @@ -92,18 +82,24 @@ 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: &ast::Expr) -> bool { - match func { - ast::Expr::Name(ast::ExprName { id, .. }) => id == "_", - _ => false, - } +/// Returns `true` if an expression appears to be a `gettext` call. +/// +/// We want to avoid statement expressions and assignments related to aliases +/// of the gettext API. +/// +/// See 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_gettext(expr: &ast::Expr) -> bool { + let ast::Expr::Call(ast::ExprCall { func, .. }) = expr else { + return false; + }; + let ast::Expr::Name(ast::ExprName { id, .. }) = func.as_ref() else { + return false; + }; + matches!(id.as_str(), "_" | "gettext" | "ngettext") } /// Returns `true` if `literal` is likely an f-string with a missing `f` prefix.