Skip to content

Commit

Permalink
[flake8-pyi] Implement PYI026 (#5844)
Browse files Browse the repository at this point in the history
## Summary
Checks for `typehint.TypeAlias` annotation in type aliases. See
[original
source](https://github.com/PyCQA/flake8-pyi/blob/main/pyi.py#L1085).
```
$ flake8 --select Y026 crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:4:1: Y026 Use typing_extensions.TypeAlias for type aliases, e.g. "NewAny: TypeAlias = Any"
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:5:1: Y026 Use typing_extensions.TypeAlias for type aliases, e.g. "OptinalStr: TypeAlias = typing.Optional[str]"
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:6:1: Y026 Use typing_extensions.TypeAlias for type aliases, e.g. "Foo: TypeAlias = Literal['foo']"
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:7:1: Y026 Use typing_extensions.TypeAlias for type aliases, e.g. "IntOrStr: TypeAlias = int | str"
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:8:1: Y026 Use typing_extensions.TypeAlias for type aliases, e.g. "AliasNone: TypeAlias = None"
```

```
$ ./target/debug/ruff --select PYI026 crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi --no-cache
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:4:1: PYI026 Use `typing.TypeAlias` for type aliases in `NewAny`, e.g. "NewAny: typing.TypeAlias = Any"
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:5:1: PYI026 Use `typing.TypeAlias` for type aliases in `OptinalStr`, e.g. "OptinalStr: typing.TypeAlias = typing.Optional[str]"
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:6:1: PYI026 Use `typing.TypeAlias` for type aliases in `Foo`, e.g. "Foo: typing.TypeAlias = Literal["foo"]"
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:7:1: PYI026 Use `typing.TypeAlias` for type aliases in `IntOrStr`, e.g. "IntOrStr: typing.TypeAlias = int | str"
crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi:8:1: PYI026 Use `typing.TypeAlias` for type aliases in `AliasNone`, e.g. "AliasNone: typing.TypeAlias = None"
Found 5 errors.
```

ref: #848 

## Test Plan

Snapshots, manual runs of flake8.
  • Loading branch information
LaBatata101 committed Jul 20, 2023
1 parent 963f240 commit a37d915
Show file tree
Hide file tree
Showing 9 changed files with 296 additions and 15 deletions.
19 changes: 19 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import typing
from typing import TypeAlias, Literal, Any

NewAny = Any
OptionalStr = typing.Optional[str]
Foo = Literal["foo"]
IntOrStr = int | str
AliasNone = None

NewAny: typing.TypeAlias = Any
OptionalStr: TypeAlias = typing.Optional[str]
Foo: typing.TypeAlias = Literal["foo"]
IntOrStr: TypeAlias = int | str
IntOrFloat: Foo = int | float
AliasNone: typing.TypeAlias = None

# these are ok
VarAlias = str
AliasFoo = Foo
18 changes: 18 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI026.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from typing import Literal, Any

NewAny = Any
OptionalStr = typing.Optional[str]
Foo = Literal["foo"]
IntOrStr = int | str
AliasNone = None

NewAny: typing.TypeAlias = Any
OptionalStr: TypeAlias = typing.Optional[str]
Foo: typing.TypeAlias = Literal["foo"]
IntOrStr: TypeAlias = int | str
IntOrFloat: Foo = int | float
AliasNone: typing.TypeAlias = None

# these are ok
VarAlias = str
AliasFoo = Foo
6 changes: 6 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1557,6 +1557,7 @@ where
Rule::UnprefixedTypeParam,
Rule::AssignmentDefaultInStub,
Rule::UnannotatedAssignmentInStub,
Rule::TypeAliasWithoutAnnotation,
]) {
// Ignore assignments in function bodies; those are covered by other rules.
if !self
Expand All @@ -1575,6 +1576,11 @@ where
self, targets, value,
);
}
if self.enabled(Rule::TypeAliasWithoutAnnotation) {
flake8_pyi::rules::type_alias_without_annotation(
self, value, targets,
);
}
}
}
}
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 @@ -633,6 +633,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Pyi, "021") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DocstringInStub),
(Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple),
(Flake8Pyi, "025") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnaliasedCollectionsAbcSetImport),
(Flake8Pyi, "026") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::TypeAliasWithoutAnnotation),
(Flake8Pyi, "029") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::StrOrReprDefinedInStub),
(Flake8Pyi, "030") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::UnnecessaryLiteralUnion),
(Flake8Pyi, "032") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::AnyEqNeAnnotation),
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 @@ -87,6 +87,8 @@ mod tests {
#[test_case(Rule::UnrecognizedVersionInfoCheck, Path::new("PYI003.pyi"))]
#[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.py"))]
#[test_case(Rule::WrongTupleLengthVersionComparison, Path::new("PYI005.pyi"))]
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.py"))]
#[test_case(Rule::TypeAliasWithoutAnnotation, Path::new("PYI026.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
143 changes: 128 additions & 15 deletions crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use ruff_python_ast::source_code::Locator;
use ruff_python_semantic::{ScopeKind, SemanticModel};

use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
use crate::registry::AsRule;

#[violation]
Expand Down Expand Up @@ -97,6 +98,47 @@ impl Violation for UnassignedSpecialVariableInStub {
}
}

