From 837e70677b23329ff158e7664cbb2bf447a8ef2d Mon Sep 17 00:00:00 2001 From: Justin Prieto Date: Fri, 19 May 2023 11:39:55 -0400 Subject: [PATCH] [`flake8-pyi`] Implement `PYI013` (#4517) --- .../test/fixtures/flake8_pyi/PYI013.py | 65 +++++++ .../test/fixtures/flake8_pyi/PYI013.pyi | 56 ++++++ crates/ruff/src/checkers/ast/mod.rs | 7 + crates/ruff/src/codes.rs | 1 + crates/ruff/src/registry.rs | 1 + crates/ruff/src/rules/flake8_pyi/mod.rs | 2 + .../rules/ellipsis_in_non_empty_class_body.rs | 93 +++++++++ crates/ruff/src/rules/flake8_pyi/rules/mod.rs | 4 + ...__flake8_pyi__tests__PYI013_PYI013.py.snap | 4 + ..._flake8_pyi__tests__PYI013_PYI013.pyi.snap | 177 ++++++++++++++++++ ruff.schema.json | 1 + 11 files changed, 411 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI013.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI013.pyi create mode 100644 crates/ruff/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI013_PYI013.py.snap create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI013_PYI013.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI013.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI013.py new file mode 100644 index 0000000000..9b3635962e --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI013.py @@ -0,0 +1,65 @@ +class OneAttributeClass: + value: int + ... + + +class OneAttributeClass2: + ... + value: int + + +class TwoEllipsesClass: + ... + ... + + +class DocstringClass: + """ + My body only contains an ellipsis. + """ + + ... + + +class NonEmptyChild(Exception): + value: int + ... + + +class NonEmptyChild2(Exception): + ... + value: int + + +class NonEmptyWithInit: + value: int + ... + + def __init__(): + pass + + +class EmptyClass: + ... + + +class EmptyEllipsis: + ... + + +class Dog: + eyes: int = 2 + + +class WithInit: + value: int = 0 + + def __init__(): + ... + + +def function(): + ... + + +... diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI013.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI013.pyi new file mode 100644 index 0000000000..aaf2cb0f79 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI013.pyi @@ -0,0 +1,56 @@ +# Violations of PYI013 + +class OneAttributeClass: + value: int + ... # Error + +class OneAttributeClass2: + ... # Error + value: int + +class MyClass: + ... + value: int + +class TwoEllipsesClass: + ... + ... # Error + +class DocstringClass: + """ + My body only contains an ellipsis. + """ + + ... # Error + +class NonEmptyChild(Exception): + value: int + ... # Error + +class NonEmptyChild2(Exception): + ... # Error + value: int + +class NonEmptyWithInit: + value: int + ... # Error + + def __init__(): + pass + +# Not violations + +class EmptyClass: ... +class EmptyEllipsis: ... + +class Dog: + eyes: int = 2 + +class WithInit: + value: int = 0 + + def __init__(): ... + +def function(): ... + +... diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index f7f835dbcb..3a41d785ab 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -742,6 +742,13 @@ where if self.settings.rules.enabled(Rule::PassInClassBody) { flake8_pyi::rules::pass_in_class_body(self, stmt, body); } + if self + .settings + .rules + .enabled(Rule::EllipsisInNonEmptyClassBody) + { + flake8_pyi::rules::ellipsis_in_non_empty_class_body(self, stmt, body); + } } if self diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 9580d3e94e..7d2ddb5160 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -582,6 +582,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "010") => (RuleGroup::Unspecified, Rule::NonEmptyStubBody), (Flake8Pyi, "011") => (RuleGroup::Unspecified, Rule::TypedArgumentDefaultInStub), (Flake8Pyi, "012") => (RuleGroup::Unspecified, Rule::PassInClassBody), + (Flake8Pyi, "013") => (RuleGroup::Unspecified, Rule::EllipsisInNonEmptyClassBody), (Flake8Pyi, "014") => (RuleGroup::Unspecified, Rule::ArgumentDefaultInStub), (Flake8Pyi, "015") => (RuleGroup::Unspecified, Rule::AssignmentDefaultInStub), (Flake8Pyi, "016") => (RuleGroup::Unspecified, Rule::DuplicateUnionMember), diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 1411ba792e..d5c0cc0715 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -512,6 +512,7 @@ ruff_macros::register_rules!( rules::flake8_pyi::rules::BadVersionInfoComparison, rules::flake8_pyi::rules::DocstringInStub, rules::flake8_pyi::rules::DuplicateUnionMember, + rules::flake8_pyi::rules::EllipsisInNonEmptyClassBody, rules::flake8_pyi::rules::NonEmptyStubBody, rules::flake8_pyi::rules::PassInClassBody, rules::flake8_pyi::rules::PassStatementStubBody, diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index e923989379..9b42ec42b8 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -23,6 +23,8 @@ mod tests { #[test_case(Rule::DocstringInStub, Path::new("PYI021.pyi"))] #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.py"))] #[test_case(Rule::DuplicateUnionMember, Path::new("PYI016.pyi"))] + #[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.py"))] + #[test_case(Rule::EllipsisInNonEmptyClassBody, Path::new("PYI013.pyi"))] #[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.py"))] #[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.pyi"))] #[test_case(Rule::PassInClassBody, Path::new("PYI012.py"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs b/crates/ruff/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs new file mode 100644 index 0000000000..966c0fa8b5 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs @@ -0,0 +1,93 @@ +use rustpython_parser::ast::{Expr, ExprConstant, Ranged, Stmt, StmtExpr}; + +use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::types::RefEquality; + +use crate::autofix::actions::delete_stmt; +use crate::checkers::ast::Checker; +use crate::registry::AsRule; + +/// ## What it does +/// Removes ellipses (`...`) in otherwise non-empty class bodies. +/// +/// ## Why is this bad? +/// An ellipsis in a class body is only necessary if the class body is +/// otherwise empty. If the class body is non-empty, then the ellipsis +/// is redundant. +/// +/// ## Example +/// ```python +/// class Foo: +/// ... +/// value: int +/// ``` +/// +/// Use instead: +/// ```python +/// class Foo: +/// value: int +/// ``` +#[violation] +pub struct EllipsisInNonEmptyClassBody; + +impl Violation for EllipsisInNonEmptyClassBody { + const AUTOFIX: AutofixKind = AutofixKind::Sometimes; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Non-empty class body must not contain `...`") + } + + fn autofix_title(&self) -> Option { + Some("Remove unnecessary `...`".to_string()) + } +} + +/// PYI013 +pub(crate) fn ellipsis_in_non_empty_class_body<'a>( + checker: &mut Checker<'a>, + parent: &'a Stmt, + body: &'a [Stmt], +) { + // If the class body contains a single statement, then it's fine for it to be an ellipsis. + if body.len() == 1 { + return; + } + + for stmt in body { + if let Stmt::Expr(StmtExpr { value, .. }) = &stmt { + if let Expr::Constant(ExprConstant { value, .. }) = value.as_ref() { + if value.is_ellipsis() { + let mut diagnostic = Diagnostic::new(EllipsisInNonEmptyClassBody, stmt.range()); + + if checker.patch(diagnostic.kind.rule()) { + diagnostic.try_set_fix(|| { + let deleted: Vec<&Stmt> = + checker.deletions.iter().map(Into::into).collect(); + let edit = delete_stmt( + stmt, + Some(parent), + &deleted, + checker.locator, + checker.indexer, + checker.stylist, + )?; + + // In the unlikely event the class body consists solely of several + // consecutive ellipses, `delete_stmt` can actually result in a + // `pass`. + if edit.is_deletion() || edit.content() == Some("pass") { + checker.deletions.insert(RefEquality(stmt)); + } + + Ok(Fix::automatic(edit)) + }); + } + + checker.diagnostics.push(diagnostic); + } + } + } + } +} diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index c26ec99be5..14f00d1d24 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -3,6 +3,9 @@ pub(crate) use bad_version_info_comparison::{ }; pub(crate) use docstring_in_stubs::{docstring_in_stubs, DocstringInStub}; pub(crate) use duplicate_union_member::{duplicate_union_member, DuplicateUnionMember}; +pub(crate) use ellipsis_in_non_empty_class_body::{ + ellipsis_in_non_empty_class_body, EllipsisInNonEmptyClassBody, +}; pub(crate) use non_empty_stub_body::{non_empty_stub_body, NonEmptyStubBody}; pub(crate) use pass_in_class_body::{pass_in_class_body, PassInClassBody}; pub(crate) use pass_statement_stub_body::{pass_statement_stub_body, PassStatementStubBody}; @@ -24,6 +27,7 @@ pub(crate) use unrecognized_platform::{ mod bad_version_info_comparison; mod docstring_in_stubs; mod duplicate_union_member; +mod ellipsis_in_non_empty_class_body; mod non_empty_stub_body; mod pass_in_class_body; mod pass_statement_stub_body; diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI013_PYI013.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI013_PYI013.py.snap new file mode 100644 index 0000000000..d1aa2e9116 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI013_PYI013.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI013_PYI013.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI013_PYI013.pyi.snap new file mode 100644 index 0000000000..b8998c167b --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI013_PYI013.pyi.snap @@ -0,0 +1,177 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI013.pyi:5:5: PYI013 [*] Non-empty class body must not contain `...` + | +5 | class OneAttributeClass: +6 | value: int +7 | ... # Error + | ^^^ PYI013 +8 | +9 | class OneAttributeClass2: + | + = help: Remove unnecessary `...` + +ℹ Fix +2 2 | +3 3 | class OneAttributeClass: +4 4 | value: int +5 |- ... # Error +6 5 | +7 6 | class OneAttributeClass2: +8 7 | ... # Error + +PYI013.pyi:8:5: PYI013 [*] Non-empty class body must not contain `...` + | + 8 | class OneAttributeClass2: + 9 | ... # Error + | ^^^ PYI013 +10 | value: int + | + = help: Remove unnecessary `...` + +ℹ Fix +5 5 | ... # Error +6 6 | +7 7 | class OneAttributeClass2: +8 |- ... # Error +9 8 | value: int +10 9 | +11 10 | class MyClass: + +PYI013.pyi:12:5: PYI013 [*] Non-empty class body must not contain `...` + | +12 | class MyClass: +13 | ... + | ^^^ PYI013 +14 | value: int + | + = help: Remove unnecessary `...` + +ℹ Fix +9 9 | value: int +10 10 | +11 11 | class MyClass: +12 |- ... +13 12 | value: int +14 13 | +15 14 | class TwoEllipsesClass: + +PYI013.pyi:16:5: PYI013 [*] Non-empty class body must not contain `...` + | +16 | class TwoEllipsesClass: +17 | ... + | ^^^ PYI013 +18 | ... # Error + | + = help: Remove unnecessary `...` + +ℹ Fix +13 13 | value: int +14 14 | +15 15 | class TwoEllipsesClass: +16 |- ... +17 16 | ... # Error +18 17 | +19 18 | class DocstringClass: + +PYI013.pyi:17:5: PYI013 [*] Non-empty class body must not contain `...` + | +17 | class TwoEllipsesClass: +18 | ... +19 | ... # Error + | ^^^ PYI013 +20 | +21 | class DocstringClass: + | + = help: Remove unnecessary `...` + +ℹ Fix +14 14 | +15 15 | class TwoEllipsesClass: +16 16 | ... +17 |- ... # Error + 17 |+ pass # Error +18 18 | +19 19 | class DocstringClass: +20 20 | """ + +PYI013.pyi:24:5: PYI013 [*] Non-empty class body must not contain `...` + | +24 | """ +25 | +26 | ... # Error + | ^^^ PYI013 +27 | +28 | class NonEmptyChild(Exception): + | + = help: Remove unnecessary `...` + +ℹ Fix +21 21 | My body only contains an ellipsis. +22 22 | """ +23 23 | +24 |- ... # Error +25 24 | +26 25 | class NonEmptyChild(Exception): +27 26 | value: int + +PYI013.pyi:28:5: PYI013 [*] Non-empty class body must not contain `...` + | +28 | class NonEmptyChild(Exception): +29 | value: int +30 | ... # Error + | ^^^ PYI013 +31 | +32 | class NonEmptyChild2(Exception): + | + = help: Remove unnecessary `...` + +ℹ Fix +25 25 | +26 26 | class NonEmptyChild(Exception): +27 27 | value: int +28 |- ... # Error +29 28 | +30 29 | class NonEmptyChild2(Exception): +31 30 | ... # Error + +PYI013.pyi:31:5: PYI013 [*] Non-empty class body must not contain `...` + | +31 | class NonEmptyChild2(Exception): +32 | ... # Error + | ^^^ PYI013 +33 | value: int + | + = help: Remove unnecessary `...` + +ℹ Fix +28 28 | ... # Error +29 29 | +30 30 | class NonEmptyChild2(Exception): +31 |- ... # Error +32 31 | value: int +33 32 | +34 33 | class NonEmptyWithInit: + +PYI013.pyi:36:5: PYI013 [*] Non-empty class body must not contain `...` + | +36 | class NonEmptyWithInit: +37 | value: int +38 | ... # Error + | ^^^ PYI013 +39 | +40 | def __init__(): + | + = help: Remove unnecessary `...` + +ℹ Fix +33 33 | +34 34 | class NonEmptyWithInit: +35 35 | value: int +36 |- ... # Error +37 36 | +38 37 | def __init__(): +39 38 | pass + + diff --git a/ruff.schema.json b/ruff.schema.json index 2d66ecb020..747b1a490a 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2159,6 +2159,7 @@ "PYI010", "PYI011", "PYI012", + "PYI013", "PYI014", "PYI015", "PYI016",