Skip to content

Commit 0aded52

Browse files
committed
Stabilize FURB169 preview behavior (#16666)
## Summary This PR stabilizes the preview behavior introduced in #15905 The behavior change is that the rule now also recognizes `type(expr) is type(None)` comparisons where `expr` isn't a name expression. For example, the rule now detects `type(a.b) is type(None)` and suggests rewriting the comparison to `a.b is None`. The new behavior was introduced with Ruff 0.9.5 (6th of February), about a month ago. There are no open issues or PRs related to this rule (or behavior change).
1 parent 7af5b98 commit 0aded52

File tree

4 files changed

+99
-383
lines changed

4 files changed

+99
-383
lines changed

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

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ mod tests {
1212
use test_case::test_case;
1313

1414
use crate::registry::Rule;
15-
use crate::settings::types::PreviewMode;
1615
use crate::test::test_path;
1716
use crate::{assert_messages, settings};
1817

@@ -62,24 +61,6 @@ mod tests {
6261
Ok(())
6362
}
6463

65-
#[test_case(Rule::TypeNoneComparison, Path::new("FURB169.py"))]
66-
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
67-
let snapshot = format!(
68-
"preview__{}_{}",
69-
rule_code.noqa_code(),
70-
path.to_string_lossy()
71-
);
72-
let diagnostics = test_path(
73-
Path::new("refurb").join(path).as_path(),
74-
&settings::LinterSettings {
75-
preview: PreviewMode::Enabled,
76-
..settings::LinterSettings::for_rule(rule_code)
77-
},
78-
)?;
79-
assert_messages!(snapshot, diagnostics);
80-
Ok(())
81-
}
82-
8364
#[test]
8465
fn write_whole_file_python_39() -> Result<()> {
8566
let diagnostics = test_path(

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ use crate::rules::refurb::helpers::replace_with_identity_check;
1313
/// There is only ever one instance of `None`, so it is more efficient and
1414
/// readable to use the `is` operator to check if an object is `None`.
1515
///
16-
/// Only name expressions (e.g., `type(foo) == type(None)`) are reported.
17-
/// In [preview], the rule will also report other kinds of expressions.
18-
///
1916
/// ## Example
2017
/// ```python
2118
/// type(obj) is type(None)
@@ -34,8 +31,6 @@ use crate::rules::refurb::helpers::replace_with_identity_check;
3431
/// - [Python documentation: `None`](https://docs.python.org/3/library/constants.html#None)
3532
/// - [Python documentation: `type`](https://docs.python.org/3/library/functions.html#type)
3633
/// - [Python documentation: Identity comparisons](https://docs.python.org/3/reference/expressions.html#is-not)
37-
///
38-
/// [preview]: https://docs.astral.sh/ruff/preview/
3934
#[derive(ViolationMetadata)]
4035
pub(crate) struct TypeNoneComparison {
4136
replacement: IdentityCheck,
@@ -80,12 +75,6 @@ pub(crate) fn type_none_comparison(checker: &Checker, compare: &ast::ExprCompare
8075
_ => return,
8176
};
8277

83-
if checker.settings.preview.is_disabled()
84-
&& !matches!(other_arg, Expr::Name(_) | Expr::NoneLiteral(_))
85-
{
86-
return;
87-
}
88-
8978
let diagnostic = Diagnostic::new(TypeNoneComparison { replacement }, compare.range);
9079

9180
let negate = replacement == IdentityCheck::IsNot;

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB169_FURB169.py.snap

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,4 +251,102 @@ FURB169.py:27:1: FURB169 [*] When checking against `None`, use `is not` instead
251251
27 |+None is not None
252252
28 28 |
253253
29 29 | type(a.b) is type(None)
254-
30 30 |
254+
30 30 |
255+
256+
FURB169.py:29:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
257+
|
258+
27 | type(None) != type(None)
259+
28 |
260+
29 | type(a.b) is type(None)
261+
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
262+
30 |
263+
31 | type(
264+
|
265+
= help: Replace with `is None`
266+
267+
Safe fix
268+
26 26 |
269+
27 27 | type(None) != type(None)
270+
28 28 |
271+
29 |-type(a.b) is type(None)
272+
29 |+a.b is None
273+
30 30 |
274+
31 31 | type(
275+
32 32 | a(
276+
277+
FURB169.py:31:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
278+
|
279+
29 | type(a.b) is type(None)
280+
30 |
281+
31 | / type(
282+
32 | | a(
283+
33 | | # Comment
284+
34 | | )
285+
35 | | ) != type(None)
286+
| |_______________^ FURB169
287+
36 |
288+
37 | type(
289+
|
290+
= help: Replace with `is not None`
291+
292+
Unsafe fix
293+
28 28 |
294+
29 29 | type(a.b) is type(None)
295+
30 30 |
296+
31 |-type(
297+
32 |- a(
298+
33 |- # Comment
299+
34 |- )
300+
35 |-) != type(None)
301+
31 |+a() is not None
302+
36 32 |
303+
37 33 | type(
304+
38 34 | a := 1
305+
306+
FURB169.py:37:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
307+
|
308+
35 | ) != type(None)
309+
36 |
310+
37 | / type(
311+
38 | | a := 1
312+
39 | | ) == type(None)
313+
| |_______________^ FURB169
314+
40 |
315+
41 | type(
316+
|
317+
= help: Replace with `is None`
318+
319+
Safe fix
320+
34 34 | )
321+
35 35 | ) != type(None)
322+
36 36 |
323+
37 |-type(
324+
38 |- a := 1
325+
39 |-) == type(None)
326+
37 |+(a := 1) is None
327+
40 38 |
328+
41 39 | type(
329+
42 40 | a for a in range(0)
330+
331+
FURB169.py:41:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
332+
|
333+
39 | ) == type(None)
334+
40 |
335+
41 | / type(
336+
42 | | a for a in range(0)
337+
43 | | ) is not type(None)
338+
| |___________________^ FURB169
339+
|
340+
= help: Replace with `is not None`
341+
342+
Safe fix
343+
38 38 | a := 1
344+
39 39 | ) == type(None)
345+
40 40 |
346+
41 |-type(
347+
42 |- a for a in range(0)
348+
43 |-) is not type(None)
349+
41 |+(a for a in range(0)) is not None
350+
44 42 |
351+
45 43 |
352+
46 44 | # Ok.

0 commit comments

Comments
 (0)