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

Fix RUF027 false positives if gettext is imported using an alias #12025

Merged
merged 4 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 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 @@ -38,3 +38,10 @@ def negative_cases():
print(("{a}" "{c}").format(a=1, c=2))
print("{a}".attribute.chaining.call(a=2))
print("{a} {c}".format(a))

from gettext import gettext as foo
should = 42
x = foo("This {should} also be understood as a translation string")

import django.utils.translations
y = django.utils.translations.gettext("This {should} be understood as a translation string too!")
38 changes: 31 additions & 7 deletions crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ pub(crate) fn missing_fstring_syntax(
}

// We also want to avoid expressions that are intended to be translated.
if semantic.current_expressions().any(is_gettext) {
if semantic
.current_expressions()
.any(|expr| is_gettext(expr, semantic))
{
return;
}

Expand All @@ -92,14 +95,33 @@ pub(crate) fn missing_fstring_syntax(
/// 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 {
fn is_gettext(expr: &ast::Expr, semantic: &SemanticModel) -> 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;

let short_circuit = match func.as_ref() {
ast::Expr::Name(ast::ExprName { id, .. }) => {
matches!(id.as_str(), "gettext" | "ngettext" | "_")
}
ast::Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
matches!(attr.as_str(), "gettext" | "ngettext")
}
_ => false,
};
matches!(id.as_str(), "_" | "gettext" | "ngettext")

if short_circuit {
return true;
}

semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["gettext", "gettext" | "ngettext"]
)
})
}

/// Returns `true` if `literal` is likely an f-string with a missing `f` prefix.
Expand All @@ -119,7 +141,7 @@ fn should_be_fstring(
};

// Note: Range offsets for `value` are based on `fstring_expr`
let Some(ast::ExprFString { value, .. }) = parsed.expr().as_f_string_expr() else {
let ast::Expr::FString(ast::ExprFString { value, .. }) = parsed.expr() else {
return false;
};

Expand Down Expand Up @@ -203,7 +225,9 @@ fn should_be_fstring(
fn has_brackets(possible_fstring: &str) -> bool {
// this qualifies rare false positives like "{ unclosed bracket"
// but it's faster in the general case
memchr2_iter(b'{', b'}', possible_fstring.as_bytes()).count() > 1
memchr2_iter(b'{', b'}', possible_fstring.as_bytes())
.nth(1)
.is_some()
}

fn fix_fstring_syntax(range: TextRange) -> Fix {
Expand Down
Loading