Split UP007 Union and Optional to two individual rules

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP007.py

@@ -1,16 +1,7 @@
 import typing
-from typing import Optional
 from typing import Union
 
 
-def f(x: Optional[str]) -> None:
-    ...
-
-
-def f(x: typing.Optional[str]) -> None:
-    ...
-
-
 def f(x: Union[str, int, Union[float, bytes]]) -> None:
     ...
 
@@ -52,9 +43,6 @@ def f(x: Union[("str", "int"), float]) -> None:
 
 
 def f() -> None:
-    x: Optional[str]
-    x = Optional[str]
-
     x = Union[str, int]
     x = Union["str", "int"]
     x: Union[str, int]
@@ -85,31 +73,6 @@ def f(x: Union[str, lambda: int]) -> None:
     ...
 
 
-def f(x: Optional[int : float]) -> None:
-    ...
-
-
-def f(x: Optional[str, int : float]) -> None:
-    ...
-
-
-def f(x: Optional[int, float]) -> None:
-    ...
-
-
-# Regression test for: https://github.com/astral-sh/ruff/issues/7131
-class ServiceRefOrValue:
-    service_specification: Optional[
-        list[ServiceSpecificationRef]
-        | list[ServiceSpecification]
-    ] = None
-
-
-# Regression test for: https://github.com/astral-sh/ruff/issues/7201
-class ServiceRefOrValue:
-    service_specification: Optional[str]is not True = None
-
-
 # Regression test for: https://github.com/astral-sh/ruff/issues/7452
 class Collection(Protocol[*_B0]):
     def __iter__(self) -> Iterator[Union[*_B0]]:

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP007B.py

@@ -0,0 +1,45 @@
+import typing
+from typing import Optional
+
+
+def f(x: Optional[str]) -> None:
+    ...
+
+
+def f(x: typing.Optional[str]) -> None:
+    ...
+
+
+def f() -> None:
+    x: Optional[str]
+    x = Optional[str]
+
+
+def f(x: list[Optional[int]]) -> None:
+    ...
+
+
+def f(x: Optional[int : float]) -> None:
+    ...
+
+
+def f(x: Optional[str, int : float]) -> None:
+    ...
+
+
+def f(x: Optional[int, float]) -> None:
+    ...
+
+
+# Regression test for: https://github.com/astral-sh/ruff/issues/7131
+class ServiceRefOrValue:
+    service_specification: Optional[
+        list[ServiceSpecificationRef]
+        | list[ServiceSpecification]
+    ] = None
+
+
+# Regression test for: https://github.com/astral-sh/ruff/issues/7201
+class ServiceRefOrValue:
+    service_specification: Optional[str]is not True = None
+

crates/ruff_linter/resources/test/fixtures/ruff/redirects.py

@@ -1,9 +1,9 @@
 from typing import Optional
 
 
-def f(x: Optional[str]) -> None:  # noqa: U007
+def f(x: Optional[str]) -> None:  # noqa: U007B
     ...
 
 
-def f(x: Optional[str]) -> None:  # noqa: UP007
+def f(x: Optional[str]) -> None:  # noqa: UP007B
     ...

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -27,7 +27,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             // Ex) Optional[...], Union[...]
             if checker.any_enabled(&[
                 Rule::FutureRewritableTypeAnnotation,
-                Rule::NonPEP604Annotation,
+                Rule::NonPEP604AnnotationOptional,
+                Rule::NonPEP604AnnotationUnion,
             ]) {
                 if let Some(operator) = typing::to_pep604_operator(value, slice, &checker.semantic)
                 {
@@ -43,7 +44,22 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                             );
                         }
                     }
