Skip to content

Commit

Permalink
Use Python version to determine typing rewrite safety
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Nov 30, 2023
1 parent e8da95d commit 8806c68
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -86,14 +92,24 @@ pub(crate) fn use_pep585_annotation(
expr.range(),
);
if !checker.semantic().in_complex_string_type_definition() {
let applicability = if checker.settings.preview.is_enabled() {
if checker.settings.target_version >= PythonVersion::Py39 {
Applicability::Safe
} else {
Applicability::Unsafe
}
} else {
Applicability::Safe
};

match replacement {
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()),
applicability,
));
}
}
ModuleMember::Member(module, member) => {
Expand All @@ -105,7 +121,11 @@ 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],
applicability,
))
});
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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.
Expand Down Expand Up @@ -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());
Expand All @@ -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,
));
}
}
}
Expand All @@ -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,
));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ UP006_0.py:61:10: UP006 [*] Use `collections.deque` instead of `typing.Deque` fo
|
= help: Replace with `collections.deque`

Unsafe fix
Safe fix
20 20 |
21 21 |
22 22 | from typing import List as IList
Expand All @@ -265,7 +265,7 @@ UP006_0.py:65:10: UP006 [*] Use `collections.defaultdict` instead of `typing.Def
|
= help: Replace with `collections.defaultdict`

Unsafe fix
Safe fix
20 20 |
21 21 |
22 22 | from typing import List as IList
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ UP006_1.py:9:10: UP006 [*] Use `collections.defaultdict` instead of `typing.Defa
|
= help: Replace with `collections.defaultdict`

Unsafe fix
Safe fix
6 6 | from collections import defaultdict
7 7 |
8 8 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ UP006_3.py:7:11: UP006 [*] Use `collections.defaultdict` instead of `typing.Defa
|
= help: Replace with `collections.defaultdict`

Unsafe fix
Safe fix
4 4 | from collections import defaultdict
5 5 |
6 6 |
Expand Down

0 comments on commit 8806c68

Please sign in to comment.