Pass ast::PythonVersion to type_hint_resolves_to_any (#16236)

This is a follow-up to #16170 to use `ast::PythonVersion` in the
`type_hint_resolves_to_any` call chain, as suggested (and implemented!)
by Alex
[here](https://github.com/astral-sh/ruff/pull/16170#discussion_r1960015181).

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This commit is contained in:
Brent Westbrook 2025-02-18 13:22:50 -05:00 committed by GitHub
parent a9efdea113
commit 1907e60fab
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 54 additions and 57 deletions

View file

@ -523,7 +523,7 @@ fn check_dynamically_typed<F>(
if type_hint_resolves_to_any(
parsed_annotation.expression(),
checker,
checker.settings.target_version.minor,
checker.settings.target_version,
) {
diagnostics.push(Diagnostic::new(
AnyType { name: func() },
@ -532,7 +532,7 @@ fn check_dynamically_typed<F>(
}
}
} else {
if type_hint_resolves_to_any(annotation, checker, checker.settings.target_version.minor) {
if type_hint_resolves_to_any(annotation, checker, checker.settings.target_version) {
diagnostics.push(Diagnostic::new(
AnyType { name: func() },
annotation.range(),

View file

@ -177,7 +177,7 @@ pub(crate) fn implicit_optional(checker: &Checker, parameters: &Parameters) {
let Some(expr) = type_hint_explicitly_allows_none(
parsed_annotation.expression(),
checker,
checker.settings.target_version.minor,
checker.settings.target_version,
) else {
continue;
};
@ -195,7 +195,7 @@ pub(crate) fn implicit_optional(checker: &Checker, parameters: &Parameters) {
let Some(expr) = type_hint_explicitly_allows_none(
annotation,
checker,
checker.settings.target_version.minor,
checker.settings.target_version,
) else {
continue;
};

View file

@ -10,10 +10,10 @@ use crate::checkers::ast::Checker;
///
/// A known type is either a builtin type, any object from the standard library,
/// or a type from the `typing_extensions` module.
fn is_known_type(qualified_name: &QualifiedName, minor_version: u8) -> bool {
fn is_known_type(qualified_name: &QualifiedName, version: ast::PythonVersion) -> bool {
match qualified_name.segments() {
["" | "typing_extensions", ..] => true,
[module, ..] => is_known_standard_library(minor_version, module),
[module, ..] => is_known_standard_library(version.minor, module),
_ => false,
}
}
@ -70,7 +70,11 @@ enum TypingTarget<'a> {
}
impl<'a> TypingTarget<'a> {
fn try_from_expr(expr: &'a Expr, checker: &'a Checker, minor_version: u8) -> Option<Self> {
fn try_from_expr(
expr: &'a Expr,
checker: &'a Checker,
version: ast::PythonVersion,
) -> Option<Self> {
let semantic = checker.semantic();
match expr {
Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
@ -91,7 +95,7 @@ impl<'a> TypingTarget<'a> {
resolve_slice_value(slice.as_ref()).next(),
))
} else {
if is_known_type(&qualified_name, minor_version) {
if is_known_type(&qualified_name, version) {
Some(TypingTarget::Known)
} else {
Some(TypingTarget::Unknown)
@ -137,7 +141,7 @@ impl<'a> TypingTarget<'a> {
)
{
Some(TypingTarget::Hashable)
} else if !is_known_type(&qualified_name, minor_version) {
} else if !is_known_type(&qualified_name, version) {
// If it's not a known type, we assume it's `Any`.
Some(TypingTarget::Unknown)
} else {
@ -149,7 +153,7 @@ impl<'a> TypingTarget<'a> {
}
/// Check if the [`TypingTarget`] explicitly allows `None`.
fn contains_none(&self, checker: &Checker, minor_version: u8) -> bool {
fn contains_none(&self, checker: &Checker, version: ast::PythonVersion) -> bool {
match self {
TypingTarget::None
| TypingTarget::Optional(_)
@ -162,10 +166,10 @@ impl<'a> TypingTarget<'a> {
resolve_slice_value(slice).any(|element| {
// Literal can only contain `None`, a literal value, other `Literal`
// or an enum value.
match TypingTarget::try_from_expr(element, checker, minor_version) {
match TypingTarget::try_from_expr(element, checker, version) {
None | Some(TypingTarget::None) => true,
Some(new_target @ TypingTarget::Literal(_)) => {
new_target.contains_none(checker, minor_version)
new_target.contains_none(checker, version)
}
_ => false,
}
@ -173,35 +177,32 @@ impl<'a> TypingTarget<'a> {
}),
TypingTarget::Union(slice) => slice.is_some_and(|slice| {
resolve_slice_value(slice).any(|element| {
TypingTarget::try_from_expr(element, checker, minor_version)
TypingTarget::try_from_expr(element, checker, version)
.map_or(true, |new_target| {
new_target.contains_none(checker, minor_version)
new_target.contains_none(checker, version)
})
})
}),
TypingTarget::PEP604Union(left, right) => [left, right].iter().any(|element| {
TypingTarget::try_from_expr(element, checker, minor_version)
.map_or(true, |new_target| {
new_target.contains_none(checker, minor_version)
})
TypingTarget::try_from_expr(element, checker, version).map_or(true, |new_target| {
new_target.contains_none(checker, version)
})
}),
TypingTarget::Annotated(expr) => expr.is_some_and(|expr| {
TypingTarget::try_from_expr(expr, checker, minor_version)
.map_or(true, |new_target| {
new_target.contains_none(checker, minor_version)
})
TypingTarget::try_from_expr(expr, checker, version).map_or(true, |new_target| {
new_target.contains_none(checker, version)
})
}),
TypingTarget::ForwardReference(expr) => {
TypingTarget::try_from_expr(expr, checker, minor_version)
.map_or(true, |new_target| {
new_target.contains_none(checker, minor_version)
})
TypingTarget::try_from_expr(expr, checker, version).map_or(true, |new_target| {
new_target.contains_none(checker, version)
})
}
}
}
/// Check if the [`TypingTarget`] explicitly allows `Any`.
fn contains_any(&self, checker: &Checker, minor_version: u8) -> bool {
fn contains_any(&self, checker: &Checker, version: ast::PythonVersion) -> bool {
match self {
TypingTarget::Any => true,
// `Literal` cannot contain `Any` as it's a dynamic value.
@ -213,31 +214,23 @@ impl<'a> TypingTarget<'a> {
| TypingTarget::Unknown => false,
TypingTarget::Union(slice) => slice.is_some_and(|slice| {
resolve_slice_value(slice).any(|element| {
TypingTarget::try_from_expr(element, checker, minor_version)
.map_or(true, |new_target| {
new_target.contains_any(checker, minor_version)
})
TypingTarget::try_from_expr(element, checker, version)
.map_or(true, |new_target| new_target.contains_any(checker, version))
})
}),
TypingTarget::PEP604Union(left, right) => [left, right].iter().any(|element| {
TypingTarget::try_from_expr(element, checker, minor_version)
.map_or(true, |new_target| {
new_target.contains_any(checker, minor_version)
})
TypingTarget::try_from_expr(element, checker, version)
.map_or(true, |new_target| new_target.contains_any(checker, version))
}),
TypingTarget::Annotated(expr) | TypingTarget::Optional(expr) => {
expr.is_some_and(|expr| {
TypingTarget::try_from_expr(expr, checker, minor_version)
.map_or(true, |new_target| {
new_target.contains_any(checker, minor_version)
})
TypingTarget::try_from_expr(expr, checker, version)
.map_or(true, |new_target| new_target.contains_any(checker, version))
})
}
TypingTarget::ForwardReference(expr) => {
TypingTarget::try_from_expr(expr, checker, minor_version)
.map_or(true, |new_target| {
new_target.contains_any(checker, minor_version)
})
TypingTarget::try_from_expr(expr, checker, version)
.map_or(true, |new_target| new_target.contains_any(checker, version))
}
}
}
@ -253,9 +246,9 @@ impl<'a> TypingTarget<'a> {
pub(crate) fn type_hint_explicitly_allows_none<'a>(
annotation: &'a Expr,
checker: &'a Checker,
minor_version: u8,
version: ast::PythonVersion,
) -> Option<&'a Expr> {
match TypingTarget::try_from_expr(annotation, checker, minor_version) {
match TypingTarget::try_from_expr(annotation, checker, version) {
None |
// Short circuit on top level `None`, `Any` or `Optional`
Some(TypingTarget::None | TypingTarget::Optional(_) | TypingTarget::Any) => None,
@ -264,10 +257,10 @@ pub(crate) fn type_hint_explicitly_allows_none<'a>(
// is found nested inside another type, then the outer type should
// be returned.
Some(TypingTarget::Annotated(expr)) => {
expr.and_then(|expr| type_hint_explicitly_allows_none(expr, checker, minor_version))
expr.and_then(|expr| type_hint_explicitly_allows_none(expr, checker, version))
}
Some(target) => {
if target.contains_none(checker, minor_version) {
if target.contains_none(checker, version) {
return None;
}
Some(annotation)
@ -281,9 +274,9 @@ pub(crate) fn type_hint_explicitly_allows_none<'a>(
pub(crate) fn type_hint_resolves_to_any(
annotation: &Expr,
checker: &Checker,
minor_version: u8,
version: ast::PythonVersion,
) -> bool {
match TypingTarget::try_from_expr(annotation, checker, minor_version) {
match TypingTarget::try_from_expr(annotation, checker, version) {
// Short circuit on top level `Any`
None | Some(TypingTarget::Any) => true,
// `Optional` is `Optional[Any]` which is `Any | None`.
@ -291,39 +284,43 @@ pub(crate) fn type_hint_resolves_to_any(
// Top-level `Annotated` node should check if the inner type resolves
// to `Any`.
Some(TypingTarget::Annotated(expr)) => {
expr.is_some_and(|expr| type_hint_resolves_to_any(expr, checker, minor_version))
expr.is_some_and(|expr| type_hint_resolves_to_any(expr, checker, version))
}
Some(target) => target.contains_any(checker, minor_version),
Some(target) => target.contains_any(checker, version),
}
}
#[cfg(test)]
mod tests {
use super::is_known_type;
use ruff_python_ast as ast;
use ruff_python_ast::name::QualifiedName;
#[test]
fn test_is_known_type() {
assert!(is_known_type(&QualifiedName::builtin("int"), 11));
assert!(is_known_type(
&QualifiedName::builtin("int"),
ast::PythonVersion::PY311
));
assert!(is_known_type(
&QualifiedName::from_iter(["builtins", "int"]),
11
ast::PythonVersion::PY311
));
assert!(is_known_type(
&QualifiedName::from_iter(["typing", "Optional"]),
11
ast::PythonVersion::PY311
));
assert!(is_known_type(
&QualifiedName::from_iter(["typing_extensions", "Literal"]),
11
ast::PythonVersion::PY311
));
assert!(is_known_type(
&QualifiedName::from_iter(["zoneinfo", "ZoneInfo"]),
11
ast::PythonVersion::PY311
));
assert!(!is_known_type(
&QualifiedName::from_iter(["zoneinfo", "ZoneInfo"]),
8
ast::PythonVersion::PY38
));
}
}