Stabilize FURB169 preview behavior (#16666)

## Summary

This PR stabilizes the preview behavior introduced in
https://github.com/astral-sh/ruff/pull/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).
This commit is contained in:
Micha Reiser 2025-03-13 08:43:29 +01:00
parent 7af5b98606
commit 0aded52c40
4 changed files with 99 additions and 383 deletions

View file

@ -12,7 +12,6 @@ mod tests {
use test_case::test_case; use test_case::test_case;
use crate::registry::Rule; use crate::registry::Rule;
use crate::settings::types::PreviewMode;
use crate::test::test_path; use crate::test::test_path;
use crate::{assert_messages, settings}; use crate::{assert_messages, settings};
@ -62,24 +61,6 @@ mod tests {
Ok(()) Ok(())
} }
#[test_case(Rule::TypeNoneComparison, Path::new("FURB169.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("refurb").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Enabled,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
#[test] #[test]
fn write_whole_file_python_39() -> Result<()> { fn write_whole_file_python_39() -> Result<()> {
let diagnostics = test_path( let diagnostics = test_path(

View file

@ -13,9 +13,6 @@ use crate::rules::refurb::helpers::replace_with_identity_check;
/// There is only ever one instance of `None`, so it is more efficient and /// There is only ever one instance of `None`, so it is more efficient and
/// readable to use the `is` operator to check if an object is `None`. /// readable to use the `is` operator to check if an object is `None`.
/// ///
/// Only name expressions (e.g., `type(foo) == type(None)`) are reported.
/// In [preview], the rule will also report other kinds of expressions.
///
/// ## Example /// ## Example
/// ```python /// ```python
/// type(obj) is type(None) /// type(obj) is type(None)
@ -34,8 +31,6 @@ use crate::rules::refurb::helpers::replace_with_identity_check;
/// - [Python documentation: `None`](https://docs.python.org/3/library/constants.html#None) /// - [Python documentation: `None`](https://docs.python.org/3/library/constants.html#None)
/// - [Python documentation: `type`](https://docs.python.org/3/library/functions.html#type) /// - [Python documentation: `type`](https://docs.python.org/3/library/functions.html#type)
/// - [Python documentation: Identity comparisons](https://docs.python.org/3/reference/expressions.html#is-not) /// - [Python documentation: Identity comparisons](https://docs.python.org/3/reference/expressions.html#is-not)
///
/// [preview]: https://docs.astral.sh/ruff/preview/
#[derive(ViolationMetadata)] #[derive(ViolationMetadata)]
pub(crate) struct TypeNoneComparison { pub(crate) struct TypeNoneComparison {
replacement: IdentityCheck, replacement: IdentityCheck,
@ -80,12 +75,6 @@ pub(crate) fn type_none_comparison(checker: &Checker, compare: &ast::ExprCompare
_ => return, _ => return,
}; };
if checker.settings.preview.is_disabled()
&& !matches!(other_arg, Expr::Name(_) | Expr::NoneLiteral(_))
{
return;
}
let diagnostic = Diagnostic::new(TypeNoneComparison { replacement }, compare.range); let diagnostic = Diagnostic::new(TypeNoneComparison { replacement }, compare.range);
let negate = replacement == IdentityCheck::IsNot; let negate = replacement == IdentityCheck::IsNot;

View file

@ -252,3 +252,101 @@ FURB169.py:27:1: FURB169 [*] When checking against `None`, use `is not` instead
28 28 | 28 28 |
29 29 | type(a.b) is type(None) 29 29 | type(a.b) is type(None)
30 30 | 30 30 |
FURB169.py:29:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
27 | type(None) != type(None)
28 |
29 | type(a.b) is type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
30 |
31 | type(
|
= help: Replace with `is None`
Safe fix
26 26 |
27 27 | type(None) != type(None)
28 28 |
29 |-type(a.b) is type(None)
29 |+a.b is None
30 30 |
31 31 | type(
32 32 | a(
FURB169.py:31:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
29 | type(a.b) is type(None)
30 |
31 | / type(
32 | | a(
33 | | # Comment
34 | | )
35 | | ) != type(None)
| |_______________^ FURB169
36 |
37 | type(
|
= help: Replace with `is not None`
Unsafe fix
28 28 |
29 29 | type(a.b) is type(None)
30 30 |
31 |-type(
32 |- a(
33 |- # Comment
34 |- )
35 |-) != type(None)
31 |+a() is not None
36 32 |
37 33 | type(
38 34 | a := 1
FURB169.py:37:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
35 | ) != type(None)
36 |
37 | / type(
38 | | a := 1
39 | | ) == type(None)
| |_______________^ FURB169
40 |
41 | type(
|
= help: Replace with `is None`
Safe fix
34 34 | )
35 35 | ) != type(None)
36 36 |
37 |-type(
38 |- a := 1
39 |-) == type(None)
37 |+(a := 1) is None
40 38 |
41 39 | type(
42 40 | a for a in range(0)
FURB169.py:41:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
39 | ) == type(None)
40 |
41 | / type(
42 | | a for a in range(0)
43 | | ) is not type(None)
| |___________________^ FURB169
|
= help: Replace with `is not None`
Safe fix
38 38 | a := 1
39 39 | ) == type(None)
40 40 |
41 |-type(
42 |- a for a in range(0)
43 |-) is not type(None)
41 |+(a for a in range(0)) is not None
44 42 |
45 43 |
46 44 | # Ok.

View file

@ -1,352 +0,0 @@
---
source: crates/ruff_linter/src/rules/refurb/mod.rs
---
FURB169.py:5:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
3 | # Error.
4 |
5 | type(foo) is type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
6 |
7 | type(None) is type(foo)
|
= help: Replace with `is None`
Safe fix
2 2 |
3 3 | # Error.
4 4 |
5 |-type(foo) is type(None)
5 |+foo is None
6 6 |
7 7 | type(None) is type(foo)
8 8 |
FURB169.py:7:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
5 | type(foo) is type(None)
6 |
7 | type(None) is type(foo)
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
8 |
9 | type(None) is type(None)
|
= help: Replace with `is None`
Safe fix
4 4 |
5 5 | type(foo) is type(None)
6 6 |
7 |-type(None) is type(foo)
7 |+foo is None
8 8 |
9 9 | type(None) is type(None)
10 10 |
FURB169.py:9:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
7 | type(None) is type(foo)
8 |
9 | type(None) is type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB169
10 |
11 | type(foo) is not type(None)
|
= help: Replace with `is None`
Safe fix
6 6 |
7 7 | type(None) is type(foo)
8 8 |
9 |-type(None) is type(None)
9 |+None is None
10 10 |
11 11 | type(foo) is not type(None)
12 12 |
FURB169.py:11:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
9 | type(None) is type(None)
10 |
11 | type(foo) is not type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB169
12 |
13 | type(None) is not type(foo)
|
= help: Replace with `is not None`
Safe fix
8 8 |
9 9 | type(None) is type(None)
10 10 |
11 |-type(foo) is not type(None)
11 |+foo is not None
12 12 |
13 13 | type(None) is not type(foo)
14 14 |
FURB169.py:13:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
11 | type(foo) is not type(None)
12 |
13 | type(None) is not type(foo)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB169
14 |
15 | type(None) is not type(None)
|
= help: Replace with `is not None`
Safe fix
10 10 |
11 11 | type(foo) is not type(None)
12 12 |
13 |-type(None) is not type(foo)
13 |+foo is not None
14 14 |
15 15 | type(None) is not type(None)
16 16 |
FURB169.py:15:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
13 | type(None) is not type(foo)
14 |
15 | type(None) is not type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FURB169
16 |
17 | type(foo) == type(None)
|
= help: Replace with `is not None`
Safe fix
12 12 |
13 13 | type(None) is not type(foo)
14 14 |
15 |-type(None) is not type(None)
15 |+None is not None
16 16 |
17 17 | type(foo) == type(None)
18 18 |
FURB169.py:17:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
15 | type(None) is not type(None)
16 |
17 | type(foo) == type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
18 |
19 | type(None) == type(foo)
|
= help: Replace with `is None`
Safe fix
14 14 |
15 15 | type(None) is not type(None)
16 16 |
17 |-type(foo) == type(None)
17 |+foo is None
18 18 |
19 19 | type(None) == type(foo)
20 20 |
FURB169.py:19:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
17 | type(foo) == type(None)
18 |
19 | type(None) == type(foo)
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
20 |
21 | type(None) == type(None)
|
= help: Replace with `is None`
Safe fix
16 16 |
17 17 | type(foo) == type(None)
18 18 |
19 |-type(None) == type(foo)
19 |+foo is None
20 20 |
21 21 | type(None) == type(None)
22 22 |
FURB169.py:21:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
19 | type(None) == type(foo)
20 |
21 | type(None) == type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB169
22 |
23 | type(foo) != type(None)
|
= help: Replace with `is None`
Safe fix
18 18 |
19 19 | type(None) == type(foo)
20 20 |
21 |-type(None) == type(None)
21 |+None is None
22 22 |
23 23 | type(foo) != type(None)
24 24 |
FURB169.py:23:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
21 | type(None) == type(None)
22 |
23 | type(foo) != type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
24 |
25 | type(None) != type(foo)
|
= help: Replace with `is not None`
Safe fix
20 20 |
21 21 | type(None) == type(None)
22 22 |
23 |-type(foo) != type(None)
23 |+foo is not None
24 24 |
25 25 | type(None) != type(foo)
26 26 |
FURB169.py:25:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
23 | type(foo) != type(None)
24 |
25 | type(None) != type(foo)
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
26 |
27 | type(None) != type(None)
|
= help: Replace with `is not None`
Safe fix
22 22 |
23 23 | type(foo) != type(None)
24 24 |
25 |-type(None) != type(foo)
25 |+foo is not None
26 26 |
27 27 | type(None) != type(None)
28 28 |
FURB169.py:27:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
25 | type(None) != type(foo)
26 |
27 | type(None) != type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^^ FURB169
28 |
29 | type(a.b) is type(None)
|
= help: Replace with `is not None`
Safe fix
24 24 |
25 25 | type(None) != type(foo)
26 26 |
27 |-type(None) != type(None)
27 |+None is not None
28 28 |
29 29 | type(a.b) is type(None)
30 30 |
FURB169.py:29:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
27 | type(None) != type(None)
28 |
29 | type(a.b) is type(None)
| ^^^^^^^^^^^^^^^^^^^^^^^ FURB169
30 |
31 | type(
|
= help: Replace with `is None`
Safe fix
26 26 |
27 27 | type(None) != type(None)
28 28 |
29 |-type(a.b) is type(None)
29 |+a.b is None
30 30 |
31 31 | type(
32 32 | a(
FURB169.py:31:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
29 | type(a.b) is type(None)
30 |
31 | / type(
32 | | a(
33 | | # Comment
34 | | )
35 | | ) != type(None)
| |_______________^ FURB169
36 |
37 | type(
|
= help: Replace with `is not None`
Unsafe fix
28 28 |
29 29 | type(a.b) is type(None)
30 30 |
31 |-type(
32 |- a(
33 |- # Comment
34 |- )
35 |-) != type(None)
31 |+a() is not None
36 32 |
37 33 | type(
38 34 | a := 1
FURB169.py:37:1: FURB169 [*] When checking against `None`, use `is` instead of comparison with `type(None)`
|
35 | ) != type(None)
36 |
37 | / type(
38 | | a := 1
39 | | ) == type(None)
| |_______________^ FURB169
40 |
41 | type(
|
= help: Replace with `is None`
Safe fix
34 34 | )
35 35 | ) != type(None)
36 36 |
37 |-type(
38 |- a := 1
39 |-) == type(None)
37 |+(a := 1) is None
40 38 |
41 39 | type(
42 40 | a for a in range(0)
FURB169.py:41:1: FURB169 [*] When checking against `None`, use `is not` instead of comparison with `type(None)`
|
39 | ) == type(None)
40 |
41 | / type(
42 | | a for a in range(0)
43 | | ) is not type(None)
| |___________________^ FURB169
|
= help: Replace with `is not None`
Safe fix
38 38 | a := 1
39 39 | ) == type(None)
40 40 |
41 |-type(
42 |- a for a in range(0)
43 |-) is not type(None)
41 |+(a for a in range(0)) is not None
44 42 |
45 43 |
46 44 | # Ok.