Use Python version to determine typing rewrite safety

astral-sh · Nov 30, 2023 · ad80cee · ad80cee
1 parent e8da95d
commit ad80cee
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 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
@@ -1,13 +1,14 @@
 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;
 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
@@ -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,
+                        ));
                     }
                 }
             }