Skip to content

Commit

Permalink
Pass ast::PythonVersion to type_hint_resolves_to_any
Browse files Browse the repository at this point in the history 
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](#16170 (comment)).

Co-authored-by: Alex Waygood <[email protected]>
  • Loading branch information
@ntBre @AlexWaygood
ntBre and AlexWaygood committed Feb 18, 2025
1 parent a9efdea commit ccfcb5f
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 57 deletions.
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() },
Expand All @@ -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(),
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/ruff/rules/implicit_optional.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -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;
};
Expand Down
103 changes: 50 additions & 53 deletions crates/ruff_linter/src/rules/ruff/typing.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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, .. }) => {
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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(_)
Expand All @@ -162,46 +166,43 @@ 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,
}
})
}),
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.
Expand All @@ -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))
}
}
}
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -281,49 +274,53 @@ 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`.
Some(TypingTarget::Optional(None)) => true,
// 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
));
}
}

0 comments on commit ccfcb5f

Please sign in to comment.