/// ## What it does
/// Checks for type alias definitions that are not annotated with
/// `typing.TypeAlias`.
///
/// ## Why is this bad?
/// In Python, a type alias is defined by assigning a type to a variable (e.g.,
/// `Vector = list[float]`).
///
/// It's best to annotate type aliases with the `typing.TypeAlias` type to
/// make it clear that the statement is a type alias declaration, as opposed
/// to a normal variable assignment.
///
/// ## Example
/// ```python
/// Vector = list[float]
/// ```
///
/// Use instead:
/// ```python
/// from typing import TypeAlias
///
/// Vector: TypeAlias = list[float]
/// ```
#[violation]
pub struct TypeAliasWithoutAnnotation {
name: String,
value: String,
}

impl AlwaysAutofixableViolation for TypeAliasWithoutAnnotation {
#[derive_message_formats]
fn message(&self) -> String {
let TypeAliasWithoutAnnotation { name, value } = self;
format!("Use `typing.TypeAlias` for type alias, e.g., `{name}: typing.TypeAlias = {value}`")
}

fn autofix_title(&self) -> String {
"Add `typing.TypeAlias` annotation".to_string()
}
}

fn is_allowed_negated_math_attribute(call_path: &CallPath) -> bool {
matches!(call_path.as_slice(), ["math", "inf" | "e" | "pi" | "tau"])
}
Expand Down Expand Up @@ -234,22 +276,39 @@ fn is_valid_default_value_with_annotation(

/// Returns `true` if an [`Expr`] appears to be a valid PEP 604 union. (e.g. `int | None`)
fn is_valid_pep_604_union(annotation: &Expr) -> bool {
match annotation {
Expr::BinOp(ast::ExprBinOp {
left,
op: Operator::BitOr,
right,
range: _,
}) => is_valid_pep_604_union(left) && is_valid_pep_604_union(right),
Expr::Name(_)
| Expr::Subscript(_)
| Expr::Attribute(_)
| Expr::Constant(ast::ExprConstant {
value: Constant::None,
..
}) => true,
_ => false,
/// Returns `true` if an [`Expr`] appears to be a valid PEP 604 union member.
fn is_valid_pep_604_union_member(value: &Expr) -> bool {
match value {
Expr::BinOp(ast::ExprBinOp {
left,
op: Operator::BitOr,
right,
range: _,
}) => is_valid_pep_604_union_member(left) && is_valid_pep_604_union_member(right),
Expr::Name(_)
| Expr::Subscript(_)
| Expr::Attribute(_)
| Expr::Constant(ast::ExprConstant {
value: Constant::None,
..
}) => true,
_ => false,
}
}

// The top-level expression must be a bit-or operation.
let Expr::BinOp(ast::ExprBinOp {
left,
op: Operator::BitOr,
right,
range: _,
}) = annotation
else {
return false;
};

// The left and right operands must be valid union members.
is_valid_pep_604_union_member(left) && is_valid_pep_604_union_member(right)
}

/// Returns `true` if an [`Expr`] appears to be a valid default value without an annotation.
Expand Down Expand Up @@ -323,6 +382,23 @@ fn is_enum(bases: &[Expr], semantic: &SemanticModel) -> bool {
});
}

/// Returns `true` if an [`Expr`] is a value that should be annotated with `typing.TypeAlias`.
///
/// This is relatively conservative, as it's hard to reliably detect whether a right-hand side is a
/// valid type alias. In particular, this function checks for uses of `typing.Any`, `None`,
/// parameterized generics, and PEP 604-style unions.
fn is_annotatable_type_alias(value: &Expr, semantic: &SemanticModel) -> bool {
matches!(
value,
Expr::Subscript(_)
| Expr::Constant(ast::ExprConstant {
value: Constant::None,
..
}),
) || is_valid_pep_604_union(value)
|| semantic.match_typing_expr(value, "Any")
}

/// PYI011
pub(crate) fn typed_argument_simple_defaults(checker: &mut Checker, arguments: &Arguments) {
for ArgWithDefault {
Expand Down Expand Up @@ -523,3 +599,40 @@ pub(crate) fn unassigned_special_variable_in_stub(
stmt.range(),
));
}

