Skip to content

Commit 0dc00e6

Browse files
InSyncWithFooMichaReiser
InSyncWithFoo
and
MichaReiser
authored
[pyupgrade] Split UP007 to two individual rules for Union and Optional (UP007, UP045) (#15313)
Co-authored-by: Micha Reiser <[email protected]>
1 parent ce9c496 commit 0dc00e6

File tree

11 files changed

+496
-360
lines changed

11 files changed

+496
-360
lines changed

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

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,7 @@
11
import typing
2-
from typing import Optional
32
from typing import Union
43

54

6-
def f(x: Optional[str]) -> None:
7-
...
8-
9-
10-
def f(x: typing.Optional[str]) -> None:
11-
...
12-
13-
145
def f(x: Union[str, int, Union[float, bytes]]) -> None:
156
...
167

@@ -52,9 +43,6 @@ def f(x: Union[("str", "int"), float]) -> None:
5243

5344

5445
def f() -> None:
55-
x: Optional[str]
56-
x = Optional[str]
57-
5846
x = Union[str, int]
5947
x = Union["str", "int"]
6048
x: Union[str, int]
@@ -85,31 +73,6 @@ def f(x: Union[str, lambda: int]) -> None:
8573
...
8674

8775

88-
def f(x: Optional[int : float]) -> None:
89-
...
90-
91-
92-
def f(x: Optional[str, int : float]) -> None:
93-
...
94-
95-
96-
def f(x: Optional[int, float]) -> None:
97-
...
98-
99-
100-
# Regression test for: https://github.com/astral-sh/ruff/issues/7131
101-
class ServiceRefOrValue:
102-
service_specification: Optional[
103-
list[ServiceSpecificationRef]
104-
| list[ServiceSpecification]
105-
] = None
106-
107-
108-
# Regression test for: https://github.com/astral-sh/ruff/issues/7201
109-
class ServiceRefOrValue:
110-
service_specification: Optional[str]is not True = None
111-
112-
11376
# Regression test for: https://github.com/astral-sh/ruff/issues/7452
11477
class Collection(Protocol[*_B0]):
11578
def __iter__(self) -> Iterator[Union[*_B0]]:

‎crates/ruff_linter/resources/test/fixtures/pyupgrade/UP045.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import typing
2+
from typing import Optional
3+
4+
5+
def f(x: Optional[str]) -> None:
6+
...
7+
8+
9+
def f(x: typing.Optional[str]) -> None:
10+
...
11+
12+
13+
def f() -> None:
14+
x: Optional[str]
15+
x = Optional[str]
16+
17+
18+
def f(x: list[Optional[int]]) -> None:
19+
...
20+
21+
22+
def f(x: Optional[int : float]) -> None:
23+
...
24+
25+
26+
def f(x: Optional[str, int : float]) -> None:
27+
...
28+
29+
30+
def f(x: Optional[int, float]) -> None:
31+
...
32+
33+
34+
# Regression test for: https://github.com/astral-sh/ruff/issues/7131
35+
class ServiceRefOrValue:
36+
service_specification: Optional[
37+
list[ServiceSpecificationRef]
38+
| list[ServiceSpecification]
39+
] = None
40+
41+
42+
# Regression test for: https://github.com/astral-sh/ruff/issues/7201
43+
class ServiceRefOrValue:
44+
service_specification: Optional[str]is not True = None

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
2727
// Ex) Optional[...], Union[...]
2828
if checker.any_enabled(&[
2929
Rule::FutureRewritableTypeAnnotation,
30-
Rule::NonPEP604Annotation,
30+
Rule::NonPEP604AnnotationUnion,
31+
Rule::NonPEP604AnnotationOptional,
3132
]) {
3233
if let Some(operator) = typing::to_pep604_operator(value, slice, &checker.semantic)
3334
{
@@ -43,15 +44,18 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
4344
);
4445
}
4546
}
46-
if checker.enabled(Rule::NonPEP604Annotation) {
47+
if checker.any_enabled(&[
48+
Rule::NonPEP604AnnotationUnion,
49+
Rule::NonPEP604AnnotationOptional,
50+
]) {
4751
if checker.source_type.is_stub()
4852
|| checker.settings.target_version >= PythonVersion::Py310
4953
|| (checker.settings.target_version >= PythonVersion::Py37
5054
&& checker.semantic.future_annotations_or_stub()
5155
&& checker.semantic.in_annotation()
5256
&& !checker.settings.pyupgrade.keep_runtime_typing)
5357
{
54-
pyupgrade::rules::use_pep604_annotation(checker, expr, slice, operator);
58+
pyupgrade::rules::non_pep604_annotation(checker, expr, slice, operator);
5559
}
5660
}
5761
}

