mirror of https://github.com/astral-sh/ruff
Ignore type aliases for RUF013 (#5344)
## Summary
Ignore type aliases for RUF013 to avoid flagging false positives:
```python
from typing import Optional
MaybeInt = Optional[int]
def f(arg: MaybeInt = None):
pass
```
But, at the expense of having false negatives:
```python
Text = str | bytes
def f(arg: Text = None):
pass
```
## Test Plan
`cargo test`
fixes: #5295
This commit is contained in:
parent
d3d69a031e
commit
adf5cb5ff7
|
|
@ -221,3 +221,23 @@ def f(arg: Union["No" "ne", "int"] = None):
|
||||||
# Avoid flagging when there's a parse error in the forward reference
|
# Avoid flagging when there's a parse error in the forward reference
|
||||||
def f(arg: Union["<>", "int"] = None):
|
def f(arg: Union["<>", "int"] = None):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
# Type aliases
|
||||||
|
|
||||||
|
Text = str | bytes
|
||||||
|
|
||||||
|
|
||||||
|
def f(arg: Text = None):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
def f(arg: "Text" = None):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
from custom_typing import MaybeInt
|
||||||
|
|
||||||
|
|
||||||
|
def f(arg: MaybeInt = None):
|
||||||
|
pass
|
||||||
|
|
|
||||||
|
|
@ -6,10 +6,12 @@ use rustpython_parser::ast::{self, ArgWithDefault, Arguments, Constant, Expr, Op
|
||||||
|
|
||||||
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
|
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
|
||||||
use ruff_macros::{derive_message_formats, violation};
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
use ruff_python_ast::call_path::CallPath;
|
||||||
use ruff_python_ast::helpers::is_const_none;
|
use ruff_python_ast::helpers::is_const_none;
|
||||||
use ruff_python_ast::source_code::Locator;
|
use ruff_python_ast::source_code::Locator;
|
||||||
use ruff_python_ast::typing::parse_type_annotation;
|
use ruff_python_ast::typing::parse_type_annotation;
|
||||||
use ruff_python_semantic::SemanticModel;
|
use ruff_python_semantic::SemanticModel;
|
||||||
|
use ruff_python_stdlib::sys::is_known_standard_library;
|
||||||
|
|
||||||
use crate::checkers::ast::Checker;
|
use crate::checkers::ast::Checker;
|
||||||
use crate::importer::ImportRequest;
|
use crate::importer::ImportRequest;
|
||||||
|
|
@ -58,6 +60,18 @@ use crate::settings::types::PythonVersion;
|
||||||
/// pass
|
/// pass
|
||||||
/// ```
|
/// ```
|
||||||
///
|
///
|
||||||
|
/// ## Limitations
|
||||||
|
///
|
||||||
|
/// Type aliases are not supported and could result in false negatives.
|
||||||
|
/// For example, the following code will not be flagged:
|
||||||
|
/// ```python
|
||||||
|
/// Text = str | bytes
|
||||||
|
///
|
||||||
|
///
|
||||||
|
/// def foo(arg: Text = None):
|
||||||
|
/// pass
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
/// ## Options
|
/// ## Options
|
||||||
/// - `target-version`
|
/// - `target-version`
|
||||||
///
|
///
|
||||||
|
|
@ -141,6 +155,18 @@ impl<'a> Iterator for PEP604UnionIterator<'a> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if the given call path is a known type.
|
||||||
|
///
|
||||||
|
/// 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(call_path: &CallPath, target_version: PythonVersion) -> bool {
|
||||||
|
match call_path.as_slice() {
|
||||||
|
["" | "typing_extensions", ..] => true,
|
||||||
|
[module, ..] => is_known_standard_library(target_version.minor(), module),
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
enum TypingTarget<'a> {
|
enum TypingTarget<'a> {
|
||||||
None,
|
None,
|
||||||
|
|
@ -154,7 +180,12 @@ enum TypingTarget<'a> {
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a> TypingTarget<'a> {
|
impl<'a> TypingTarget<'a> {
|
||||||
fn try_from_expr(expr: &'a Expr, semantic: &SemanticModel, locator: &Locator) -> Option<Self> {
|
fn try_from_expr(
|
||||||
|
expr: &'a Expr,
|
||||||
|
semantic: &SemanticModel,
|
||||||
|
locator: &Locator,
|
||||||
|
target_version: PythonVersion,
|
||||||
|
) -> Option<Self> {
|
||||||
match expr {
|
match expr {
|
||||||
Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
|
Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
|
||||||
if semantic.match_typing_expr(value, "Optional") {
|
if semantic.match_typing_expr(value, "Optional") {
|
||||||
|
|
@ -170,7 +201,20 @@ impl<'a> TypingTarget<'a> {
|
||||||
} else if semantic.match_typing_expr(value, "Annotated") {
|
} else if semantic.match_typing_expr(value, "Annotated") {
|
||||||
elements.first().map(TypingTarget::Annotated)
|
elements.first().map(TypingTarget::Annotated)
|
||||||
} else {
|
} else {
|
||||||
None
|
semantic.resolve_call_path(value).map_or(
|
||||||
|
// If we can't resolve the call path, it must be defined
|
||||||
|
// in the same file, so we assume it's `Any` as it could
|
||||||
|
// be a type alias.
|
||||||
|
Some(TypingTarget::Any),
|
||||||
|
|call_path| {
|
||||||
|
if is_known_type(&call_path, target_version) {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
// If it's not a known type, we assume it's `Any`.
|
||||||
|
Some(TypingTarget::Any)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Expr::BinOp(..) => Some(TypingTarget::Union(
|
Expr::BinOp(..) => Some(TypingTarget::Union(
|
||||||
|
|
@ -189,54 +233,67 @@ impl<'a> TypingTarget<'a> {
|
||||||
.map_or(Some(TypingTarget::Any), |(expr, _)| {
|
.map_or(Some(TypingTarget::Any), |(expr, _)| {
|
||||||
Some(TypingTarget::ForwardReference(expr))
|
Some(TypingTarget::ForwardReference(expr))
|
||||||
}),
|
}),
|
||||||
_ => semantic.resolve_call_path(expr).and_then(|call_path| {
|
_ => semantic.resolve_call_path(expr).map_or(
|
||||||
if semantic.match_typing_call_path(&call_path, "Any") {
|
// If we can't resolve the call path, it must be defined in the
|
||||||
Some(TypingTarget::Any)
|
// same file, so we assume it's `Any` as it could be a type alias.
|
||||||
} else if matches!(call_path.as_slice(), ["" | "builtins", "object"]) {
|
Some(TypingTarget::Any),
|
||||||
Some(TypingTarget::Object)
|
|call_path| {
|
||||||
} else {
|
if semantic.match_typing_call_path(&call_path, "Any") {
|
||||||
None
|
Some(TypingTarget::Any)
|
||||||
}
|
} else if matches!(call_path.as_slice(), ["" | "builtins", "object"]) {
|
||||||
}),
|
Some(TypingTarget::Object)
|
||||||
|
} else if !is_known_type(&call_path, target_version) {
|
||||||
|
// If it's not a known type, we assume it's `Any`.
|
||||||
|
Some(TypingTarget::Any)
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
},
|
||||||
|
),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Check if the [`TypingTarget`] explicitly allows `None`.
|
/// Check if the [`TypingTarget`] explicitly allows `None`.
|
||||||
fn contains_none(&self, semantic: &SemanticModel, locator: &Locator) -> bool {
|
fn contains_none(
|
||||||
|
&self,
|
||||||
|
semantic: &SemanticModel,
|
||||||
|
locator: &Locator,
|
||||||
|
target_version: PythonVersion,
|
||||||
|
) -> bool {
|
||||||
match self {
|
match self {
|
||||||
TypingTarget::None
|
TypingTarget::None
|
||||||
| TypingTarget::Optional
|
| TypingTarget::Optional
|
||||||
| TypingTarget::Any
|
| TypingTarget::Any
|
||||||
| TypingTarget::Object => true,
|
| TypingTarget::Object => true,
|
||||||
TypingTarget::Literal(elements) => elements.iter().any(|element| {
|
TypingTarget::Literal(elements) => elements.iter().any(|element| {
|
||||||
let Some(new_target) = TypingTarget::try_from_expr(element, semantic, locator) else {
|
let Some(new_target) = TypingTarget::try_from_expr(element, semantic, locator, target_version) else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
// Literal can only contain `None`, a literal value, other `Literal`
|
// Literal can only contain `None`, a literal value, other `Literal`
|
||||||
// or an enum value.
|
// or an enum value.
|
||||||
match new_target {
|
match new_target {
|
||||||
TypingTarget::None => true,
|
TypingTarget::None => true,
|
||||||
TypingTarget::Literal(_) => new_target.contains_none(semantic, locator),
|
TypingTarget::Literal(_) => new_target.contains_none(semantic, locator, target_version),
|
||||||
_ => false,
|
_ => false,
|
||||||
}
|
}
|
||||||
}),
|
}),
|
||||||
TypingTarget::Union(elements) => elements.iter().any(|element| {
|
TypingTarget::Union(elements) => elements.iter().any(|element| {
|
||||||
let Some(new_target) = TypingTarget::try_from_expr(element, semantic, locator) else {
|
let Some(new_target) = TypingTarget::try_from_expr(element, semantic, locator, target_version) else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
new_target.contains_none(semantic, locator)
|
new_target.contains_none(semantic, locator, target_version)
|
||||||
}),
|
}),
|
||||||
TypingTarget::Annotated(element) => {
|
TypingTarget::Annotated(element) => {
|
||||||
let Some(new_target) = TypingTarget::try_from_expr(element, semantic, locator) else {
|
let Some(new_target) = TypingTarget::try_from_expr(element, semantic, locator, target_version) else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
new_target.contains_none(semantic, locator)
|
new_target.contains_none(semantic, locator, target_version)
|
||||||
}
|
}
|
||||||
TypingTarget::ForwardReference(expr) => {
|
TypingTarget::ForwardReference(expr) => {
|
||||||
let Some(new_target) = TypingTarget::try_from_expr(expr, semantic, locator) else {
|
let Some(new_target) = TypingTarget::try_from_expr(expr, semantic, locator, target_version) else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
new_target.contains_none(semantic, locator)
|
new_target.contains_none(semantic, locator, target_version)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -253,8 +310,9 @@ fn type_hint_explicitly_allows_none<'a>(
|
||||||
annotation: &'a Expr,
|
annotation: &'a Expr,
|
||||||
semantic: &SemanticModel,
|
semantic: &SemanticModel,
|
||||||
locator: &Locator,
|
locator: &Locator,
|
||||||
|
target_version: PythonVersion,
|
||||||
) -> Option<&'a Expr> {
|
) -> Option<&'a Expr> {
|
||||||
let Some(target) = TypingTarget::try_from_expr(annotation, semantic, locator) else {
|
let Some(target) = TypingTarget::try_from_expr(annotation, semantic, locator, target_version) else {
|
||||||
return Some(annotation);
|
return Some(annotation);
|
||||||
};
|
};
|
||||||
match target {
|
match target {
|
||||||
|
|
@ -264,9 +322,11 @@ fn type_hint_explicitly_allows_none<'a>(
|
||||||
// return the inner type if it doesn't allow `None`. If `Annotated`
|
// return the inner type if it doesn't allow `None`. If `Annotated`
|
||||||
// is found nested inside another type, then the outer type should
|
// is found nested inside another type, then the outer type should
|
||||||
// be returned.
|
// be returned.
|
||||||
TypingTarget::Annotated(expr) => type_hint_explicitly_allows_none(expr, semantic, locator),
|
TypingTarget::Annotated(expr) => {
|
||||||
|
type_hint_explicitly_allows_none(expr, semantic, locator, target_version)
|
||||||
|
}
|
||||||
_ => {
|
_ => {
|
||||||
if target.contains_none(semantic, locator) {
|
if target.contains_none(semantic, locator, target_version) {
|
||||||
None
|
None
|
||||||
} else {
|
} else {
|
||||||
Some(annotation)
|
Some(annotation)
|
||||||
|
|
@ -350,7 +410,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, arguments: &Arguments) {
|
||||||
{
|
{
|
||||||
// Quoted annotation.
|
// Quoted annotation.
|
||||||
if let Ok((annotation, kind)) = parse_type_annotation(string, *range, checker.locator) {
|
if let Ok((annotation, kind)) = parse_type_annotation(string, *range, checker.locator) {
|
||||||
let Some(expr) = type_hint_explicitly_allows_none(&annotation, checker.semantic(), checker.locator) else {
|
let Some(expr) = type_hint_explicitly_allows_none(&annotation, checker.semantic(), checker.locator, checker.settings.target_version) else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
let conversion_type = checker.settings.target_version.into();
|
let conversion_type = checker.settings.target_version.into();
|
||||||
|
|
@ -366,7 +426,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, arguments: &Arguments) {
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Unquoted annotation.
|
// Unquoted annotation.
|
||||||
let Some(expr) = type_hint_explicitly_allows_none(annotation, checker.semantic(), checker.locator) else {
|
let Some(expr) = type_hint_explicitly_allows_none(annotation, checker.semantic(), checker.locator, checker.settings.target_version) else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
let conversion_type = checker.settings.target_version.into();
|
let conversion_type = checker.settings.target_version.into();
|
||||||
|
|
@ -380,3 +440,40 @@ pub(crate) fn implicit_optional(checker: &mut Checker, arguments: &Arguments) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use ruff_python_ast::call_path::CallPath;
|
||||||
|
|
||||||
|
use crate::settings::types::PythonVersion;
|
||||||
|
|
||||||
|
use super::is_known_type;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_is_known_type() {
|
||||||
|
assert!(is_known_type(
|
||||||
|
&CallPath::from_slice(&["", "int"]),
|
||||||
|
PythonVersion::Py311
|
||||||
|
));
|
||||||
|
assert!(is_known_type(
|
||||||
|
&CallPath::from_slice(&["builtins", "int"]),
|
||||||
|
PythonVersion::Py311
|
||||||
|
));
|
||||||
|
assert!(is_known_type(
|
||||||
|
&CallPath::from_slice(&["typing", "Optional"]),
|
||||||
|
PythonVersion::Py311
|
||||||
|
));
|
||||||
|
assert!(is_known_type(
|
||||||
|
&CallPath::from_slice(&["typing_extensions", "Literal"]),
|
||||||
|
PythonVersion::Py311
|
||||||
|
));
|
||||||
|
assert!(is_known_type(
|
||||||
|
&CallPath::from_slice(&["zoneinfo", "ZoneInfo"]),
|
||||||
|
PythonVersion::Py311
|
||||||
|
));
|
||||||
|
assert!(!is_known_type(
|
||||||
|
&CallPath::from_slice(&["zoneinfo", "ZoneInfo"]),
|
||||||
|
PythonVersion::Py38
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue