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 8b312cef97..b0563d3fda 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py @@ -45,3 +45,17 @@ def negative_cases(): import django.utils.translations y = django.utils.translations.gettext("This {should} be understood as a translation string too!") + + # Calling `gettext.install()` literall monkey-patches `builtins._ = ...`, + # so even the fully qualified access of `builtins._()` should be considered + # a possible `gettext` call. + import builtins + another = 42 + z = builtins._("{another} translation string") + + # Usually logging strings use `%`-style string interpolation, + # but `logging` can be configured to use `{}` the same as f-strings, + # so these should also be ignored. + # See https://docs.python.org/3/howto/logging-cookbook.html#formatting-styles + import logging + logging.info("yet {another} non-f-string") diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index ff80afe538..789d5e0704 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1077,12 +1077,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { } if checker.enabled(Rule::MissingFStringSyntax) { for string_literal in value.literals() { - ruff::rules::missing_fstring_syntax( - &mut checker.diagnostics, - string_literal, - checker.locator, - &checker.semantic, - ); + ruff::rules::missing_fstring_syntax(checker, string_literal); } } } @@ -1378,12 +1373,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { } if checker.enabled(Rule::MissingFStringSyntax) { for string_literal in value.as_slice() { - ruff::rules::missing_fstring_syntax( - &mut checker.diagnostics, - string_literal, - checker.locator, - &checker.semantic, - ); + ruff::rules::missing_fstring_syntax(checker, string_literal); } } } 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 dca53174d5..7331673035 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 @@ -1,14 +1,18 @@ -use memchr::memchr2_iter; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast}; use ruff_python_literal::format::FormatSpec; use ruff_python_parser::parse_expression; +use ruff_python_semantic::analyze::logging; use ruff_python_semantic::SemanticModel; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange}; + +use memchr::memchr2_iter; use rustc_hash::FxHashSet; +use crate::checkers::ast::Checker; + /// ## What it does /// Searches for strings that look like they were meant to be f-strings, but are missing an `f` prefix. /// @@ -59,12 +63,9 @@ impl AlwaysFixableViolation for MissingFStringSyntax { } /// RUF027 -pub(crate) fn missing_fstring_syntax( - diagnostics: &mut Vec, - literal: &ast::StringLiteral, - locator: &Locator, - semantic: &SemanticModel, -) { +pub(crate) fn missing_fstring_syntax(checker: &mut Checker, literal: &ast::StringLiteral) { + let semantic = checker.semantic(); + // 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() { @@ -75,20 +76,27 @@ pub(crate) fn missing_fstring_syntax( } // We also want to avoid expressions that are intended to be translated. - if semantic - .current_expressions() - .any(|expr| is_gettext(expr, semantic)) - { + if semantic.current_expressions().any(|expr| { + is_gettext(expr, semantic) + || is_logger_call(expr, semantic, &checker.settings.logger_objects) + }) { return; } - if should_be_fstring(literal, locator, semantic) { + if should_be_fstring(literal, checker.locator(), semantic) { let diagnostic = Diagnostic::new(MissingFStringSyntax, literal.range()) .with_fix(fix_fstring_syntax(literal.range())); - diagnostics.push(diagnostic); + checker.diagnostics.push(diagnostic); } } +fn is_logger_call(expr: &ast::Expr, semantic: &SemanticModel, logger_objects: &[String]) -> bool { + let ast::Expr::Call(ast::ExprCall { func, .. }) = expr else { + return false; + }; + logging::is_logger_candidate(func, semantic, logger_objects) +} + /// Returns `true` if an expression appears to be a `gettext` call. /// /// We want to avoid statement expressions and assignments related to aliases @@ -123,7 +131,7 @@ fn is_gettext(expr: &ast::Expr, semantic: &SemanticModel) -> bool { .is_some_and(|qualified_name| { matches!( qualified_name.segments(), - ["gettext", "gettext" | "ngettext"] + ["gettext", "gettext" | "ngettext"] | ["builtins", "_"] ) }) }