Skip to content

Commit f6416d0

Browse files
danparizherntBre
danparizher
and
ntBre
authored
[flake8-gettext] Fix false negatives for plural argument of ngettext (INT001, INT002, INT003) (#21078)
## Summary Fixes #21062. Extends the `INT001`, `INT002`, and `INT003` lint rules to check both the singular and plural arguments of `ngettext` function calls, fixing false negatives where formatting issues in the plural argument were not being detected. ## Problem Analysis The three rules (`f-string-in-get-text-func-call`, `format-in-get-text-func-call`, and `printf-in-get-text-func-call`) only checked the first argument (singular) of `ngettext` calls but ignored the second argument (plural). This caused false negatives when f-strings, `.format()` calls, or `%` formatting were used in the plural argument. The root cause was: 1. Rule functions only examined `args.first()` 2. No logic to determine if a function call was specifically `ngettext` vs `gettext` 3. Missing checks for `args.get(1)` (the second argument) Example of the bug: ```python ngettext(f"one item", f"{n} items", n) # Only flagged first f-string, not second ``` ## Approach 1. **Modified function signatures**: Updated all three rule functions to accept the `func` parameter 2. **Added ngettext detection**: Created `is_ngettext_call()` helper that checks: - Direct name references (e.g., `ngettext(...)`) - Qualified names ending with `ngettext` (e.g., `gettext_mod.ngettext(...)`) 3. **Extended argument checking**: For `ngettext` calls, now check both first and second arguments 4. **Code refactoring**: Created shared `helpers.rs` module to eliminate duplication 5. **Updated main checker**: Modified expression analyzer to pass `func` parameter --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
1 parent 9d00f3d commit f6416d0

9 files changed

Lines changed: 237 additions & 31 deletions

‎crates/ruff_linter/src/checkers/ast/analyze/expression.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,13 +1046,13 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
10461046
&checker.settings().flake8_gettext.function_names,
10471047
) {
10481048
if checker.is_rule_enabled(Rule::FStringInGetTextFuncCall) {
1049-
flake8_gettext::rules::f_string_in_gettext_func_call(checker, args);
1049+
flake8_gettext::rules::f_string_in_gettext_func_call(checker, func, args);
10501050
}
10511051
if checker.is_rule_enabled(Rule::FormatInGetTextFuncCall) {
1052-
flake8_gettext::rules::format_in_gettext_func_call(checker, args);
1052+
flake8_gettext::rules::format_in_gettext_func_call(checker, func, args);
10531053
}
10541054
if checker.is_rule_enabled(Rule::PrintfInGetTextFuncCall) {
1055-
flake8_gettext::rules::printf_in_gettext_func_call(checker, args);
1055+
flake8_gettext::rules::printf_in_gettext_func_call(checker, func, args);
10561056
}
10571057
}
10581058
if checker.is_rule_enabled(Rule::UncapitalizedEnvironmentVariables) {

‎crates/ruff_linter/src/preview.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,3 +282,8 @@ pub(crate) const fn is_standalone_mock_non_existent_enabled(settings: &LinterSet
282282
pub(crate) const fn is_up024_precise_highlighting_enabled(settings: &LinterSettings) -> bool {
283283
settings.preview.is_enabled()
284284
}
285+
286+
// https://github.com/astral-sh/ruff/pull/21078
287+
pub(crate) const fn is_plural_ngettext_check_enabled(settings: &LinterSettings) -> bool {
288+
settings.preview.is_enabled()
289+
}

‎crates/ruff_linter/src/rules/flake8_gettext/mod.rs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,35 @@
11
//! Rules from [flake8-gettext](https://pypi.org/project/flake8-gettext/).
22
use crate::checkers::ast::Checker;
3-
use crate::preview::is_extended_i18n_function_matching_enabled;
3+
use crate::preview::{
4+
is_extended_i18n_function_matching_enabled, is_plural_ngettext_check_enabled,
5+
};
46
use ruff_python_ast::name::Name;
57
use ruff_python_ast::{self as ast, Expr};
68
use ruff_python_semantic::Modules;
79

810
pub(crate) mod rules;
911
pub mod settings;
1012

13+
/// Returns true if the function call is ngettext
14+
pub(crate) fn is_ngettext_call(checker: &Checker, func: &Expr) -> bool {
15+
if !is_plural_ngettext_check_enabled(checker.settings()) {
16+
return false;
17+
}
18+
19+
// Check direct name reference first (e.g., `ngettext(...)`)
20+
if let Some(name) = func.as_name_expr() {
21+
if name.id == "ngettext" {
22+
return true;
23+
}
24+
}
25+
26+
// Check qualified name (e.g., `gettext_mod.ngettext(...)`)
27+
checker
28+
.semantic()
29+
.resolve_qualified_name(func)
30+
.is_some_and(|qualified_name| matches!(qualified_name.segments(), [.., "ngettext"]))
31+
}
32+
1133
/// Returns true if the [`Expr`] is an internationalization function call.
1234
pub(crate) fn is_gettext_func_call(
1335
checker: &Checker,

‎crates/ruff_linter/src/rules/flake8_gettext/rules/f_string_in_gettext_func_call.rs‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use ruff_text_size::Ranged;
55

66
use crate::Violation;
77
use crate::checkers::ast::Checker;
8+
use crate::rules::flake8_gettext::is_ngettext_call;
89

910
/// ## What it does
1011
/// Checks for f-strings in `gettext` function calls.
@@ -46,20 +47,36 @@ use crate::checkers::ast::Checker;
4647
/// - [Python documentation: `gettext` — Multilingual internationalization services](https://docs.python.org/3/library/gettext.html)
4748
#[derive(ViolationMetadata)]
4849
#[violation_metadata(stable_since = "v0.0.260")]
49-
pub(crate) struct FStringInGetTextFuncCall;
50+
pub(crate) struct FStringInGetTextFuncCall {
51+
is_plural: bool,
52+
}
5053

5154
impl Violation for FStringInGetTextFuncCall {
5255
#[derive_message_formats]
5356
fn message(&self) -> String {
54-
"f-string is resolved before function call; consider `_(\"string %s\") % arg`".to_string()
57+
if self.is_plural {
58+
"f-string in plural argument is resolved before function call".to_string()
59+
} else {
60+
"f-string is resolved before function call; consider `_(\"string %s\") % arg`"
61+
.to_string()
62+
}
5563
}
5664
}
5765

5866
/// INT001
59-
pub(crate) fn f_string_in_gettext_func_call(checker: &Checker, args: &[Expr]) {
67+
pub(crate) fn f_string_in_gettext_func_call(checker: &Checker, func: &Expr, args: &[Expr]) {
68+
// Check first argument (singular)
6069
if let Some(first) = args.first() {
6170
if first.is_f_string_expr() {
62-
checker.report_diagnostic(FStringInGetTextFuncCall {}, first.range());
71+
checker.report_diagnostic(FStringInGetTextFuncCall { is_plural: false }, first.range());
6372
}
6473
}
74+
75+
// Check second argument (plural) for ngettext calls
76+
if is_ngettext_call(checker, func)
77+
&& let Some(second) = args.get(1)
78+
&& second.is_f_string_expr()
79+
{
80+
checker.report_diagnostic(FStringInGetTextFuncCall { is_plural: true }, second.range());
81+
}
6582
}

‎crates/ruff_linter/src/rules/flake8_gettext/rules/format_in_gettext_func_call.rs‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
use ruff_python_ast::{self as ast, Expr};
1+
use ruff_python_ast::Expr;
22

33
use ruff_macros::{ViolationMetadata, derive_message_formats};
44
use ruff_text_size::Ranged;
55

66
use crate::Violation;
77
use crate::checkers::ast::Checker;
8+
use crate::rules::flake8_gettext::is_ngettext_call;
89

910
/// ## What it does
1011
/// Checks for `str.format` calls in `gettext` function calls.
@@ -46,24 +47,45 @@ use crate::checkers::ast::Checker;
4647
/// - [Python documentation: `gettext` — Multilingual internationalization services](https://docs.python.org/3/library/gettext.html)
4748
#[derive(ViolationMetadata)]
4849
#[violation_metadata(stable_since = "v0.0.260")]
49-
pub(crate) struct FormatInGetTextFuncCall;
50+
pub(crate) struct FormatInGetTextFuncCall {
51+
is_plural: bool,
52+
}
5053

5154
impl Violation for FormatInGetTextFuncCall {
5255
#[derive_message_formats]
5356
fn message(&self) -> String {
54-
"`format` method argument is resolved before function call; consider `_(\"string %s\") % arg`".to_string()
57+
if self.is_plural {
58+
"`format` method in plural argument is resolved before function call".to_string()
59+
} else {
60+
"`format` method argument is resolved before function call; consider `_(\"string %s\") % arg`".to_string()
61+
}
5562
}
5663
}
5764

5865
/// INT002
59-
pub(crate) fn format_in_gettext_func_call(checker: &Checker, args: &[Expr]) {
66+
pub(crate) fn format_in_gettext_func_call(checker: &Checker, func: &Expr, args: &[Expr]) {
67+
// Check first argument (singular)
6068
if let Some(first) = args.first() {
61-
if let Expr::Call(ast::ExprCall { func, .. }) = &first {
62-
if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func.as_ref() {
63-
if attr == "format" {
64-
checker.report_diagnostic(FormatInGetTextFuncCall {}, first.range());
65-
}
66-
}
69+
if is_format_call(first) {
70+
checker.report_diagnostic(FormatInGetTextFuncCall { is_plural: false }, first.range());
6771
}
6872
}
73+
74+
// Check second argument (plural) for ngettext calls
75+
if is_ngettext_call(checker, func)
76+
&& let Some(second) = args.get(1)
77+
&& is_format_call(second)
78+
{
79+
checker.report_diagnostic(FormatInGetTextFuncCall { is_plural: true }, second.range());
80+
}
81+
}
82+
83+
/// Return `true` if `expr` is a call to the `format` attribute, as in
84+
/// `s.format(...)`.
85+
fn is_format_call(expr: &Expr) -> bool {
86+
expr.as_call_expr().is_some_and(|call| {
87+
call.func
88+
.as_attribute_expr()
89+
.is_some_and(|attr| &attr.attr == "format")
90+
})
6991
}

‎crates/ruff_linter/src/rules/flake8_gettext/rules/printf_in_gettext_func_call.rs‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use ruff_text_size::Ranged;
44

55
use crate::Violation;
66
use crate::checkers::ast::Checker;
7+
use crate::rules::flake8_gettext::is_ngettext_call;
78

89
/// ## What it does
910
/// Checks for printf-style formatted strings in `gettext` function calls.
@@ -45,28 +46,51 @@ use crate::checkers::ast::Checker;
4546
/// - [Python documentation: `gettext` — Multilingual internationalization services](https://docs.python.org/3/library/gettext.html)
4647
#[derive(ViolationMetadata)]
4748
#[violation_metadata(stable_since = "v0.0.260")]
48-
pub(crate) struct PrintfInGetTextFuncCall;
49+
pub(crate) struct PrintfInGetTextFuncCall {
50+
is_plural: bool,
51+
}
4952

5053
impl Violation for PrintfInGetTextFuncCall {
5154
#[derive_message_formats]
5255
fn message(&self) -> String {
53-
"printf-style format is resolved before function call; consider `_(\"string %s\") % arg`"
54-
.to_string()
56+
if self.is_plural {
57+
"printf-style format in plural argument is resolved before function call".to_string()
58+
} else {
59+
"printf-style format is resolved before function call; consider `_(\"string %s\") % arg`"
60+
.to_string()
61+
}
5562
}
5663
}
5764

5865
/// INT003
59-
pub(crate) fn printf_in_gettext_func_call(checker: &Checker, args: &[Expr]) {
66+
pub(crate) fn printf_in_gettext_func_call(checker: &Checker, func: &Expr, args: &[Expr]) {
67+
// Check first argument (singular)
6068
if let Some(first) = args.first() {
61-
if let Expr::BinOp(ast::ExprBinOp {
62-
op: Operator::Mod,
63-
left,
64-
..
65-
}) = &first
66-
{
67-
if left.is_string_literal_expr() {
68-
checker.report_diagnostic(PrintfInGetTextFuncCall {}, first.range());
69-
}
69+
if is_printf_format(first) {
70+
checker.report_diagnostic(PrintfInGetTextFuncCall { is_plural: false }, first.range());
7071
}
7172
}
73+
74+
// Check second argument (plural) for ngettext calls
75+
if is_ngettext_call(checker, func)
76+
&& let Some(second) = args.get(1)
77+
&& is_printf_format(second)
78+
{
79+
checker.report_diagnostic(PrintfInGetTextFuncCall { is_plural: true }, second.range());
80+
}
81+
}
82+
83+
/// Return `true` if `expr` is a printf-style formatting expression, such as
84+
/// `"%s" % 123`.
85+
fn is_printf_format(expr: &Expr) -> bool {
86+
let Expr::BinOp(ast::ExprBinOp {
87+
op: Operator::Mod,
88+
left,
89+
..
90+
}) = expr
91+
else {
92+
return false;
93+
};
94+
95+
left.is_string_literal_expr()
7296
}

‎crates/ruff_linter/src/rules/flake8_gettext/snapshots/ruff_linter__rules__flake8_gettext__tests__preview__f-string-in-get-text-func-call_INT001.py.snap‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@ INT001 f-string is resolved before function call; consider `_("string %s") % arg
3030
9 | _gettext(f"{'value'}") # no lint
3131
|
3232

33+
INT001 f-string in plural argument is resolved before function call
34+
--> INT001.py:7:24
35+
|
36+
6 | gettext(f"{'value'}")
37+
7 | ngettext(f"{'value'}", f"{'values'}", 2)
38+
| ^^^^^^^^^^^^^
39+
8 |
40+
9 | _gettext(f"{'value'}") # no lint
41+
|
42+
3343
INT001 f-string is resolved before function call; consider `_("string %s") % arg`
3444
--> INT001.py:22:21
3545
|
@@ -51,6 +61,16 @@ INT001 f-string is resolved before function call; consider `_("string %s") % arg
5161
25 | ngettext_fn(f"Hello, {name}!", f"Hello, {name}s!", 2)
5262
|
5363

64+
INT001 f-string in plural argument is resolved before function call
65+
--> INT001.py:23:41
66+
|
67+
22 | gettext_mod.gettext(f"Hello, {name}!")
68+
23 | gettext_mod.ngettext(f"Hello, {name}!", f"Hello, {name}s!", 2)
69+
| ^^^^^^^^^^^^^^^^^^
70+
24 | gettext_fn(f"Hello, {name}!")
71+
25 | ngettext_fn(f"Hello, {name}!", f"Hello, {name}s!", 2)
72+
|
73+
5474
INT001 f-string is resolved before function call; consider `_("string %s") % arg`
5575
--> INT001.py:24:12
5676
|
@@ -70,6 +90,15 @@ INT001 f-string is resolved before function call; consider `_("string %s") % arg
7090
| ^^^^^^^^^^^^^^^^^
7191
|
7292

93+
INT001 f-string in plural argument is resolved before function call
94+
--> INT001.py:25:32
95+
|
96+
23 | gettext_mod.ngettext(f"Hello, {name}!", f"Hello, {name}s!", 2)
97+
24 | gettext_fn(f"Hello, {name}!")
98+
25 | ngettext_fn(f"Hello, {name}!", f"Hello, {name}s!", 2)
99+
| ^^^^^^^^^^^^^^^^^^
100+
|
101+
73102
INT001 f-string is resolved before function call; consider `_("string %s") % arg`
74103
--> INT001.py:31:14
75104
|
@@ -160,3 +189,12 @@ INT001 f-string is resolved before function call; consider `_("string %s") % arg
160189
53 | builtins.ngettext(f"{'value'}", f"{'values'}", 2)
161190
| ^^^^^^^^^^^^
162191
|
192+
193+
INT001 f-string in plural argument is resolved before function call
194+
--> INT001.py:53:33
195+
|
196+
51 | builtins._(f"{'value'}")
197+
52 | builtins.gettext(f"{'value'}")
198+
53 | builtins.ngettext(f"{'value'}", f"{'values'}", 2)
199+
| ^^^^^^^^^^^^^
200+
|

‎crates/ruff_linter/src/rules/flake8_gettext/snapshots/ruff_linter__rules__flake8_gettext__tests__preview__format-in-get-text-func-call_INT002.py.snap‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,17 @@ INT002 `format` method argument is resolved before function call; consider `_("s
3030
5 | _gettext("{}".format("line")) # no lint
3131
|
3232

33+
INT002 `format` method in plural argument is resolved before function call
34+
--> INT002.py:3:31
35+
|
36+
1 | _("{}".format("line"))
37+
2 | gettext("{}".format("line"))
38+
3 | ngettext("{}".format("line"), "{}".format("lines"), 2)
39+
| ^^^^^^^^^^^^^^^^^^^^
40+
4 |
41+
5 | _gettext("{}".format("line")) # no lint
42+
|
43+
3344
INT002 `format` method argument is resolved before function call; consider `_("string %s") % arg`
3445
--> INT002.py:18:21
3546
|
@@ -51,6 +62,16 @@ INT002 `format` method argument is resolved before function call; consider `_("s
5162
21 | ngettext_fn("Hello, {}!".format(name), "Hello, {}s!".format(name), 2)
5263
|
5364

65+
INT002 `format` method in plural argument is resolved before function call
66+
--> INT002.py:19:49
67+
|
68+
18 | gettext_mod.gettext("Hello, {}!".format(name))
69+
19 | gettext_mod.ngettext("Hello, {}!".format(name), "Hello, {}s!".format(name), 2)
70+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
71+
20 | gettext_fn("Hello, {}!".format(name))
72+
21 | ngettext_fn("Hello, {}!".format(name), "Hello, {}s!".format(name), 2)
73+
|
74+
5475
INT002 `format` method argument is resolved before function call; consider `_("string %s") % arg`
5576
--> INT002.py:20:12
5677
|
@@ -70,6 +91,15 @@ INT002 `format` method argument is resolved before function call; consider `_("s
7091
| ^^^^^^^^^^^^^^^^^^^^^^^^^
7192
|
7293

94+
INT002 `format` method in plural argument is resolved before function call
95+
--> INT002.py:21:40
96+
|
97+
19 | gettext_mod.ngettext("Hello, {}!".format(name), "Hello, {}s!".format(name), 2)
98+
20 | gettext_fn("Hello, {}!".format(name))
99+
21 | ngettext_fn("Hello, {}!".format(name), "Hello, {}s!".format(name), 2)
100+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
101+
|
102+
73103
INT002 `format` method argument is resolved before function call; consider `_("string %s") % arg`
74104
--> INT002.py:27:14
75105
|
@@ -160,3 +190,12 @@ INT002 `format` method argument is resolved before function call; consider `_("s
160190
49 | builtins.ngettext("{}".format("line"), "{}".format("lines"), 2)
161191
| ^^^^^^^^^^^^^^^^^^^
162192
|
193+
194+
INT002 `format` method in plural argument is resolved before function call
195+
--> INT002.py:49:40
196+
|
197+
47 | builtins._("{}".format("line"))
198+
48 | builtins.gettext("{}".format("line"))
199+
49 | builtins.ngettext("{}".format("line"), "{}".format("lines"), 2)
200+
| ^^^^^^^^^^^^^^^^^^^^
201+
|

0 commit comments

Comments
 (0)
⚡