From ea86edf12e54ea3addc9c9babcdc1d62a7e652fa Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 2 Mar 2023 15:50:10 -0500 Subject: [PATCH] Rename, etc. --- .../test/fixtures/pyupgrade/UP038.py | 9 ++- crates/ruff/src/checkers/ast.rs | 2 +- crates/ruff/src/codes.rs | 2 +- crates/ruff/src/registry.rs | 2 +- crates/ruff/src/rules/pyupgrade/mod.rs | 2 +- crates/ruff/src/rules/pyupgrade/rules/mod.rs | 2 +- .../pyupgrade/rules/use_pep604_isinstance.rs | 65 ++++++++++++++----- ...ff__rules__pyupgrade__tests__UP038.py.snap | 6 +- docs/configuration.md | 2 + 9 files changed, 67 insertions(+), 25 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyupgrade/UP038.py b/crates/ruff/resources/test/fixtures/pyupgrade/UP038.py index c608e4c251..5bfbf30b17 100644 --- a/crates/ruff/resources/test/fixtures/pyupgrade/UP038.py +++ b/crates/ruff/resources/test/fixtures/pyupgrade/UP038.py @@ -1,2 +1,7 @@ -isinstance(1, (int, float)) -issubclass("yes", (int, float, str)) +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 6db2576fea..7ccf91e364 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -2584,7 +2584,7 @@ where if self.settings.rules.enabled(&Rule::OSErrorAlias) { pyupgrade::rules::os_error_alias(self, &expr); } - if self.settings.rules.enabled(&Rule::IsInstanceTypingUnion) { + if self.settings.rules.enabled(&Rule::IsinstanceWithTuple) { pyupgrade::rules::use_pep604_isinstance(self, expr, func, args); } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 15d9ed8326..cade353efc 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -344,7 +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::IsInstanceTypingUnion, + (Pyupgrade, "038") => Rule::IsinstanceWithTuple, // pydocstyle (Pydocstyle, "100") => Rule::PublicModule, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 9232c61dfb..2c6e7ef9c7 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -331,7 +331,7 @@ ruff_macros::register_rules!( rules::pyupgrade::rules::ImportReplacements, rules::pyupgrade::rules::OutdatedVersionBlock, rules::pyupgrade::rules::QuotedAnnotation, - rules::pyupgrade::rules::IsInstanceTypingUnion, + 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 eb7da3960a..d4e1cbd10f 100644 --- a/crates/ruff/src/rules/pyupgrade/mod.rs +++ b/crates/ruff/src/rules/pyupgrade/mod.rs @@ -66,7 +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::IsInstanceTypingUnion, Path::new("UP038.py"); "UP038")] + #[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 f540acd0ed..a50fd8ac67 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/mod.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/mod.rs @@ -39,7 +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, IsInstanceTypingUnion}; +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}; diff --git a/crates/ruff/src/rules/pyupgrade/rules/use_pep604_isinstance.rs b/crates/ruff/src/rules/pyupgrade/rules/use_pep604_isinstance.rs index a1179fa7a8..4611303fac 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/use_pep604_isinstance.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/use_pep604_isinstance.rs @@ -1,5 +1,7 @@ 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; @@ -8,14 +10,41 @@ 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 IsInstanceTypingUnion; + pub struct IsinstanceWithTuple { + pub kind: CallKind, + } ); -impl AlwaysAutofixableViolation for IsInstanceTypingUnion { +impl AlwaysAutofixableViolation for IsinstanceWithTuple { #[derive_message_formats] fn message(&self) -> String { - format!("Use `X | Y` for type annotations") + format!("Use `X | Y` in `{}` call instead of `(X, Y)`", self.kind) } fn autofix_title(&self) -> String { @@ -41,20 +70,24 @@ fn union(elts: &[Expr]) -> Expr { pub fn use_pep604_isinstance(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { if let ExprKind::Name { id, .. } = &func.node { - if (id == "isinstance" || id == "issubclass") && checker.is_builtin(id) { - if let Some(types) = args.get(1) { - if let ExprKind::Tuple { elts, .. } = &types.node { - let mut diagnostic = - Diagnostic::new(IsInstanceTypingUnion, 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); + 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 index c9cab4030a..098c753706 100644 --- 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 @@ -3,7 +3,8 @@ source: crates/ruff/src/rules/pyupgrade/mod.rs expression: diagnostics --- - kind: - IsInstanceTypingUnion: ~ + IsinstanceWithTuple: + kind: Isinstance location: row: 1 column: 0 @@ -20,7 +21,8 @@ expression: diagnostics column: 26 parent: ~ - kind: - IsInstanceTypingUnion: ~ + IsinstanceWithTuple: + kind: Issubclass location: row: 2 column: 0 diff --git a/docs/configuration.md b/docs/configuration.md index 4417c093e6..5587637c91 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -206,6 +206,8 @@ Options: Run in watch mode by re-running whenever files change --fix-only Fix any fixable lint violations, but don't report on leftover violations. Implies `--fix` + --ignore-noqa + Ignore any `# noqa` comments --format Output serialization format for violations [env: RUFF_FORMAT=] [possible values: text, json, junit, grouped, github, gitlab, pylint] --target-version