From cc736c3a51a83e47f5f89a9ccc8a20d196272634 Mon Sep 17 00:00:00 2001 From: GiGaGon <107241144+MeGaGiGaGon@users.noreply.github.com> Date: Tue, 1 Jul 2025 07:26:41 -0700 Subject: [PATCH] [`refurb`] Fix false positive on empty tuples (`FURB168`) (#19058) ## Summary This PR fixes #19047 / the [isinstance-type-none (FURB168)](https://docs.astral.sh/ruff/rules/isinstance-type-none/#isinstance-type-none-furb168) tuple false positive by adding a check if the tuple is empty to the code. I also noticed there was another false positive with the other tuple check in the same function, so I fixed it the same way. `Union[()]` is invalid at runtime with `TypeError: Cannot take a Union of no types.`, but it is accepted by `basedpyright` [playground](https://basedpyright.com/?pythonVersion=3.8&typeCheckingMode=all&code=GYJw9gtgBALgngBwJYDsDmUkQWEMoCqKSYKAsAFAgCmAbtQIYA2A%2BvAtQBREkoDanAJQBdQUA) and is equivalent to `Never`, so I fixed it anyways. I'm getting on a side tangent here, but it looks like MyPy doesn't accept it, and ty [playground](https://play.ty.dev/c2c468b6-38e4-4dd9-a9fa-0276e843e395) gives `@Todo`. ## Test Plan Added two test cases for the two false positives. [playground](https://play.ruff.rs/a53afc21-9a1d-4b9b-9346-abfbeabeb449) --- .../ruff_linter/resources/test/fixtures/refurb/FURB168.py | 7 +++++++ .../src/rules/refurb/rules/isinstance_type_none.rs | 7 +++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB168.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB168.py index 17238d520c..a9f31e3a78 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB168.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB168.py @@ -85,3 +85,10 @@ def _(): if isinstance(foo, type(None)): ... + +# https://github.com/astral-sh/ruff/issues/19047 +if isinstance(foo, ()): + pass + +if isinstance(foo, Union[()]): + pass diff --git a/crates/ruff_linter/src/rules/refurb/rules/isinstance_type_none.rs b/crates/ruff_linter/src/rules/refurb/rules/isinstance_type_none.rs index 793e0968ad..4ff1f1c4af 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/isinstance_type_none.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/isinstance_type_none.rs @@ -100,7 +100,9 @@ fn is_none(expr: &Expr, semantic: &SemanticModel) -> bool { } // Ex) `(type(None),)` - Expr::Tuple(tuple) => tuple.iter().all(|element| inner(element, false, semantic)), + Expr::Tuple(tuple) => { + !tuple.is_empty() && tuple.iter().all(|element| inner(element, false, semantic)) + } // Ex) `type(None) | type(None)` Expr::BinOp(ast::ExprBinOp { @@ -125,7 +127,8 @@ fn is_none(expr: &Expr, semantic: &SemanticModel) -> bool { match slice.as_ref() { Expr::Tuple(ast::ExprTuple { elts, .. }) => { - elts.iter().all(|element| inner(element, true, semantic)) + !elts.is_empty() + && elts.iter().all(|element| inner(element, true, semantic)) } slice => inner(slice, true, semantic), }