diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF066.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF066.py new file mode 100644 index 0000000000000..b3e403a28eed2 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF066.py @@ -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"] + +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 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 53081e3681840..3a33f3f0f0f9f 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -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 { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 172841dc7c1f7..f71516dfbcb71 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -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, diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 7cdc55784176e..61baafee44e45 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -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"))] diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 9d5ade77f3f46..ef84c3fa2f53b 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -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::*; @@ -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; diff --git a/crates/ruff_linter/src/rules/ruff/rules/union_with_annotated_type.rs b/crates/ruff_linter/src/rules/ruff/rules/union_with_annotated_type.rs new file mode 100644 index 0000000000000..3e11702e339f9 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/union_with_annotated_type.rs @@ -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")] +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, +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF066_RUF066.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF066_RUF066.py.snap new file mode 100644 index 0000000000000..da0fbada80207 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF066_RUF066.py.snap @@ -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 + | diff --git a/ruff.schema.json b/ruff.schema.json index a16e91fbd7ed8..01ade2dcb95a8 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4037,6 +4037,7 @@ "RUF063", "RUF064", "RUF065", + "RUF066", "RUF1", "RUF10", "RUF100",