From 886992c6c2dff1e8dba44ee6a92ffaa52d21291d Mon Sep 17 00:00:00 2001 From: Martin Lehoux Date: Thu, 2 Mar 2023 21:59:15 +0100 Subject: [PATCH] Replace tuples with type union in isinstance or issubclass calls (#3280) --- .../test/fixtures/pyupgrade/UP038.py | 7 ++ crates/ruff/src/checkers/ast.rs | 3 + crates/ruff/src/codes.rs | 1 + crates/ruff/src/registry.rs | 1 + crates/ruff/src/rules/pyupgrade/mod.rs | 1 + crates/ruff/src/rules/pyupgrade/rules/mod.rs | 2 + .../pyupgrade/rules/use_pep604_isinstance.rs | 94 +++++++++++++++++++ ...ff__rules__pyupgrade__tests__UP038.py.snap | 41 ++++++++ ruff.schema.json | 1 + 9 files changed, 151 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/pyupgrade/UP038.py create mode 100644 crates/ruff/src/rules/pyupgrade/rules/use_pep604_isinstance.rs create mode 100644 crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP038.py.snap diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP038.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP038.py new file mode 100644 index 0000000000..5bfbf30b17 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP038.py @@ -0,0 +1,7 @@ +isinstance(1, (int, float)) # UP038 +issubclass("yes", (int, float, str)) # UP038 + +isinstance(1, int) # OK +issubclass("yes", int) # OK +isinstance(1, int | float) # OK +issubclass("yes", int | str) # OK diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 2497ed5109..7ccf91e364 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -2584,6 +2584,9 @@ where if self.settings.rules.enabled(&Rule::OSErrorAlias) { pyupgrade::rules::os_error_alias(self, &expr); } + if self.settings.rules.enabled(&Rule::IsinstanceWithTuple) { + pyupgrade::rules::use_pep604_isinstance(self, expr, func, args); + } // flake8-print if self.settings.rules.enabled(&Rule::PrintFound) diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 28f11f2605..cade353efc 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -344,6 +344,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Pyupgrade, "035") => Rule::ImportReplacements, (Pyupgrade, "036") => Rule::OutdatedVersionBlock, (Pyupgrade, "037") => Rule::QuotedAnnotation, + (Pyupgrade, "038") => Rule::IsinstanceWithTuple, // pydocstyle (Pydocstyle, "100") => Rule::PublicModule, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 49c4f84f7f..2c6e7ef9c7 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -331,6 +331,7 @@ ruff_macros::register_rules!( rules::pyupgrade::rules::ImportReplacements, rules::pyupgrade::rules::OutdatedVersionBlock, rules::pyupgrade::rules::QuotedAnnotation, + rules::pyupgrade::rules::IsinstanceWithTuple, // pydocstyle rules::pydocstyle::rules::PublicModule, rules::pydocstyle::rules::PublicClass, diff --git a/crates/ruff/src/rules/pyupgrade/mod.rs b/crates/ruff/src/rules/pyupgrade/mod.rs index 4154fe583f..d4e1cbd10f 100644 --- a/crates/ruff/src/rules/pyupgrade/mod.rs +++ b/crates/ruff/src/rules/pyupgrade/mod.rs @@ -66,6 +66,7 @@ mod tests { #[test_case(Rule::OutdatedVersionBlock, Path::new("UP036_3.py"); "UP036_3")] #[test_case(Rule::OutdatedVersionBlock, Path::new("UP036_4.py"); "UP036_4")] #[test_case(Rule::QuotedAnnotation, Path::new("UP037.py"); "UP037")] + #[test_case(Rule::IsinstanceWithTuple, Path::new("UP038.py"); "UP038")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = path.to_string_lossy().to_string(); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/pyupgrade/rules/mod.rs b/crates/ruff/src/rules/pyupgrade/rules/mod.rs index 081d8701a9..a50fd8ac67 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/mod.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/mod.rs @@ -39,6 +39,7 @@ pub(crate) use unnecessary_future_import::{unnecessary_future_import, Unnecessar pub(crate) use unpack_list_comprehension::{unpack_list_comprehension, RewriteListComprehension}; pub(crate) use use_pep585_annotation::{use_pep585_annotation, DeprecatedCollectionType}; pub(crate) use use_pep604_annotation::{use_pep604_annotation, TypingUnion}; +pub(crate) use use_pep604_isinstance::{use_pep604_isinstance, IsinstanceWithTuple}; pub(crate) use useless_metaclass_type::{useless_metaclass_type, UselessMetaclassType}; pub(crate) use useless_object_inheritance::{useless_object_inheritance, UselessObjectInheritance}; @@ -75,5 +76,6 @@ mod unnecessary_future_import; mod unpack_list_comprehension; mod use_pep585_annotation; mod use_pep604_annotation; +mod use_pep604_isinstance; mod useless_metaclass_type; mod useless_object_inheritance; diff --git a/crates/ruff/src/rules/pyupgrade/rules/use_pep604_isinstance.rs b/crates/ruff/src/rules/pyupgrade/rules/use_pep604_isinstance.rs new file mode 100644 index 0000000000..4611303fac --- /dev/null +++ b/crates/ruff/src/rules/pyupgrade/rules/use_pep604_isinstance.rs @@ -0,0 +1,94 @@ +use ruff_macros::{define_violation, derive_message_formats}; +use rustpython_parser::ast::{Expr, ExprKind, Location, Operator}; +use serde::{Deserialize, Serialize}; +use std::fmt; + +use crate::ast::helpers::unparse_expr; +use crate::ast::types::Range; +use crate::checkers::ast::Checker; +use crate::fix::Fix; +use crate::registry::Diagnostic; +use crate::violation::AlwaysAutofixableViolation; + +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] +pub enum CallKind { + Isinstance, + Issubclass, +} + +impl fmt::Display for CallKind { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match self { + CallKind::Isinstance => fmt.write_str("isinstance"), + CallKind::Issubclass => fmt.write_str("issubclass"), + } + } +} + +impl CallKind { + pub fn from_name(name: &str) -> Option { + match name { + "isinstance" => Some(CallKind::Isinstance), + "issubclass" => Some(CallKind::Issubclass), + _ => None, + } + } +} + +define_violation!( + // TODO: document referencing [PEP 604]: https://peps.python.org/pep-0604/ + pub struct IsinstanceWithTuple { + pub kind: CallKind, + } +); +impl AlwaysAutofixableViolation for IsinstanceWithTuple { + #[derive_message_formats] + fn message(&self) -> String { + format!("Use `X | Y` in `{}` call instead of `(X, Y)`", self.kind) + } + + fn autofix_title(&self) -> String { + "Convert to `X | Y`".to_string() + } +} + +fn union(elts: &[Expr]) -> Expr { + if elts.len() == 1 { + elts[0].clone() + } else { + Expr::new( + Location::default(), + Location::default(), + ExprKind::BinOp { + left: Box::new(union(&elts[..elts.len() - 1])), + op: Operator::BitOr, + right: Box::new(elts[elts.len() - 1].clone()), + }, + ) + } +} + +pub fn use_pep604_isinstance(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { + if let ExprKind::Name { id, .. } = &func.node { + let Some(kind) = CallKind::from_name(id) else { + return; + }; + if !checker.is_builtin(id) { + return; + }; + if let Some(types) = args.get(1) { + if let ExprKind::Tuple { elts, .. } = &types.node { + let mut diagnostic = + Diagnostic::new(IsinstanceWithTuple { kind }, Range::from_located(expr)); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.amend(Fix::replacement( + unparse_expr(&union(elts), checker.stylist), + types.location, + types.end_location.unwrap(), + )); + } + checker.diagnostics.push(diagnostic); + } + } + } +} diff --git a/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP038.py.snap b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP038.py.snap new file mode 100644 index 0000000000..098c753706 --- /dev/null +++ b/crates/ruff/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP038.py.snap @@ -0,0 +1,41 @@ +--- +source: crates/ruff/src/rules/pyupgrade/mod.rs +expression: diagnostics +--- +- kind: + IsinstanceWithTuple: + kind: Isinstance + location: + row: 1 + column: 0 + end_location: + row: 1 + column: 27 + fix: + content: int | float + location: + row: 1 + column: 14 + end_location: + row: 1 + column: 26 + parent: ~ +- kind: + IsinstanceWithTuple: + kind: Issubclass + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 36 + fix: + content: int | float | str + location: + row: 2 + column: 18 + end_location: + row: 2 + column: 35 + parent: ~ + diff --git a/ruff.schema.json b/ruff.schema.json index 82c130e102..37b4dc9553 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2122,6 +2122,7 @@ "UP035", "UP036", "UP037", + "UP038", "W", "W1", "W19",