RUF027: Ignore template strings passed to logging calls and `builtins._()` calls (#12889)

This commit is contained in:
Alex Waygood 2024-08-14 11:27:35 +01:00 committed by GitHub
parent bebed67bf1
commit c487149b7d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 38 additions and 26 deletions

View File

@ -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")

View File

@ -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);
}
}
}

View File

@ -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<Diagnostic>,
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", "_"]
)
})
}