-                    if checker.enabled(Rule::NonPEP604Annotation) {
+                    if checker.enabled(Rule::NonPEP604AnnotationOptional)
+                        && matches!(operator, typing::Pep604Operator::Optional)
+                    {
+                        if checker.source_type.is_stub()
+                            || checker.settings.target_version >= PythonVersion::Py310
+                            || (checker.settings.target_version >= PythonVersion::Py37
+                                && checker.semantic.future_annotations_or_stub()
+                                && checker.semantic.in_annotation()
+                                && !checker.settings.pyupgrade.keep_runtime_typing)
+                        {
+                            pyupgrade::rules::use_pep604_annotation(checker, expr, slice, operator);
+                        }
+                    }
+                    if checker.enabled(Rule::NonPEP604AnnotationUnion)
+                        && matches!(operator, typing::Pep604Operator::Union)
+                    {
                         if checker.source_type.is_stub()
                             || checker.settings.target_version >= PythonVersion::Py310
                             || (checker.settings.target_version >= PythonVersion::Py37

crates/ruff_linter/src/codes.rs

@@ -521,7 +521,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pyupgrade, "004") => (RuleGroup::Stable, rules::pyupgrade::rules::UselessObjectInheritance),
         (Pyupgrade, "005") => (RuleGroup::Stable, rules::pyupgrade::rules::DeprecatedUnittestAlias),
         (Pyupgrade, "006") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP585Annotation),
-        (Pyupgrade, "007") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP604Annotation),
+        (Pyupgrade, "007B") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP604AnnotationOptional),
+        (Pyupgrade, "007") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP604AnnotationUnion),
         (Pyupgrade, "008") => (RuleGroup::Stable, rules::pyupgrade::rules::SuperCallWithParameters),
         (Pyupgrade, "009") => (RuleGroup::Stable, rules::pyupgrade::rules::UTF8EncodingDeclaration),
         (Pyupgrade, "010") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryFutureImport),

crates/ruff_linter/src/rules/pyupgrade/mod.rs

