[flake8-pyi] Implement PYI017 (#5895)

## Summary

Implements `PYI017` or `Y017` from `flake8-pyi` plug-in. Mirrors
[upstream
implementation](ceab86d16b/pyi.py (L1039-L1048)).
It checks for any assignment with more than 1 target or an assignment to
anything other than a name, and raises a violation for these in stub
files.

Couldn't find a clear and concise explanation for why this is to be
avoided and what is preferred for attribute cases like:

```python
a.b = int
```
So welcome some input there, to learn and to finish up the docs.

## Test Plan

Added test cases from upstream plug-in in a fixture (both `.py` and
`.pyi`). Added a few more.

## Issue link

Refers: https://github.com/astral-sh/ruff/issues/848
This commit is contained in:
qdegraaf 2023-07-20 18:35:38 +02:00 committed by GitHub
parent c948dcc203
commit 2cde9b8aa6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 141 additions and 1 deletions

View file

@ -0,0 +1,14 @@
var: int
a = var # OK
b = c = int # OK
a.b = int # OK
d, e = int, str # OK
f, g, h = int, str, TypeVar("T") # OK
i: TypeAlias = int | str # OK
j: TypeAlias = int # OK

View file

@ -0,0 +1,14 @@
var: int
a = var # OK
b = c = int # PYI017
a.b = int # PYI017
d, e = int, str # PYI017
f, g, h = int, str, TypeVar("T") # PYI017
i: TypeAlias = int | str # OK
j: TypeAlias = int # OK

View file

@ -1492,7 +1492,7 @@ where
tryceratops::rules::error_instead_of_exception(self, handlers); tryceratops::rules::error_instead_of_exception(self, handlers);
} }
} }
Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { Stmt::Assign(stmt_assign @ ast::StmtAssign { targets, value, .. }) => {
if self.enabled(Rule::LambdaAssignment) { if self.enabled(Rule::LambdaAssignment) {
if let [target] = &targets[..] { if let [target] = &targets[..] {
pycodestyle::rules::lambda_assignment(self, target, value, None, stmt); pycodestyle::rules::lambda_assignment(self, target, value, None, stmt);
@ -1557,6 +1557,7 @@ where
Rule::UnprefixedTypeParam, Rule::UnprefixedTypeParam,
Rule::AssignmentDefaultInStub, Rule::AssignmentDefaultInStub,
Rule::UnannotatedAssignmentInStub, Rule::UnannotatedAssignmentInStub,
Rule::ComplexAssignmentInStub,
Rule::TypeAliasWithoutAnnotation, Rule::TypeAliasWithoutAnnotation,
]) { ]) {
// Ignore assignments in function bodies; those are covered by other rules. // Ignore assignments in function bodies; those are covered by other rules.
@ -1576,6 +1577,9 @@ where
self, targets, value, self, targets, value,
); );
} }
if self.enabled(Rule::ComplexAssignmentInStub) {
flake8_pyi::rules::complex_assignment_in_stub(self, stmt_assign);
}
if self.enabled(Rule::TypeAliasWithoutAnnotation) { if self.enabled(Rule::TypeAliasWithoutAnnotation) {
flake8_pyi::rules::type_alias_without_annotation( flake8_pyi::rules::type_alias_without_annotation(
self, value, targets, self, value, targets,

View file

@ -629,6 +629,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Pyi, "014") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::ArgumentDefaultInStub), (Flake8Pyi, "014") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::ArgumentDefaultInStub),
(Flake8Pyi, "015") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::AssignmentDefaultInStub), (Flake8Pyi, "015") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::AssignmentDefaultInStub),
(Flake8Pyi, "016") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DuplicateUnionMember), (Flake8Pyi, "016") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DuplicateUnionMember),
(Flake8Pyi, "017") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::ComplexAssignmentInStub),
(Flake8Pyi, "020") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::QuotedAnnotationInStub), (Flake8Pyi, "020") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::QuotedAnnotationInStub),
(Flake8Pyi, "021") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DocstringInStub), (Flake8Pyi, "021") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::DocstringInStub),
(Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple), (Flake8Pyi, "024") => (RuleGroup::Unspecified, rules::flake8_pyi::rules::CollectionsNamedTuple),

View file

