Skip to content

Commit

Permalink
[flake8-pyi] Implement PYI041 (astral-sh#5722)
Browse files Browse the repository at this point in the history
## Summary

Implements PYI041 from flake8-pyi. See [original
code](https://github.com/PyCQA/flake8-pyi/blob/2a86db8271dc500247a8a69419536240334731cf/pyi.py#L1283).

This check only applies to function parameters in order to avoid issues
with mypy. See PyCQA/flake8-pyi#299.

ref: astral-sh#848

## Test Plan

Snapshots, manual runs of flake8.
  • Loading branch information
density authored and evanrittenhouse committed Jul 19, 2023
1 parent bbe3c51 commit d56880c
Show file tree
Hide file tree
Showing 10 changed files with 276 additions and 0 deletions.
47 changes: 47 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI041.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
from typing import (
Union,
)

from typing_extensions import (
TypeAlias,
)

TA0: TypeAlias = int
TA1: TypeAlias = int | float | bool
TA2: TypeAlias = Union[int, float, bool]


def good1(arg: int) -> int | bool:
...


def good2(arg: int, arg2: int | bool) -> None:
...


def f0(arg1: float | int) -> None:
...


def f1(arg1: float, *, arg2: float | list[str] | type[bool] | complex) -> None:
...


def f2(arg1: int, /, arg2: int | int | float) -> None:
...


def f3(arg1: int, *args: Union[int | int | float]) -> None:
...


async def f4(**kwargs: int | int | float) -> None:
...


class Foo:
def good(self, arg: int) -> None:
...

def bad(self, arg: int | float | complex) -> None:
...
39 changes: 39 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI041.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
from typing import (
Union,
)

from typing_extensions import (
TypeAlias,
)

# Type aliases not flagged
TA0: TypeAlias = int
TA1: TypeAlias = int | float | bool
TA2: TypeAlias = Union[int, float, bool]


def good1(arg: int) -> int | bool: ...


def good2(arg: int, arg2: int | bool) -> None: ...


def f0(arg1: float | int) -> None: ... # PYI041


def f1(arg1: float, *, arg2: float | list[str] | type[bool] | complex) -> None: ... # PYI041


def f2(arg1: int, /, arg2: int | int | float) -> None: ... # PYI041


def f3(arg1: int, *args: Union[int | int | float]) -> None: ... # PYI041


async def f4(**kwargs: int | int | float) -> None: ... # PYI041


class Foo:
def good(self, arg: int) -> None: ...

def bad(self, arg: int | float | complex) -> None: ... # PYI041
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,9 @@ where
args,
);
}
if self.enabled(Rule::RedundantNumericUnion) {
flake8_pyi::rules::redundant_numeric_union(self, args);
}
}
if self.enabled(Rule::DunderFunctionName) {
if let Some(diagnostic) = pep8_naming::rules::dunder_function_name(
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Pyi, "034") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::NonSelfReturnType),
(Flake8Pyi, "035") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnassignedSpecialVariableInStub),
(Flake8Pyi, "036") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::BadExitAnnotation),
(Flake8Pyi, "041") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::RedundantNumericUnion),
(Flake8Pyi, "042") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::SnakeCaseTypeAlias),
(Flake8Pyi, "043") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::TSuffixedTypeAlias),
(Flake8Pyi, "044") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::FutureAnnotationsInStub),
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ mod tests {
#[test_case(Rule::PassStatementStubBody, Path::new("PYI009.pyi"))]
#[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.py"))]
#[test_case(Rule::QuotedAnnotationInStub, Path::new("PYI020.pyi"))]
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041.py"))]
#[test_case(Rule::RedundantNumericUnion, Path::new("PYI041.pyi"))]
#[test_case(Rule::SnakeCaseTypeAlias, Path::new("PYI042.py"))]
#[test_case(Rule::SnakeCaseTypeAlias, Path::new("PYI042.pyi"))]
#[test_case(Rule::UnassignedSpecialVariableInStub, Path::new("PYI035.py"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub(crate) use pass_in_class_body::*;
pub(crate) use pass_statement_stub_body::*;
pub(crate) use prefix_type_params::*;
pub(crate) use quoted_annotation_in_stub::*;
pub(crate) use redundant_numeric_union::*;
pub(crate) use simple_defaults::*;
pub(crate) use str_or_repr_defined_in_stub::*;
pub(crate) use string_or_bytes_too_long::*;
Expand Down Expand Up @@ -45,6 +46,7 @@ mod pass_in_class_body;
mod pass_statement_stub_body;
mod prefix_type_params;
mod quoted_annotation_in_stub;
mod redundant_numeric_union;
mod simple_defaults;
mod str_or_repr_defined_in_stub;
mod string_or_bytes_too_long;
Expand Down
133 changes: 133 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/redundant_numeric_union.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
use rustpython_parser::ast::{Arguments, Expr, Ranged};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;
use crate::rules::flake8_pyi::helpers::traverse_union;

/// ## What it does
/// Checks for union annotations that contain redundant numeric types (e.g.,
/// `int | float`).
///
/// ## Why is this bad?
/// In Python, `int` is a subtype of `float`, and `float` is a subtype of
/// `complex`. As such, a union that includes both `int` and `float` is
/// redundant, as it is equivalent to a union that only includes `float`.
///
/// For more, see [PEP 3141], which defines Python's "numeric tower".
///
/// Unions with redundant elements are less readable than unions without them.
///
/// ## Example
/// ```python
/// def foo(x: float | int) -> None:
/// ...
/// ```
///
/// Use instead:
/// ```python
/// def foo(x: float) -> None:
/// ...
/// ```
///
/// ## References
/// - [Python documentation: The numeric tower](https://docs.python.org/3/library/numbers.html#the-numeric-tower)
///
/// [PEP 3141]: https://peps.python.org/pep-3141/
#[violation]
pub struct RedundantNumericUnion {
redundancy: Redundancy,
}

impl Violation for RedundantNumericUnion {
#[derive_message_formats]
fn message(&self) -> String {
let (subtype, supertype) = match self.redundancy {
Redundancy::FloatComplex => ("float", "complex"),
Redundancy::IntComplex => ("int", "complex"),
Redundancy::IntFloat => ("int", "float"),
};
format!("Use `{supertype}` instead of `{subtype} | {supertype}`")
}
}

/// PYI041
pub(crate) fn redundant_numeric_union(checker: &mut Checker, args: &Arguments) {
for annotation in args
.args
.iter()
.chain(args.posonlyargs.iter())
.chain(args.kwonlyargs.iter())
.filter_map(|arg| arg.def.annotation.as_ref())
{
check_annotation(checker, annotation);
}

// If annotations on `args` or `kwargs` are flagged by this rule, the annotations themselves
// are not accurate, but check them anyway. It's possible that flagging them will help the user
// realize they're incorrect.
for annotation in args
.vararg
.iter()
.chain(args.kwarg.iter())
.filter_map(|arg| arg.annotation.as_ref())
{
check_annotation(checker, annotation);
}
}

#[derive(Debug, Clone, Copy, Eq, PartialEq)]
enum Redundancy {
FloatComplex,
IntComplex,
IntFloat,
}

fn check_annotation(checker: &mut Checker, annotation: &Expr) {
let mut has_float = false;
let mut has_complex = false;
let mut has_int = false;

let mut func = |expr: &Expr, _parent: Option<&Expr>| {
let Some(call_path) = checker.semantic().resolve_call_path(expr) else {
return;
};

match call_path.as_slice() {
["" | "builtins", "int"] => has_int = true,
["" | "builtins", "float"] => has_float = true,
["" | "builtins", "complex"] => has_complex = true,
_ => (),
}
};

traverse_union(&mut func, checker.semantic(), annotation, None);

if has_complex {
if has_float {
checker.diagnostics.push(Diagnostic::new(
RedundantNumericUnion {
redundancy: Redundancy::FloatComplex,
},
annotation.range(),
));
}

if has_int {
checker.diagnostics.push(Diagnostic::new(
RedundantNumericUnion {
redundancy: Redundancy::IntComplex,
},
annotation.range(),
));
}
} else if has_float && has_int {
checker.diagnostics.push(Diagnostic::new(
RedundantNumericUnion {
redundancy: Redundancy::IntFloat,
},
annotation.range(),
));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI041.pyi:21:14: PYI041 Use `float` instead of `int | float`
|
21 | def f0(arg1: float | int) -> None: ... # PYI041
| ^^^^^^^^^^^ PYI041
|

PYI041.pyi:24:30: PYI041 Use `complex` instead of `float | complex`
|
24 | def f1(arg1: float, *, arg2: float | list[str] | type[bool] | complex) -> None: ... # PYI041
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI041
|

PYI041.pyi:27:28: PYI041 Use `float` instead of `int | float`
|
27 | def f2(arg1: int, /, arg2: int | int | float) -> None: ... # PYI041
| ^^^^^^^^^^^^^^^^^ PYI041
|

PYI041.pyi:33:24: PYI041 Use `float` instead of `int | float`
|
33 | async def f4(**kwargs: int | int | float) -> None: ... # PYI041
| ^^^^^^^^^^^^^^^^^ PYI041
|

PYI041.pyi:39:24: PYI041 Use `complex` instead of `float | complex`
|
37 | def good(self, arg: int) -> None: ...
38 |
39 | def bad(self, arg: int | float | complex) -> None: ... # PYI041
| ^^^^^^^^^^^^^^^^^^^^^ PYI041
|

PYI041.pyi:39:24: PYI041 Use `complex` instead of `int | complex`
|
37 | def good(self, arg: int) -> None: ...
38 |
39 | def bad(self, arg: int | float | complex) -> None: ... # PYI041
| ^^^^^^^^^^^^^^^^^^^^^ PYI041
|


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.

0 comments on commit d56880c

Please sign in to comment.