@@ -39,7 +39,8 @@ mod tests {
     #[test_case(Rule::NonPEP585Annotation, Path::new("UP006_1.py"))]
     #[test_case(Rule::NonPEP585Annotation, Path::new("UP006_2.py"))]
     #[test_case(Rule::NonPEP585Annotation, Path::new("UP006_3.py"))]
-    #[test_case(Rule::NonPEP604Annotation, Path::new("UP007.py"))]
+    #[test_case(Rule::NonPEP604AnnotationUnion, Path::new("UP007.py"))]
+    #[test_case(Rule::NonPEP604AnnotationOptional, Path::new("UP007B.py"))]
     #[test_case(Rule::NonPEP604Isinstance, Path::new("UP038.py"))]
     #[test_case(Rule::OSErrorAlias, Path::new("UP024_0.py"))]
     #[test_case(Rule::OSErrorAlias, Path::new("UP024_1.py"))]
@@ -187,7 +188,10 @@ mod tests {
             Path::new("pyupgrade/future_annotations.py"),
             &settings::LinterSettings {
                 target_version: PythonVersion::Py37,
-                ..settings::LinterSettings::for_rule(Rule::NonPEP604Annotation)
+                ..settings::LinterSettings::for_rules(vec![
+                    Rule::NonPEP604AnnotationUnion,
+                    Rule::NonPEP604AnnotationOptional,
+                ])
             },
         )?;
         assert_messages!(diagnostics);
@@ -200,7 +204,10 @@ mod tests {
             Path::new("pyupgrade/future_annotations.py"),
             &settings::LinterSettings {
                 target_version: PythonVersion::Py310,
-                ..settings::LinterSettings::for_rule(Rule::NonPEP604Annotation)
+                ..settings::LinterSettings::for_rules(vec![
+                    Rule::NonPEP604AnnotationUnion,
+                    Rule::NonPEP604AnnotationOptional,
+                ])
             },
         )?;
         assert_messages!(diagnostics);

crates/ruff_linter/src/rules/pyupgrade/rules/use_pep604_annotation.rs

@@ -10,12 +10,58 @@ 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.
+/// Check for `typing.Optional` annotations that can be rewritten based on [PEP 604] syntax.
 ///
 /// ## Why is this bad?
 /// [PEP 604] introduced a new syntax for union type annotations based on the
 /// `|` operator. This syntax is more concise and readable than the previous
-/// `typing.Union` and `typing.Optional` syntaxes.
+/// `typing.Optional` syntax.
+///
+/// This rule is enabled when targeting Python 3.10 or later (see:
+/// [`target-version`]). By default, it's _also_ enabled for earlier Python
+/// versions if `from __future__ import annotations` is present, as
+/// `__future__` annotations are not evaluated at runtime. If your code relies
+/// on runtime type annotations (either directly or via a library like
+/// Pydantic), you can disable this behavior for Python versions prior to 3.10
+/// by setting [`lint.pyupgrade.keep-runtime-typing`] to `true`.
+///
+/// ## Example
+/// ```python
+/// from typing import Optional
+///
+/// foo: Optional[int] = None
+/// ```
+///
+/// Use instead:
+/// ```python
+/// foo: int | None = None
+/// ```
+///
+/// ## Note
+/// Previously, this rule was covered under the `UP007` rule, but it has now been moved to this new, specific rule.
+///
+/// ## 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.10. It may also lead to runtime errors
+/// in unusual and likely incorrect type annotations where the type does not
+/// support the `|` operator.
+///
+/// ## Options
+/// - `target-version`
+/// - `lint.pyupgrade.keep-runtime-typing`
+///
+/// [PEP 604]: https://peps.python.org/pep-0604/
+#[violation]
+pub struct NonPEP604AnnotationOptional;
+
+/// ## What it does
+/// Check for `typing.Union` annotations that can be rewritten based on [PEP 604] syntax.
+///
+/// ## Why is this bad?
+/// [PEP 604] introduced a new syntax for union type annotations based on the
+/// `|` operator. This syntax is more concise and readable than the previous
+/// `typing.Union` syntax.
 ///
 /// This rule is enabled when targeting Python 3.10 or later (see:
 /// [`target-version`]). By default, it's _also_ enabled for earlier Python
@@ -37,6 +83,10 @@ use crate::settings::types::PythonVersion;
 /// foo: int | str = 1
 /// ```
 ///
+/// ## Note
+/// Previously, this rule also covered the usage of `Optional[T]` => `T | None`.
+/// This specific aspect of handling Optional types is now addressed by the `UP007B` rule instead.
+///
 /// ## 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,
@@ -50,9 +100,22 @@ use crate::settings::types::PythonVersion;
 ///
 /// [PEP 604]: https://peps.python.org/pep-0604/
 #[violation]
-pub struct NonPEP604Annotation;
+pub struct NonPEP604AnnotationUnion;
+
+impl Violation for NonPEP604AnnotationUnion {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Use `X | Y` for type annotations")
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        Some("Convert to `X | Y`".to_string())
+    }
+}
 
-impl Violation for NonPEP604Annotation {
+impl Violation for NonPEP604AnnotationOptional {
     const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
 
     #[derive_message_formats]
@@ -65,7 +128,7 @@ impl Violation for NonPEP604Annotation {
     }
 }
 
-/// UP007
+/// UP007 & UP007B
 pub(crate) fn use_pep604_annotation(
     checker: &mut Checker,
     expr: &Expr,
@@ -86,7 +149,7 @@ pub(crate) fn use_pep604_annotation(
 
     match operator {
         Pep604Operator::Optional => {
-            let mut diagnostic = Diagnostic::new(NonPEP604Annotation, expr.range());
+            let mut diagnostic = Diagnostic::new(NonPEP604AnnotationOptional, expr.range());
             if fixable {
                 match slice {
                     Expr::Tuple(_) => {
@@ -110,7 +173,7 @@ pub(crate) fn use_pep604_annotation(
             checker.diagnostics.push(diagnostic);
         }
         Pep604Operator::Union => {
-            let mut diagnostic = Diagnostic::new(NonPEP604Annotation, expr.range());
+            let mut diagnostic = Diagnostic::new(NonPEP604AnnotationUnion, expr.range());
             if fixable {
                 match slice {
                     Expr::Slice(_) => {