/// PIY026
pub(crate) fn type_alias_without_annotation(checker: &mut Checker, value: &Expr, targets: &[Expr]) {
let [target] = targets else {
return;
};

let Expr::Name(ast::ExprName { id, .. }) = target else {
return;
};

if !is_annotatable_type_alias(value, checker.semantic()) {
return;
}

let mut diagnostic = Diagnostic::new(
TypeAliasWithoutAnnotation {
name: id.to_string(),
value: checker.generator().expr(value),
},
target.range(),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer.get_or_import_symbol(
&ImportRequest::import("typing", "TypeAlias"),
target.start(),
checker.semantic(),
)?;
Ok(Fix::suggested_edits(
Edit::range_replacement(format!("{id}: {binding}"), target.range()),
[import_edit],
))
});
}
checker.diagnostics.push(diagnostic);
}
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,117 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI026.pyi:3:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `NewAny: typing.TypeAlias = Any`
|
1 | from typing import Literal, Any
2 |
3 | NewAny = Any
| ^^^^^^ PYI026
4 | OptionalStr = typing.Optional[str]
5 | Foo = Literal["foo"]
|
= help: Add `typing.TypeAlias` annotation

Suggested fix
1 |-from typing import Literal, Any
1 |+from typing import Literal, Any, TypeAlias
2 2 |
3 |-NewAny = Any
3 |+NewAny: TypeAlias = Any
4 4 | OptionalStr = typing.Optional[str]
5 5 | Foo = Literal["foo"]
6 6 | IntOrStr = int | str

PYI026.pyi:4:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `OptionalStr: typing.TypeAlias = typing.Optional[str]`
|
3 | NewAny = Any
4 | OptionalStr = typing.Optional[str]
| ^^^^^^^^^^^ PYI026
5 | Foo = Literal["foo"]
6 | IntOrStr = int | str
|
= help: Add `typing.TypeAlias` annotation

Suggested fix
1 |-from typing import Literal, Any
1 |+from typing import Literal, Any, TypeAlias
2 2 |
3 3 | NewAny = Any
4 |-OptionalStr = typing.Optional[str]
4 |+OptionalStr: TypeAlias = typing.Optional[str]
5 5 | Foo = Literal["foo"]
6 6 | IntOrStr = int | str
7 7 | AliasNone = None

PYI026.pyi:5:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `Foo: typing.TypeAlias = Literal["foo"]`
|
3 | NewAny = Any
4 | OptionalStr = typing.Optional[str]
5 | Foo = Literal["foo"]
| ^^^ PYI026
6 | IntOrStr = int | str
7 | AliasNone = None
|
= help: Add `typing.TypeAlias` annotation

Suggested fix
1 |-from typing import Literal, Any
1 |+from typing import Literal, Any, TypeAlias
2 2 |
3 3 | NewAny = Any
4 4 | OptionalStr = typing.Optional[str]
5 |-Foo = Literal["foo"]
5 |+Foo: TypeAlias = Literal["foo"]
6 6 | IntOrStr = int | str
7 7 | AliasNone = None
8 8 |

PYI026.pyi:6:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `IntOrStr: typing.TypeAlias = int | str`
|
4 | OptionalStr = typing.Optional[str]
5 | Foo = Literal["foo"]
6 | IntOrStr = int | str
| ^^^^^^^^ PYI026
7 | AliasNone = None
|
= help: Add `typing.TypeAlias` annotation

Suggested fix
1 |-from typing import Literal, Any
1 |+from typing import Literal, Any, TypeAlias
2 2 |
3 3 | NewAny = Any
4 4 | OptionalStr = typing.Optional[str]
5 5 | Foo = Literal["foo"]
6 |-IntOrStr = int | str
6 |+IntOrStr: TypeAlias = int | str
7 7 | AliasNone = None
8 8 |
9 9 | NewAny: typing.TypeAlias = Any

PYI026.pyi:7:1: PYI026 [*] Use `typing.TypeAlias` for type alias, e.g., `AliasNone: typing.TypeAlias = None`
|
5 | Foo = Literal["foo"]
6 | IntOrStr = int | str
7 | AliasNone = None
| ^^^^^^^^^ PYI026
8 |
9 | NewAny: typing.TypeAlias = Any
|
= help: Add `typing.TypeAlias` annotation

Suggested fix
1 |-from typing import Literal, Any
1 |+from typing import Literal, Any, TypeAlias
2 2 |
3 3 | NewAny = Any
4 4 | OptionalStr = typing.Optional[str]
5 5 | Foo = Literal["foo"]
6 6 | IntOrStr = int | str
7 |-AliasNone = None
7 |+AliasNone: TypeAlias = None
8 8 |
9 9 | NewAny: typing.TypeAlias = Any
10 10 | OptionalStr: TypeAlias = typing.Optional[str]


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 a37d915

Please sign in to comment.