Skip to content

Commit 4e83db4

Browse files
[pylint] Detect more exotic NaN literals in PLW0177 (astral-sh#18630)
Co-authored-by: Micha Reiser <[email protected]>
1 parent 136443b commit 4e83db4

File tree

7 files changed

+72
-30
lines changed

7 files changed

+72
-30
lines changed

crates/ruff_linter/resources/test/fixtures/pylint/nan_comparison.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,8 @@
9292
# OK
9393
if x == "nan":
9494
pass
95+
96+
# PLW0117
97+
# https://github.com/astral-sh/ruff/issues/18596
98+
assert x == float("-NaN ")
99+
assert x == float(" \n+nan \t")

crates/ruff_linter/src/linter.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ use crate::settings::{LinterSettings, TargetVersion, flags};
3939
use crate::source_kind::SourceKind;
4040
use crate::{Locator, directives, fs, warn_user_once};
4141

42+
pub(crate) mod float;
43+
4244
pub struct LinterResult {
4345
/// A collection of diagnostic messages generated by the linter.
4446
pub messages: Vec<Message>,
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
use ruff_python_ast as ast;
2+
3+
/// Checks if `expr` is a string literal that represents NaN.
4+
/// E.g., `"NaN"`, `"-nAn"`, `"+nan"`, or even `" -NaN \n \t"`
5+
/// Returns `None` if it's not. Else `Some("nan")`, `Some("-nan")`, or `Some("+nan")`.
6+
pub(crate) fn as_nan_float_string_literal(expr: &ast::Expr) -> Option<&'static str> {
7+
find_any_ignore_ascii_case(expr, &["nan", "+nan", "-nan"])
8+
}
9+
10+
/// Returns `true` if `expr` is a string literal that represents a non-finite float.
11+
/// E.g., `"NaN"`, "-inf", `"Infinity"`, or even `" +Inf \n \t"`.
12+
/// Return `None` if it's not. Else the lowercased, trimmed string literal,
13+
/// e.g., `Some("nan")`, `Some("-inf")`, or `Some("+infinity")`.
14+
pub(crate) fn as_non_finite_float_string_literal(expr: &ast::Expr) -> Option<&'static str> {
15+
find_any_ignore_ascii_case(
16+
expr,
17+
&[
18+
"nan",
19+
"+nan",
20+
"-nan",
21+
"inf",
22+
"+inf",
23+
"-inf",
24+
"infinity",
25+
"+infinity",
26+
"-infinity",
27+
],
28+
)
29+
}
30+
31+
fn find_any_ignore_ascii_case(expr: &ast::Expr, patterns: &[&'static str]) -> Option<&'static str> {
32+
let value = &expr.as_string_literal_expr()?.value;
33+
34+
let value = value.to_str().trim();
35+
patterns
36+
.iter()
37+
.find(|other| value.eq_ignore_ascii_case(other))
38+
.copied()
39+
}

crates/ruff_linter/src/rules/pylint/rules/nan_comparison.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
use ruff_macros::{ViolationMetadata, derive_message_formats};
2+
23
use ruff_python_ast::{self as ast, Expr};
34
use ruff_python_semantic::SemanticModel;
45
use ruff_text_size::Ranged;
56

67
use crate::Violation;
78
use crate::checkers::ast::Checker;
9+
use crate::linter::float::as_nan_float_string_literal;
810

911
/// ## What it does
1012
/// Checks for comparisons against NaN values.
@@ -113,14 +115,10 @@ fn is_nan_float(expr: &Expr, semantic: &SemanticModel) -> bool {
113115
return false;
114116
}
115117

116-
let [Expr::StringLiteral(ast::ExprStringLiteral { value, .. })] = &**args else {
118+
let [expr] = &**args else {
117119
return false;
118120
};
119-
120-
if !matches!(
121-
value.to_str(),
122-
"nan" | "NaN" | "NAN" | "Nan" | "nAn" | "naN" | "nAN" | "NAn"
123-
) {
121+
if as_nan_float_string_literal(expr).is_none() {
124122
return false;
125123
}
126124

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0177_nan_comparison.py.snap

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,3 +107,20 @@ nan_comparison.py:60:10: PLW0177 Comparing against a NaN value; use `math.isnan`
107107
61 |
108108
62 | # No errors
109109
|
110+
111+
nan_comparison.py:98:13: PLW0177 Comparing against a NaN value; use `math.isnan` instead
112+
|
113+
96 | # PLW0117
114+
97 | # https://github.com/astral-sh/ruff/issues/18596
115+
98 | assert x == float("-NaN ")
116+
| ^^^^^^^^^^^^^^ PLW0177
117+
99 | assert x == float(" \n+nan \t")
118+
|
119+
120+
nan_comparison.py:99:13: PLW0177 Comparing against a NaN value; use `math.isnan` instead
121+
|
122+
97 | # https://github.com/astral-sh/ruff/issues/18596
123+
98 | assert x == float("-NaN ")
124+
99 | assert x == float(" \n+nan \t")
125+
| ^^^^^^^^^^^^^^^^^^^^^ PLW0177
126+
|

crates/ruff_linter/src/rules/refurb/rules/unnecessary_from_float.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ pub(crate) fn unnecessary_from_float(checker: &Checker, call: &ExprCall) {
140140
let Some(float) = float.as_string_literal_expr() else {
141141
break 'short_circuit;
142142
};
143+
// FIXME: use `as_non_finite_float_string_literal` instead.
143144
if !matches!(
144145
float.value.to_str().to_lowercase().as_str(),
145146
"inf" | "-inf" | "infinity" | "-infinity" | "nan"

crates/ruff_linter/src/rules/refurb/rules/verbose_decimal_constructor.rs

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
use ruff_macros::{ViolationMetadata, derive_message_formats};
2+
23
use ruff_python_ast::{self as ast, Expr};
34
use ruff_python_trivia::PythonWhitespace;
45
use ruff_text_size::Ranged;
56
use std::borrow::Cow;
67

78
use crate::checkers::ast::Checker;
9+
use crate::linter::float::as_non_finite_float_string_literal;
810
use crate::{Edit, Fix, FixAvailability, Violation};
911

1012
/// ## What it does
@@ -150,35 +152,13 @@ pub(crate) fn verbose_decimal_constructor(checker: &Checker, call: &ast::ExprCal
150152
let [float] = arguments.args.as_ref() else {
151153
return;
152154
};
153-
let Some(float) = float.as_string_literal_expr() else {
155+
let Some(float_str) = as_non_finite_float_string_literal(float) else {
154156
return;
155157
};
156158

157-
let trimmed = float.value.to_str().trim();
158-
let mut matches_non_finite_keyword = false;
159-
for non_finite_keyword in [
160-
"inf",
161-
"+inf",
162-
"-inf",
163-
"infinity",
164-
"+infinity",
165-
"-infinity",
166-
"nan",
167-
"+nan",
168-
"-nan",
169-
] {
170-
if trimmed.eq_ignore_ascii_case(non_finite_keyword) {
171-
matches_non_finite_keyword = true;
172-
break;
173-
}
174-
}
175-
if !matches_non_finite_keyword {
176-
return;
177-
}
178-
179159
let mut replacement = checker.locator().slice(float).to_string();
180160
// `Decimal(float("-nan")) == Decimal("nan")`
181-
if trimmed.eq_ignore_ascii_case("-nan") {
161+
if float_str == "-nan" {
182162
// Here we do not attempt to remove just the '-' character.
183163
// It may have been encoded (e.g. as '\N{hyphen-minus}')
184164
// in the original source slice, and the added complexity

0 commit comments

Comments
 (0)