[refurb] Implement isinstance-type-none (FURB168) (#8308)

## Summary

Implement
[`no-isinstance-type-none`](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/no_isinstance_type_none.py)
as `isinstance-type-none` (`FURB168`).

Auto-fixes calls to `isinstance` to check if an object is `None` to a
`None` identity check. For example,

```python
isinstance(foo, type(None))
```

becomes

```python
foo is None
```

Related to #1348.

## Test Plan

`cargo test`

---------

Co-authored-by: Charlie Marsh <[email protected]>

crates/ruff_linter/resources/test/fixtures/refurb/FURB168.py

@@ -0,0 +1,51 @@
+foo: object
+
+# Errors.
+
+if isinstance(foo, type(None)):
+    pass
+
+if isinstance(foo, (type(None))):
+    pass
+
+if isinstance(foo, (type(None), type(None), type(None))):
+    pass
+
+if isinstance(foo, None | None):
+    pass
+
+if isinstance(foo, (None | None)):
+    pass
+
+if isinstance(foo, None | type(None)):
+    pass
+
+if isinstance(foo, (None | type(None))):
+    pass
+
+# A bit contrived, but is both technically valid and equivalent to the above.
+if isinstance(foo, (type(None) | ((((type(None))))) | ((None | type(None))))):
+    pass
+
+
+# Okay.
+
+if isinstance(foo, int):
+    pass
+
+if isinstance(foo, (int)):
+    pass
+
+if isinstance(foo, (int, str)):
+    pass
+
+if isinstance(foo, (int, type(None), str)):
+    pass
+
+# This is a TypeError, which the rule ignores.
+if isinstance(foo, None):
+    pass
+
+# This is also a TypeError, which the rule ignores.
+if isinstance(foo, (None,)):
+    pass

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

@@ -911,6 +911,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::ExceptionWithoutExcInfo) {
                 flake8_logging::rules::exception_without_exc_info(checker, call);
             }
+            if checker.enabled(Rule::IsinstanceTypeNone) {
+                refurb::rules::isinstance_type_none(checker, call);
+            }
             if checker.enabled(Rule::ImplicitCwd) {
                 refurb::rules::no_implicit_cwd(checker, call);
             }

crates/ruff_linter/src/codes.rs

