Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ruff] Implement none-not-at-end-of-union (RUF036) #14314

Merged
merged 3 commits into from
Nov 14, 2024
Merged
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
47 changes: 47 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF036.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
from typing import Union as U


def func1(arg: None | int):
...


def func2() -> None | int:
...


def func3(arg: None | None | int):
...


def func4(arg: U[None, int]):
...


def func5() -> U[None, int]:
...


def func6(arg: U[None, None, int]):
...


# Ok
def good_func1(arg: int | None):
...


def good_func2() -> int | None:
...


def good_func3(arg: None):
...


def good_func4(arg: U[None]):
...


def good_func5(arg: U[int]):
...

25 changes: 25 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF036.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
from typing import Union as U


def func1(arg: None | int): ...

def func2() -> None | int: ...

def func3(arg: None | None | int): ...

def func4(arg: U[None, int]): ...

def func5() -> U[None, int]: ...

def func6(arg: U[None, None, int]): ...

# Ok
def good_func1(arg: int | None): ...

def good_func2() -> int | None: ...

def good_func3(arg: None): ...

def good_func4(arg: U[None]): ...

def good_func5(arg: U[int]): ...
7 changes: 7 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
Rule::DuplicateUnionMember,
Rule::RedundantLiteralUnion,
Rule::UnnecessaryTypeUnion,
Rule::NoneNotAtEndOfUnion,
]) {
// Avoid duplicate checks if the parent is a union, since these rules already
// traverse nested unions.
Expand All @@ -96,6 +97,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::UnnecessaryTypeUnion) {
flake8_pyi::rules::unnecessary_type_union(checker, expr);
}
if checker.enabled(Rule::NoneNotAtEndOfUnion) {
ruff::rules::none_not_at_end_of_union(checker, expr);
}
}
}

Expand Down Expand Up @@ -1276,6 +1280,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::RuntimeStringUnion) {
flake8_type_checking::rules::runtime_string_union(checker, expr);
}
if checker.enabled(Rule::NoneNotAtEndOfUnion) {
ruff::rules::none_not_at_end_of_union(checker, expr);
}
}
}
Expr::UnaryOp(
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 @@ -967,6 +967,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "033") => (RuleGroup::Preview, rules::ruff::rules::PostInitDefault),
(Ruff, "034") => (RuleGroup::Preview, rules::ruff::rules::UselessIfElse),
(Ruff, "035") => (RuleGroup::Preview, rules::ruff::rules::UnsafeMarkupUse),
(Ruff, "036") => (RuleGroup::Preview, rules::ruff::rules::NoneNotAtEndOfUnion),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),

Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ mod tests {
#[test_case(Rule::UselessIfElse, Path::new("RUF034.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))]
#[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))]
#[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.py"))]
#[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.pyi"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
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 @@ -17,6 +17,7 @@ pub(crate) use mutable_class_default::*;
pub(crate) use mutable_dataclass_default::*;
pub(crate) use mutable_fromkeys_value::*;
pub(crate) use never_union::*;
pub(crate) use none_not_at_end_of_union::*;
pub(crate) use parenthesize_logical_operators::*;
pub(crate) use post_init_default::*;
pub(crate) use quadratic_list_summation::*;
Expand Down Expand Up @@ -55,6 +56,7 @@ mod mutable_class_default;
mod mutable_dataclass_default;
mod mutable_fromkeys_value;
mod never_union;
mod none_not_at_end_of_union;
mod parenthesize_logical_operators;
mod post_init_default;
mod quadratic_list_summation;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Expr;
use ruff_python_semantic::analyze::typing::traverse_union;
use ruff_text_size::Ranged;
use smallvec::SmallVec;

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

/// ## What it does
/// Checks for type annotations where `None` is not at the end of an union.
///
/// ## Why is this bad?
/// Type annotation unions are associative, meaning that the order of the elements
/// does not matter. The `None` literal represents the absence of a value. For
/// readability, it's preferred to write the more informative type expressions first.
///
/// ## Example
/// ```python
/// def func(arg: None | int): ...
/// ```
///
/// Use instead:
/// ```python
/// def func(arg: int | None): ...
/// ```
///
/// ## References
/// - [Python documentation: Union type](https://docs.python.org/3/library/stdtypes.html#types-union)
/// - [Python documentation: `typing.Optional`](https://docs.python.org/3/library/typing.html#typing.Optional)
/// - [Python documentation: `None`](https://docs.python.org/3/library/constants.html#None)
#[violation]
pub struct NoneNotAtEndOfUnion;

impl Violation for NoneNotAtEndOfUnion {
#[derive_message_formats]
fn message(&self) -> String {
"`None` not at the end of the type annotation.".to_string()
}
}

/// RUF036
pub(crate) fn none_not_at_end_of_union<'a>(checker: &mut Checker, union: &'a Expr) {
let semantic = checker.semantic();
let mut none_exprs: SmallVec<[&Expr; 1]> = SmallVec::new();

let mut last_expr: Option<&Expr> = None;
let mut find_none = |expr: &'a Expr, _parent: &Expr| {
if matches!(expr, Expr::NoneLiteral(_)) {
none_exprs.push(expr);
}
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
last_expr = Some(expr);
};