‎crates/ruff_linter/src/codes.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
501501
(Pyupgrade, "004") => (RuleGroup::Stable, rules::pyupgrade::rules::UselessObjectInheritance),
502502
(Pyupgrade, "005") => (RuleGroup::Stable, rules::pyupgrade::rules::DeprecatedUnittestAlias),
503503
(Pyupgrade, "006") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP585Annotation),
504-
(Pyupgrade, "007") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP604Annotation),
504+
(Pyupgrade, "007") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP604AnnotationUnion),
505505
(Pyupgrade, "008") => (RuleGroup::Stable, rules::pyupgrade::rules::SuperCallWithParameters),
506506
(Pyupgrade, "009") => (RuleGroup::Stable, rules::pyupgrade::rules::UTF8EncodingDeclaration),
507507
(Pyupgrade, "010") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryFutureImport),
@@ -538,6 +538,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
538538
(Pyupgrade, "042") => (RuleGroup::Preview, rules::pyupgrade::rules::ReplaceStrEnum),
539539
(Pyupgrade, "043") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryDefaultTypeArgs),
540540
(Pyupgrade, "044") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP646Unpack),
541+
(Pyupgrade, "045") => (RuleGroup::Preview, rules::pyupgrade::rules::NonPEP604AnnotationOptional),
541542

