From 073eddb1d91f97a943703000243dc41d693d53c3 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 29 Nov 2023 22:22:04 -0500 Subject: [PATCH] Use Python version to determine typing rewrite safety (#8919) ## Summary These rewrites are only (potentially) unsafe on Python versions that predate their introduction into the standard library and grammar, so it seems correct to mark them as safe on those later versions. --- .../pyupgrade/rules/use_pep585_annotation.rs | 38 ++++++++++-- .../pyupgrade/rules/use_pep604_annotation.rs | 58 +++++++++++++------ 2 files changed, 71 insertions(+), 25 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep585_annotation.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep585_annotation.rs index 3b91cd6504a0b..36e10b0a1c1f6 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep585_annotation.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep585_annotation.rs @@ -1,6 +1,6 @@ use ruff_python_ast::Expr; -use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::compose_call_path; use ruff_python_semantic::analyze::typing::ModuleMember; @@ -8,6 +8,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::importer::ImportRequest; +use crate::settings::types::PythonVersion; /// ## What it does /// Checks for the use of generics that can be replaced with standard library @@ -43,6 +44,11 @@ use crate::importer::ImportRequest; /// foo: list[int] = [1, 2, 3] /// ``` /// +/// ## Fix safety +/// This rule's fix is marked as unsafe, as it may lead to runtime errors when +/// alongside libraries that rely on runtime type annotations, like Pydantic, +/// on Python versions prior to Python 3.9. +/// /// ## Options /// - `target-version` /// - `pyupgrade.keep-runtime-typing` @@ -90,10 +96,18 @@ pub(crate) fn use_pep585_annotation( ModuleMember::BuiltIn(name) => { // Built-in type, like `list`. if checker.semantic().is_builtin(name) { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - (*name).to_string(), - expr.range(), - ))); + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement((*name).to_string(), expr.range()), + if checker.settings.preview.is_enabled() { + if checker.settings.target_version >= PythonVersion::Py310 { + Applicability::Safe + } else { + Applicability::Unsafe + } + } else { + Applicability::Safe + }, + )); } } ModuleMember::Member(module, member) => { @@ -105,7 +119,19 @@ pub(crate) fn use_pep585_annotation( checker.semantic(), )?; let reference_edit = Edit::range_replacement(binding, expr.range()); - Ok(Fix::unsafe_edits(import_edit, [reference_edit])) + Ok(Fix::applicable_edits( + import_edit, + [reference_edit], + if checker.settings.preview.is_enabled() { + if checker.settings.target_version >= PythonVersion::Py310 { + Applicability::Safe + } else { + Applicability::Unsafe + } + } else { + Applicability::Unsafe + }, + )) }); } } diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs index 839252ad6a1e9..70aa2cec0eeaf 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::{pep_604_optional, pep_604_union}; use ruff_python_ast::{self as ast, Expr}; @@ -7,6 +7,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::fix::edits::pad; +use crate::settings::types::PythonVersion; /// ## What it does /// Check for type annotations that can be rewritten based on [PEP 604] syntax. @@ -75,6 +76,16 @@ pub(crate) fn use_pep604_annotation( && !checker.semantic().in_complex_string_type_definition() && is_allowed_value(slice); + let applicability = if checker.settings.preview.is_enabled() { + if checker.settings.target_version >= PythonVersion::Py310 { + Applicability::Safe + } else { + Applicability::Unsafe + } + } else { + Applicability::Unsafe + }; + match operator { Pep604Operator::Optional => { let mut diagnostic = Diagnostic::new(NonPEP604Annotation, expr.range()); @@ -84,14 +95,17 @@ pub(crate) fn use_pep604_annotation( // Invalid type annotation. } _ => { - diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( - pad( - checker.generator().expr(&pep_604_optional(slice)), + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement( + pad( + checker.generator().expr(&pep_604_optional(slice)), + expr.range(), + checker.locator(), + ), expr.range(), - checker.locator(), ), - expr.range(), - ))); + applicability, + )); } } } @@ -105,25 +119,31 @@ pub(crate) fn use_pep604_annotation( // Invalid type annotation. } Expr::Tuple(ast::ExprTuple { elts, .. }) => { - diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( - pad( - checker.generator().expr(&pep_604_union(elts)), + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement( + pad( + checker.generator().expr(&pep_604_union(elts)), + expr.range(), + checker.locator(), + ), expr.range(), - checker.locator(), ), - expr.range(), - ))); + applicability, + )); } _ => { // Single argument. - diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( - pad( - checker.locator().slice(slice).to_string(), + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement( + pad( + checker.locator().slice(slice).to_string(), + expr.range(), + checker.locator(), + ), expr.range(), - checker.locator(), ), - expr.range(), - ))); + applicability, + )); } } }