Skip to content

Commit

Permalink
Detect along path
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Feb 25, 2024
1 parent 431dcd8 commit 17693ee
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
62 changes: 29 additions & 33 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,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) {
Expand All @@ -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 <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_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.
Expand Down

0 comments on commit 17693ee

Please sign in to comment.