542543
// pydocstyle
543544
(Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule),

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ mod tests {
3939
#[test_case(Rule::NonPEP585Annotation, Path::new("UP006_1.py"))]
4040
#[test_case(Rule::NonPEP585Annotation, Path::new("UP006_2.py"))]
4141
#[test_case(Rule::NonPEP585Annotation, Path::new("UP006_3.py"))]
42-
#[test_case(Rule::NonPEP604Annotation, Path::new("UP007.py"))]
42+
#[test_case(Rule::NonPEP604AnnotationUnion, Path::new("UP007.py"))]
43+
#[test_case(Rule::NonPEP604AnnotationUnion, Path::new("UP045.py"))]
4344
#[test_case(Rule::NonPEP604Isinstance, Path::new("UP038.py"))]
4445
#[test_case(Rule::OSErrorAlias, Path::new("UP024_0.py"))]
4546
#[test_case(Rule::OSErrorAlias, Path::new("UP024_1.py"))]
@@ -111,6 +112,19 @@ mod tests {
111112
Ok(())
112113
}
113114

115+
#[test]
116+
fn up007_preview() -> Result<()> {
117+
let diagnostics = test_path(
118+
Path::new("pyupgrade/UP045.py"),
119+
&settings::LinterSettings {
120+
preview: PreviewMode::Enabled,
121+
..settings::LinterSettings::for_rule(Rule::NonPEP604AnnotationUnion)
122+
},
123+
)?;
124+
assert_messages!(diagnostics);
125+
Ok(())
126+
}
127+
114128
#[test]
115129
fn async_timeout_error_alias_not_applied_py310() -> Result<()> {
116130
let diagnostics = test_path(
@@ -201,7 +215,10 @@ mod tests {
201215
Path::new("pyupgrade/future_annotations.py"),
202216
&settings::LinterSettings {
203217
target_version: PythonVersion::Py37,
204-
..settings::LinterSettings::for_rule(Rule::NonPEP604Annotation)
218+
..settings::LinterSettings::for_rules([
219+
Rule::NonPEP604AnnotationUnion,
220+
Rule::NonPEP604AnnotationOptional,
221+
])
205222
},
206223
)?;
207224
assert_messages!(diagnostics);
@@ -214,7 +231,10 @@ mod tests {
214231
Path::new("pyupgrade/future_annotations.py"),
215232
&settings::LinterSettings {
216233
target_version: PythonVersion::Py310,
217-
..settings::LinterSettings::for_rule(Rule::NonPEP604Annotation)
234+
..settings::LinterSettings::for_rules([
235+
Rule::NonPEP604AnnotationUnion,
236+
Rule::NonPEP604AnnotationOptional,
237+
])
218238
},
219239
)?;
220240
assert_messages!(diagnostics);

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

Lines changed: 91 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
1-
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation};
1+
use ruff_diagnostics::{
2+
Applicability, Diagnostic, DiagnosticKind, Edit, Fix, FixAvailability, Violation,
3+
};
24
use ruff_macros::{derive_message_formats, ViolationMetadata};
35
use ruff_python_ast::helpers::{pep_604_optional, pep_604_union};
46
use ruff_python_ast::{self as ast, Expr};
57
use ruff_python_semantic::analyze::typing::Pep604Operator;
68
use ruff_text_size::Ranged;
79

810
use crate::checkers::ast::Checker;
11+
use crate::codes::Rule;
912
use crate::fix::edits::pad;
10-
use crate::settings::types::PythonVersion;
13+
use crate::settings::types::{PreviewMode, PythonVersion};
1114

1215
/// ## What it does
1316
/// Check for type annotations that can be rewritten based on [PEP 604] syntax.
@@ -37,6 +40,10 @@ use crate::settings::types::PythonVersion;
3740
/// foo: int | str = 1
3841
/// ```
3942
///
43+
/// ## Preview
44+
/// In preview mode, this rule only checks for usages of `typing.Union`,
45+
/// while `UP045` checks for `typing.Optional`.
46+
///
4047
/// ## Fix safety
4148
/// This rule's fix is marked as unsafe, as it may lead to runtime errors when
4249
/// alongside libraries that rely on runtime type annotations, like Pydantic,
@@ -50,9 +57,9 @@ use crate::settings::types::PythonVersion;
5057
///
5158
/// [PEP 604]: https://peps.python.org/pep-0604/
5259
#[derive(ViolationMetadata)]
53-
pub(crate) struct NonPEP604Annotation;
60+
pub(crate) struct NonPEP604AnnotationUnion;
5461

55-
impl Violation for NonPEP604Annotation {
62+
impl Violation for NonPEP604AnnotationUnion {
5663
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
5764

5865
#[derive_message_formats]
@@ -65,8 +72,64 @@ impl Violation for NonPEP604Annotation {
6572
}
6673
}
6774

68-
/// UP007
69-
pub(crate) fn use_pep604_annotation(
75+
/// ## What it does
76+
/// Check for `typing.Optional` annotations that can be rewritten based on [PEP 604] syntax.
77+
///
78+
/// ## Why is this bad?
79+
/// [PEP 604] introduced a new syntax for union type annotations based on the
80+
/// `|` operator. This syntax is more concise and readable than the previous
81+
/// `typing.Optional` syntax.
82+
///
83+
/// This rule is enabled when targeting Python 3.10 or later (see:
84+
/// [`target-version`]). By default, it's _also_ enabled for earlier Python
85+
/// versions if `from __future__ import annotations` is present, as
86+
/// `__future__` annotations are not evaluated at runtime. If your code relies
87+
/// on runtime type annotations (either directly or via a library like
88+
/// Pydantic), you can disable this behavior for Python versions prior to 3.10
89+
/// by setting [`lint.pyupgrade.keep-runtime-typing`] to `true`.
90+
///
91+
/// ## Example
92+
/// ```python
93+
/// from typing import Optional
94+
///
95+
/// foo: Optional[int] = None
96+
/// ```
97+
///
98+
/// Use instead:
99+
/// ```python
100+
/// foo: int | None = None
101+
/// ```
102+
///
103+
/// ## Fix safety
104+
/// This rule's fix is marked as unsafe, as it may lead to runtime errors when
105+
/// alongside libraries that rely on runtime type annotations, like Pydantic,
106+
/// on Python versions prior to Python 3.10. It may also lead to runtime errors
107+
/// in unusual and likely incorrect type annotations where the type does not
108+
/// support the `|` operator.
109+
///
110+
/// ## Options
111+
/// - `target-version`
112+
/// - `lint.pyupgrade.keep-runtime-typing`
113+
///
114+
/// [PEP 604]: https://peps.python.org/pep-0604/
115+
#[derive(ViolationMetadata)]
116+
pub(crate) struct NonPEP604AnnotationOptional;
117+
118+
impl Violation for NonPEP604AnnotationOptional {
119+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
120+
121+
#[derive_message_formats]
122+
fn message(&self) -> String {
123+
"Use `X | None` for type annotations".to_string()
124+
}
125+
126+
fn fix_title(&self) -> Option<String> {
127+
Some("Convert to `X | None`".to_string())
128+
}
129+
}
130+
131+
/// UP007, UP045
132+
pub(crate) fn non_pep604_annotation(
70133
checker: &mut Checker,
71134
expr: &Expr,
72135
slice: &Expr,
@@ -86,7 +149,23 @@ pub(crate) fn use_pep604_annotation(
86149

87150
match operator {
88151
Pep604Operator::Optional => {
89-
let mut diagnostic = Diagnostic::new(NonPEP604Annotation, expr.range());
152+
let (rule, diagnostic_kind) = match checker.settings.preview {
153+
PreviewMode::Disabled => (
154+
Rule::NonPEP604AnnotationUnion,
155+
DiagnosticKind::from(NonPEP604AnnotationUnion),
156+
),
157+
PreviewMode::Enabled => (
158+
Rule::NonPEP604AnnotationOptional,
159+
DiagnosticKind::from(NonPEP604AnnotationOptional),
160+
),
161+
};
162+
163+
if !checker.enabled(rule) {
164+
return;
165+
}
166+
167+
let mut diagnostic = Diagnostic::new(diagnostic_kind, expr.range());
168+
90169
if fixable {
91170
match slice {
92171
Expr::Tuple(_) => {
@@ -110,7 +189,11 @@ pub(crate) fn use_pep604_annotation(
110189
checker.diagnostics.push(diagnostic);
111190
}
112191
Pep604Operator::Union => {
113-
let mut diagnostic = Diagnostic::new(NonPEP604Annotation, expr.range());
192+
if !checker.enabled(Rule::NonPEP604AnnotationUnion) {
193+
return;
194+
}
195+
196+
let mut diagnostic = Diagnostic::new(NonPEP604AnnotationUnion, expr.range());
114197
if fixable {
115198
match slice {
116199
Expr::Slice(_) => {

0 commit comments

Comments
 (0)