// Walk through all type expressions in the union and keep track of `None` literals.
traverse_union(&mut find_none, semantic, union);

let Some(last_expr) = last_expr else {
return;
};

// The must be at least one `None` expression.
let Some(last_none) = none_exprs.last() else {
return;
};

// If any of the `None` literals is last we do not emit.
if *last_none == last_expr {
return;
}

for none_expr in none_exprs {
checker
.diagnostics
.push(Diagnostic::new(NoneNotAtEndOfUnion, none_expr.range()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF036.py:4:16: RUF036 `None` not at the end of the type annotation.
|
4 | def func1(arg: None | int):
| ^^^^ RUF036
5 | ...
|

RUF036.py:8:16: RUF036 `None` not at the end of the type annotation.
|
8 | def func2() -> None | int:
| ^^^^ RUF036
9 | ...
|

RUF036.py:12:16: RUF036 `None` not at the end of the type annotation.
|
12 | def func3(arg: None | None | int):
| ^^^^ RUF036
13 | ...
|

RUF036.py:12:23: RUF036 `None` not at the end of the type annotation.
|
12 | def func3(arg: None | None | int):
| ^^^^ RUF036
13 | ...
|

RUF036.py:16:18: RUF036 `None` not at the end of the type annotation.
|
16 | def func4(arg: U[None, int]):
| ^^^^ RUF036
17 | ...
|

RUF036.py:20:18: RUF036 `None` not at the end of the type annotation.
|
20 | def func5() -> U[None, int]:
| ^^^^ RUF036
21 | ...
|

RUF036.py:24:18: RUF036 `None` not at the end of the type annotation.
|
24 | def func6(arg: U[None, None, int]):
| ^^^^ RUF036
25 | ...
|

RUF036.py:24:24: RUF036 `None` not at the end of the type annotation.
|
24 | def func6(arg: U[None, None, int]):
| ^^^^ RUF036
25 | ...
|
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF036.pyi:4:16: RUF036 `None` not at the end of the type annotation.
|
4 | def func1(arg: None | int): ...
| ^^^^ RUF036
5 |
6 | def func2() -> None | int: ...
|

RUF036.pyi:6:16: RUF036 `None` not at the end of the type annotation.
|
4 | def func1(arg: None | int): ...
5 |
6 | def func2() -> None | int: ...
| ^^^^ RUF036
7 |
8 | def func3(arg: None | None | int): ...
|

RUF036.pyi:8:16: RUF036 `None` not at the end of the type annotation.
|
6 | def func2() -> None | int: ...
7 |
8 | def func3(arg: None | None | int): ...
| ^^^^ RUF036
9 |
10 | def func4(arg: U[None, int]): ...
|

RUF036.pyi:8:23: RUF036 `None` not at the end of the type annotation.
|
6 | def func2() -> None | int: ...
7 |
8 | def func3(arg: None | None | int): ...
| ^^^^ RUF036
9 |
10 | def func4(arg: U[None, int]): ...
|

RUF036.pyi:10:18: RUF036 `None` not at the end of the type annotation.
|
8 | def func3(arg: None | None | int): ...
9 |
10 | def func4(arg: U[None, int]): ...
| ^^^^ RUF036
11 |
12 | def func5() -> U[None, int]: ...
|

RUF036.pyi:12:18: RUF036 `None` not at the end of the type annotation.
|
10 | def func4(arg: U[None, int]): ...
11 |
12 | def func5() -> U[None, int]: ...
| ^^^^ RUF036
13 |
14 | def func6(arg: U[None, None, int]): ...
|

RUF036.pyi:14:18: RUF036 `None` not at the end of the type annotation.
|
12 | def func5() -> U[None, int]: ...
13 |
14 | def func6(arg: U[None, None, int]): ...
| ^^^^ RUF036
15 |
16 | # Ok
|

RUF036.pyi:14:24: RUF036 `None` not at the end of the type annotation.
|
12 | def func5() -> U[None, int]: ...
13 |
14 | def func6(arg: U[None, None, int]): ...
| ^^^^ RUF036
15 |
16 | # Ok
|
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.

Loading