@ -25,6 +25,8 @@ mod tests {
#[test_case(Rule::BadVersionInfoComparison, Path::new("PYI006.pyi"))] #[test_case(Rule::BadVersionInfoComparison, Path::new("PYI006.pyi"))]
#[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.py"))] #[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.py"))]
#[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.pyi"))] #[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.pyi"))]
#[test_case(Rule::ComplexAssignmentInStub, Path::new("PYI017.py"))]
#[test_case(Rule::ComplexAssignmentInStub, Path::new("PYI017.pyi"))]
#[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.py"))] #[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.py"))]
#[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.pyi"))] #[test_case(Rule::ComplexIfStatementInStub, Path::new("PYI002.pyi"))]
#[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))] #[test_case(Rule::DocstringInStub, Path::new("PYI021.py"))]

View file

@ -0,0 +1,54 @@
use rustpython_parser::ast::{Expr, StmtAssign};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for assignments with multiple or non-name targets in stub files.
///
/// ## Why is this bad?
/// In general, stub files should be thought of as "data files" for a type
/// checker, and are not intended to be executed. As such, it's useful to
/// enforce that only a subset of Python syntax is allowed in a stub file, to
/// ensure that everything in the stub is unambiguous for the type checker.
///
/// The need to perform multi-assignment, or assignment to a non-name target,
/// likely indicates a misunderstanding of how stub files are intended to be
/// used.
///
/// ## Example
/// ```python
/// a = b = int
/// a.b = int
/// ```
///
/// Use instead:
/// ```python
/// a: TypeAlias = int
/// b: TypeAlias = int
///
///
/// class a:
/// b: int
/// ```
#[violation]
pub struct ComplexAssignmentInStub;
impl Violation for ComplexAssignmentInStub {
#[derive_message_formats]
fn message(&self) -> String {
format!("Stubs should not contain assignments to attributes or multiple targets")
}
}
/// PYI017
pub(crate) fn complex_assignment_in_stub(checker: &mut Checker, stmt: &StmtAssign) {
if matches!(stmt.targets.as_slice(), [Expr::Name(_)]) {
return;
}
checker
.diagnostics
.push(Diagnostic::new(ComplexAssignmentInStub, stmt.range));
}

View file

@ -1,6 +1,7 @@
pub(crate) use any_eq_ne_annotation::*; pub(crate) use any_eq_ne_annotation::*;
pub(crate) use bad_version_info_comparison::*; pub(crate) use bad_version_info_comparison::*;
pub(crate) use collections_named_tuple::*; pub(crate) use collections_named_tuple::*;
pub(crate) use complex_assignment_in_stub::*;
pub(crate) use complex_if_statement_in_stub::*; pub(crate) use complex_if_statement_in_stub::*;
pub(crate) use docstring_in_stubs::*; pub(crate) use docstring_in_stubs::*;
pub(crate) use duplicate_union_member::*; pub(crate) use duplicate_union_member::*;
@ -31,6 +32,7 @@ pub(crate) use unrecognized_version_info::*;
mod any_eq_ne_annotation; mod any_eq_ne_annotation;
mod bad_version_info_comparison; mod bad_version_info_comparison;
mod collections_named_tuple; mod collections_named_tuple;
mod complex_assignment_in_stub;
mod complex_if_statement_in_stub; mod complex_if_statement_in_stub;
mod docstring_in_stubs; mod docstring_in_stubs;
mod duplicate_union_member; mod duplicate_union_member;

View file

@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---

View file

@ -0,0 +1,44 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI017.pyi:4:1: PYI017 Stubs should not contain assignments to attributes or multiple targets
|
2 | a = var # OK
3 |
4 | b = c = int # PYI017
| ^^^^^^^^^^^ PYI017
5 |
6 | a.b = int # PYI017
|
PYI017.pyi:6:1: PYI017 Stubs should not contain assignments to attributes or multiple targets
|
4 | b = c = int # PYI017
5 |
6 | a.b = int # PYI017
| ^^^^^^^^^ PYI017
7 |
8 | d, e = int, str # PYI017
|
PYI017.pyi:8:1: PYI017 Stubs should not contain assignments to attributes or multiple targets
|
6 | a.b = int # PYI017
7 |
8 | d, e = int, str # PYI017
| ^^^^^^^^^^^^^^^ PYI017
9 |
10 | f, g, h = int, str, TypeVar("T") # PYI017
|
PYI017.pyi:10:1: PYI017 Stubs should not contain assignments to attributes or multiple targets
|
8 | d, e = int, str # PYI017
9 |
10 | f, g, h = int, str, TypeVar("T") # PYI017
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI017
11 |
12 | i: TypeAlias = int | str # OK
|

1
ruff.schema.json generated
View file

@ -2353,6 +2353,7 @@
"PYI014", "PYI014",
"PYI015", "PYI015",
"PYI016", "PYI016",
"PYI017",
"PYI02", "PYI02",
"PYI020", "PYI020",
"PYI021", "PYI021",