Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF066.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
from typing import Annotated, Optional, TypeAlias, Union

# Errors
a: Union[Annotated[int, "An integer"], None] = 5
b: Annotated[Union[int, str], "An integer or string"] | None = "World"
c: Optional[Annotated[float, "A float"]] = 2.71
d: Union[Annotated[int, "An integer"], Annotated[str, "A string"]]

def f1(x: Union[Annotated[int, "An integer"], None]) -> None:
pass

def f2(y: Annotated[Union[int, str], "An integer or string"] | None) -> None:
pass

def f3(z: Optional[Annotated[float, "A float"]]) -> None:
pass

e: Annotated[Annotated[int, "An integer"], "Another annotation"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being an error implies that list[Annotated[...]] or similar would also produce an error. That part seems fine to me, but I wonder if it would be worth having a separate error message for these cases since the mention of union/optional doesn't make sense here? Would also like to have test cases showing that behavior one way or the other.


l: TypeAlias = Annotated[int, "An integer"] | None
m: TypeAlias = Union[Annotated[int, "An integer"], str]
n: TypeAlias = Optional[Annotated[float, "A float"]]
o: TypeAlias = "Annotated[str, 'A string'] | None"

p: None | Annotated[int, "An integer"]

q = Annotated[int | str, "An integer or string"] | None
r = Optional[Annotated[float | None, "A float or None"]]

@some_decorator()
def func(x: Annotated[str, "A string"] | None) -> None:
pass


# OK
g: Annotated[Union[int, str], "An integer or string"] = 42
h: Annotated[Union[float, None], "A float or None"] = 3.14

def f4(x: Annotated[Union[int, str], "An integer or string"]) -> None:
pass

def f5(y: Annotated[Union[float, None], "A float or None"]) -> None:
pass

def f6(z: Annotated[str | None, func()]):
pass

i: Union[int, str] = 7
j: Optional[float] = None
k: Annotated[int, "An integer"] = 10
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
if checker.is_rule_enabled(Rule::AccessAnnotationsFromClassDict) {
ruff::rules::access_annotations_from_class_dict_by_key(checker, subscript);
}
if checker.is_rule_enabled(Rule::UnionWithAnnotatedType) {
ruff::rules::union_with_annotated_type(checker, subscript);
}
pandas_vet::rules::subscript(checker, value, expr);
}
Expr::Tuple(ast::ExprTuple {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "063") => rules::ruff::rules::AccessAnnotationsFromClassDict,
(Ruff, "064") => rules::ruff::rules::NonOctalPermissions,
(Ruff, "065") => rules::ruff::rules::LoggingEagerConversion,
(Ruff, "066") => rules::ruff::rules::UnionWithAnnotatedType,

(Ruff, "100") => rules::ruff::rules::UnusedNOQA,
(Ruff, "101") => rules::ruff::rules::RedirectedNOQA,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ mod tests {
#[test_case(Rule::LegacyFormPytestRaises, Path::new("RUF061_deprecated_call.py"))]
#[test_case(Rule::NonOctalPermissions, Path::new("RUF064.py"))]
#[test_case(Rule::LoggingEagerConversion, Path::new("RUF065.py"))]
#[test_case(Rule::UnionWithAnnotatedType, Path::new("RUF066.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))]
#[test_case(Rule::InvalidRuleCode, Path::new("RUF102.py"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub(crate) use mutable_class_default::*;
pub(crate) use mutable_dataclass_default::*;
pub(crate) use mutable_fromkeys_value::*;
pub(crate) use needless_else::*;
pub(crate) use union_with_annotated_type::*;
pub(crate) use never_union::*;
pub(crate) use non_octal_permissions::*;
pub(crate) use none_not_at_end_of_union::*;
Expand Down Expand Up @@ -94,6 +95,7 @@ mod mutable_class_default;
mod mutable_dataclass_default;
mod mutable_fromkeys_value;
mod needless_else;
mod union_with_annotated_type;
mod never_union;
mod non_octal_permissions;
mod none_not_at_end_of_union;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::{Expr, ExprSubscript};
use ruff_text_size::Ranged;

use crate::Violation;
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `Annotated[]` types within a `Union` or `Optional` type.
///
/// ## Why is this bad?
/// Consumers of `Annotated` types often only check the top-level type for annotations,
/// and may miss `Annotated` types inside other types, such as `Optional` or `Union`
///
/// ```python
/// from typing import Annotated, get_type_hints
///
/// def f(a: Annotated[str, "test data"]): ...
/// def z(a: Annotated[str, "test data"] | None): ...
/// def b(a: Annotated[str | None, "test data"]): ...
///
/// get_type_hints(f, include_extras=True)
/// # {'a': typing.Annotated[str, 'test data']}
/// get_type_hints(z, include_extras=True)
/// # {'a': typing.Optional[typing.Annotated[str, 'test data']]}
/// get_type_hints(b, include_extras=True)
/// # {'a': typing.Annotated[str | None, 'test data']}
/// ```
///
/// ## Example
/// ```python
/// from typing import Annotated, Optional
/// from fastapi import FastAPI, Query
///
/// app = FastAPI()
///
/// @app.get('/route')
/// def route(param: Annotated[str, Query()] | None = None):
/// ...
/// ```
/// This fails to parse `param` as a query parameter. Use instead:
/// ```python
/// @app.get('/route')
/// def route(param: Annotated[str | None, Query()] = None):
/// ...
/// ```
///
#[derive(ViolationMetadata)]
#[violation_metadata(preview_since = "0.14.4")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a bump as well 😅

pub(crate) struct UnionWithAnnotatedType {
parent_type: ParentType,
}

impl Violation for UnionWithAnnotatedType {
#[derive_message_formats]
fn message(&self) -> String {
match self.parent_type {
ParentType::Subscript => {
"`Annotated[]` type must not be part of a Union or Optional type".to_string()
}
ParentType::BinOp => {
"`Annotated[]` type must not be part of a PEP604 type union (|)".to_string()
}
}
}
}

/// RUF066
pub(crate) fn union_with_annotated_type(checker: &Checker, subscript: &ExprSubscript) {
let semantic = checker.semantic();

if !semantic.match_typing_expr(&subscript.value, "Annotated") {
return;
}

let result = semantic
.current_expressions()
.skip(1)
.filter_map(|expr| match expr {
Expr::Subscript(_) => Some((expr, ParentType::Subscript)),
Expr::BinOp(_) => Some((expr, ParentType::BinOp)),
_ => None,
})
.last();

if let Some((parent, parent_type)) = result {
checker.report_diagnostic(UnionWithAnnotatedType { parent_type }, parent.range());
}
}

enum ParentType {
Subscript,
BinOp,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:4:4
|
3 | # Errors
4 | a: Union[Annotated[int, "An integer"], None] = 5
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5 | b: Annotated[Union[int, str], "An integer or string"] | None = "World"
6 | c: Optional[Annotated[float, "A float"]] = 2.71
|

RUF066 `Annotated[]` type must not be part of a PEP604 type union (|)
--> RUF066.py:5:4
|
3 | # Errors
4 | a: Union[Annotated[int, "An integer"], None] = 5
5 | b: Annotated[Union[int, str], "An integer or string"] | None = "World"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6 | c: Optional[Annotated[float, "A float"]] = 2.71
7 | d: Union[Annotated[int, "An integer"], Annotated[str, "A string"]]
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:6:4
|
4 | a: Union[Annotated[int, "An integer"], None] = 5
5 | b: Annotated[Union[int, str], "An integer or string"] | None = "World"
6 | c: Optional[Annotated[float, "A float"]] = 2.71
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7 | d: Union[Annotated[int, "An integer"], Annotated[str, "A string"]]
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:7:4
|
5 | b: Annotated[Union[int, str], "An integer or string"] | None = "World"
6 | c: Optional[Annotated[float, "A float"]] = 2.71
7 | d: Union[Annotated[int, "An integer"], Annotated[str, "A string"]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8 |
9 | def f1(x: Union[Annotated[int, "An integer"], None]) -> None:
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:7:4
|
5 | b: Annotated[Union[int, str], "An integer or string"] | None = "World"
6 | c: Optional[Annotated[float, "A float"]] = 2.71
7 | d: Union[Annotated[int, "An integer"], Annotated[str, "A string"]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8 |
9 | def f1(x: Union[Annotated[int, "An integer"], None]) -> None:
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:9:11
|
7 | d: Union[Annotated[int, "An integer"], Annotated[str, "A string"]]
8 |
9 | def f1(x: Union[Annotated[int, "An integer"], None]) -> None:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
10 | pass
|

RUF066 `Annotated[]` type must not be part of a PEP604 type union (|)
--> RUF066.py:12:11
|
10 | pass
11 |
12 | def f2(y: Annotated[Union[int, str], "An integer or string"] | None) -> None:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13 | pass
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:15:11
|
13 | pass
14 |
15 | def f3(z: Optional[Annotated[float, "A float"]]) -> None:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16 | pass
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:18:4
|
16 | pass
17 |
18 | e: Annotated[Annotated[int, "An integer"], "Another annotation"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19 |
20 | l: TypeAlias = Annotated[int, "An integer"] | None
|

RUF066 `Annotated[]` type must not be part of a PEP604 type union (|)
--> RUF066.py:20:16
|
18 | e: Annotated[Annotated[int, "An integer"], "Another annotation"]
19 |
20 | l: TypeAlias = Annotated[int, "An integer"] | None
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21 | m: TypeAlias = Union[Annotated[int, "An integer"], str]
22 | n: TypeAlias = Optional[Annotated[float, "A float"]]
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:21:16
|
20 | l: TypeAlias = Annotated[int, "An integer"] | None
21 | m: TypeAlias = Union[Annotated[int, "An integer"], str]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
22 | n: TypeAlias = Optional[Annotated[float, "A float"]]
23 | o: TypeAlias = "Annotated[str, 'A string'] | None"
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:22:16
|
20 | l: TypeAlias = Annotated[int, "An integer"] | None
21 | m: TypeAlias = Union[Annotated[int, "An integer"], str]
22 | n: TypeAlias = Optional[Annotated[float, "A float"]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23 | o: TypeAlias = "Annotated[str, 'A string'] | None"
|

RUF066 `Annotated[]` type must not be part of a PEP604 type union (|)
--> RUF066.py:23:17
|
21 | m: TypeAlias = Union[Annotated[int, "An integer"], str]
22 | n: TypeAlias = Optional[Annotated[float, "A float"]]
23 | o: TypeAlias = "Annotated[str, 'A string'] | None"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
24 |
25 | p: None | Annotated[int, "An integer"]
|

RUF066 `Annotated[]` type must not be part of a PEP604 type union (|)
--> RUF066.py:25:4
|
23 | o: TypeAlias = "Annotated[str, 'A string'] | None"
24 |
25 | p: None | Annotated[int, "An integer"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
26 |
27 | q = Annotated[int | str, "An integer or string"] | None
|

RUF066 `Annotated[]` type must not be part of a PEP604 type union (|)
--> RUF066.py:27:5
|
25 | p: None | Annotated[int, "An integer"]
26 |
27 | q = Annotated[int | str, "An integer or string"] | None
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
28 | r = Optional[Annotated[float | None, "A float or None"]]
|

RUF066 `Annotated[]` type must not be part of a Union or Optional type
--> RUF066.py:28:5
|
27 | q = Annotated[int | str, "An integer or string"] | None
28 | r = Optional[Annotated[float | None, "A float or None"]]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29 |
30 | @some_decorator()
|

RUF066 `Annotated[]` type must not be part of a PEP604 type union (|)
--> RUF066.py:31:13
|
30 | @some_decorator()
31 | def func(x: Annotated[str, "A string"] | None) -> None:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32 | pass
|
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.