@@ -936,6 +936,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Refurb, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap),
         (Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
         (Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
+        (Refurb, "168") => (RuleGroup::Preview, rules::refurb::rules::IsinstanceTypeNone),
         (Refurb, "171") => (RuleGroup::Preview, rules::refurb::rules::SingleItemMembershipTest),
         (Refurb, "177") => (RuleGroup::Preview, rules::refurb::rules::ImplicitCwd),
 

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

@@ -24,6 +24,7 @@ mod tests {
     #[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]
     #[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))]
     #[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))]
+    #[test_case(Rule::IsinstanceTypeNone, Path::new("FURB168.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

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

@@ -0,0 +1,144 @@
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_python_ast::{self as ast, Expr, Operator};
+
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_codegen::Generator;
+use ruff_text_size::{Ranged, TextRange};
+
+use crate::checkers::ast::Checker;
+use crate::fix::edits::pad;
+
+/// ## What it does
+/// Checks for uses of `isinstance` that check if an object is of type `None`.
+///
+/// ## Why is this bad?
+/// 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`.
+///
+/// ## Example
+/// ```python
+/// isinstance(obj, type(None))
+/// ```
+///
+/// Use instead:
+/// ```python
+/// obj is None
+/// ```
+///
+/// ## References
+/// - [Python documentation: `isinstance`](https://docs.python.org/3/library/functions.html#isinstance)
+/// - [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: Identity comparisons](https://docs.python.org/3/reference/expressions.html#is-not)
+#[violation]
+pub struct IsinstanceTypeNone;
+
+impl Violation for IsinstanceTypeNone {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Prefer `is` operator over `isinstance` to check if an object is `None`")
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Replace with `is` operator".to_string())
+    }
+}
+
+/// FURB168
+pub(crate) fn isinstance_type_none(checker: &mut Checker, call: &ast::ExprCall) {
+    let Expr::Name(ast::ExprName { id, .. }) = call.func.as_ref() else {
+        return;
+    };
+    if id.as_str() != "isinstance" {
+        return;
+    }
+    if !checker.semantic().is_builtin(id) {
+        return;
+    }
+    let Some(types) = call.arguments.find_positional(1) else {
+        return;
+    };
+
+    if is_none(types) {
+        let Some(Expr::Name(ast::ExprName {
+            id: object_name, ..
+        })) = call.arguments.find_positional(0)
+        else {
+            return;
+        };
+        let mut diagnostic = Diagnostic::new(IsinstanceTypeNone, call.range());
+        let replacement = generate_replacement(object_name, checker.generator());
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+            pad(replacement, call.range(), checker.locator()),
+            call.range(),
+        )));
+        checker.diagnostics.push(diagnostic);
+    }
+}
+
+/// Returns `true` if the given expression is equivalent to checking if the
+/// object type is `None` when used with the `isinstance` builtin.
+fn is_none(expr: &Expr) -> bool {
+    fn inner(expr: &Expr, in_union_context: bool) -> bool {
+        match expr {
+            // Ex) `None`
+            // Note: `isinstance` only accepts `None` as a type when used with
+            // the union operator, so we need to check if we're in a union.
+            Expr::Constant(ast::ExprConstant { value, .. }) if in_union_context => value.is_none(),
+
+            // Ex) `type(None)`
+            Expr::Call(ast::ExprCall {
+                func, arguments, ..
+            }) if arguments.len() == 1 => {
+                if let Expr::Name(ast::ExprName { id, .. }) = func.as_ref() {
+                    if id.as_str() == "type" {
+                        if let Some(Expr::Constant(ast::ExprConstant { value, .. })) =
+                            arguments.args.get(0)
+                        {
+                            return value.is_none();
+                        }
+                    }
+                }
+                false
+            }
+
+            // Ex) `(type(None),)`
+            Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().all(|elt| inner(elt, false)),
+
+            // Ex) `type(None) | type(None)`
+            Expr::BinOp(ast::ExprBinOp {
+                left,
+                op: Operator::BitOr,
+                right,
+                ..
+            }) => inner(left, true) && inner(right, true),
+
+            // Otherwise, return false.
+            _ => false,
+        }
+    }
+    inner(expr, false)
+}
+
+/// Format a code snippet comparing `name` to `None` (e.g., `name is None`).
+fn generate_replacement(name: &str, generator: Generator) -> String {
+    // Construct `name`.
+    let var = ast::ExprName {
+        id: name.to_string(),
+        ctx: ast::ExprContext::Load,
+        range: TextRange::default(),
+    };
+    // Construct `name is None`.
+    let compare = ast::ExprCompare {
+        left: Box::new(var.into()),
+        ops: vec![ast::CmpOp::Is],
+        comparators: vec![ast::Expr::Constant(ast::ExprConstant {
+            value: ast::Constant::None,
+            range: TextRange::default(),
+        })],
+        range: TextRange::default(),
+    };
+    generator.expr(&compare.into())
+}

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

@@ -1,6 +1,7 @@
 pub(crate) use check_and_remove_from_set::*;
 pub(crate) use delete_full_slice::*;
 pub(crate) use implicit_cwd::*;
+pub(crate) use isinstance_type_none::*;
 pub(crate) use print_empty_string::*;
 pub(crate) use read_whole_file::*;
 pub(crate) use reimplemented_starmap::*;
@@ -12,6 +13,7 @@ pub(crate) use unnecessary_enumerate::*;
 mod check_and_remove_from_set;
 mod delete_full_slice;
 mod implicit_cwd;
+mod isinstance_type_none;
 mod print_empty_string;
 mod read_whole_file;
 mod reimplemented_starmap;