From c8e0435bf34298c1ff322c8b25fa60c5ec3aa1d6 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 31 Aug 2023 11:11:48 -0400 Subject: [PATCH] Improve compatibility between multi-statement PYI rules --- .../test/fixtures/flake8_pyi/PYI010.py | 12 ++++--- .../test/fixtures/flake8_pyi/PYI010.pyi | 5 ++- .../test/fixtures/flake8_pyi/PYI048.py | 18 ++++++++--- .../test/fixtures/flake8_pyi/PYI048.pyi | 18 +++++------ .../flake8_pyi/rules/non_empty_stub_body.rs | 31 ++++++++++++++----- .../rules/pass_statement_stub_body.rs | 21 ++++++------- .../rules/stub_body_multiple_statements.rs | 25 +++++---------- ..._flake8_pyi__tests__PYI010_PYI010.pyi.snap | 13 +++++--- ..._flake8_pyi__tests__PYI048_PYI048.pyi.snap | 24 +++++++++++--- 9 files changed, 103 insertions(+), 64 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.py index 1c6afa31ad129..b8235724c2282 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.py +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.py @@ -3,16 +3,20 @@ def bar(): def foo(): - """foo""" # OK + """foo""" # OK, docstrings are handled by another rule def buzz(): - print("buzz") # OK, not in stub file + print("buzz") # ERROR PYI010 def foo2(): - 123 # OK, not in a stub file + 123 # ERROR PYI010 def bizz(): - x = 123 # OK, not in a stub file + x = 123 # ERROR PYI010 + + +def foo3(): + pass # OK, pass is handled by another rule diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.pyi index b49859d8246c0..11346a51961cd 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.pyi +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.pyi @@ -1,6 +1,6 @@ def bar(): ... # OK def foo(): - """foo""" # OK, strings are handled by another rule + """foo""" # OK, docstrings are handled by another rule def buzz(): print("buzz") # ERROR PYI010 @@ -10,3 +10,6 @@ def foo2(): def bizz(): x = 123 # ERROR PYI010 + +def foo3(): + pass # OK, pass is handled by another rule diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.py index 8ec21f2d31c1d..350a18120f8af 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.py +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.py @@ -1,19 +1,27 @@ -def bar(): # OK - ... +def bar(): + ... # OK -def oof(): # OK, docstrings are handled by another rule +def bar(): + pass # OK + + +def bar(): + """oof""" # OK + + +def oof(): # ERROR PYI048 """oof""" print("foo") -def foo(): # Ok not in Stub file +def foo(): # ERROR PYI048 """foo""" print("foo") print("foo") -def buzz(): # Ok not in Stub file +def buzz(): # ERROR PYI048 print("fizz") print("buzz") print("test") diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.pyi index 29a2120f94359..fe4246396a1dc 100644 --- a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.pyi +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.pyi @@ -1,20 +1,20 @@ +def bar(): ... # OK def bar(): - ... # OK - - -def oof(): # OK, docstrings are handled by another rule - """oof""" - print("foo") + pass # OK +def bar(): + """oof""" # OK +def oof(): # ERROR PYI048 + """oof""" + print("foo") -def foo(): # ERROR PYI048 +def foo(): # ERROR PYI048 """foo""" print("foo") print("foo") - -def buzz(): # ERROR PYI048 +def buzz(): # ERROR PYI048 print("fizz") print("buzz") print("test") diff --git a/crates/ruff/src/rules/flake8_pyi/rules/non_empty_stub_body.rs b/crates/ruff/src/rules/flake8_pyi/rules/non_empty_stub_body.rs index 9ac914d2f97dc..2432df8faf79f 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/non_empty_stub_body.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/non_empty_stub_body.rs @@ -1,7 +1,7 @@ -use ruff_python_ast::{self as ast, Constant, Expr, Stmt}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::is_docstring_stmt; +use ruff_python_ast::{self as ast, Expr, Stmt}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -23,18 +23,35 @@ impl AlwaysAutofixableViolation for NonEmptyStubBody { /// PYI010 pub(crate) fn non_empty_stub_body(checker: &mut Checker, body: &[Stmt]) { - if let [Stmt::Expr(ast::StmtExpr { value, range: _ })] = body { + // Ignore multi-statement bodies (covered by PYI048). + let [stmt] = body else { + return; + }; + + // Ignore `pass` statements (covered by PYI009). + if stmt.is_pass_stmt() { + return; + } + + // Ignore docstrings (covered by PYI021). + if is_docstring_stmt(stmt) { + return; + } + + // Ignore `...` (the desired case). + if let Stmt::Expr(ast::StmtExpr { value, range: _ }) = stmt { if let Expr::Constant(ast::ExprConstant { value, .. }) = value.as_ref() { - if matches!(value, Constant::Ellipsis | Constant::Str(_)) { + if value.is_ellipsis() { return; } } } - let mut diagnostic = Diagnostic::new(NonEmptyStubBody, body[0].range()); + + let mut diagnostic = Diagnostic::new(NonEmptyStubBody, stmt.range()); if checker.patch(Rule::NonEmptyStubBody) { - diagnostic.set_fix(Fix::automatic(Edit::range_replacement( + diagnostic.set_fix(Fix::suggested(Edit::range_replacement( format!("..."), - body[0].range(), + stmt.range(), ))); }; checker.diagnostics.push(diagnostic); diff --git a/crates/ruff/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs b/crates/ruff/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs index e85433a9c5c8a..e51b28980864d 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs @@ -22,17 +22,16 @@ impl AlwaysAutofixableViolation for PassStatementStubBody { /// PYI009 pub(crate) fn pass_statement_stub_body(checker: &mut Checker, body: &[Stmt]) { - let [stmt] = body else { + let [Stmt::Pass(pass)] = body else { return; }; - if stmt.is_pass_stmt() { - let mut diagnostic = Diagnostic::new(PassStatementStubBody, stmt.range()); - if checker.patch(Rule::PassStatementStubBody) { - diagnostic.set_fix(Fix::automatic(Edit::range_replacement( - format!("..."), - stmt.range(), - ))); - }; - checker.diagnostics.push(diagnostic); - } + + let mut diagnostic = Diagnostic::new(PassStatementStubBody, pass.range()); + if checker.patch(Rule::PassStatementStubBody) { + diagnostic.set_fix(Fix::automatic(Edit::range_replacement( + format!("..."), + pass.range(), + ))); + }; + checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff/src/rules/flake8_pyi/rules/stub_body_multiple_statements.rs b/crates/ruff/src/rules/flake8_pyi/rules/stub_body_multiple_statements.rs index 6390441badcb7..77c20d5e942c9 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/stub_body_multiple_statements.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/stub_body_multiple_statements.rs @@ -1,9 +1,7 @@ -use ruff_python_ast::Stmt; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::is_docstring_stmt; use ruff_python_ast::identifier::Identifier; +use ruff_python_ast::Stmt; use crate::checkers::ast::Checker; @@ -17,21 +15,12 @@ impl Violation for StubBodyMultipleStatements { } } -/// PYI010 +/// PYI048 pub(crate) fn stub_body_multiple_statements(checker: &mut Checker, stmt: &Stmt, body: &[Stmt]) { - // If the function body consists of exactly one statement, abort. - if body.len() == 1 { - return; + if body.len() > 1 { + checker.diagnostics.push(Diagnostic::new( + StubBodyMultipleStatements, + stmt.identifier(), + )); } - - // If the function body consists of exactly two statements, and the first is a - // docstring, abort (this is covered by PYI021). - if body.len() == 2 && is_docstring_stmt(&body[0]) { - return; - } - - checker.diagnostics.push(Diagnostic::new( - StubBodyMultipleStatements, - stmt.identifier(), - )); } diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI010_PYI010.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI010_PYI010.pyi.snap index ad0bb4775fd42..1e757d88726ec 100644 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI010_PYI010.pyi.snap +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI010_PYI010.pyi.snap @@ -11,8 +11,8 @@ PYI010.pyi:6:5: PYI010 [*] Function body must contain only `...` | = help: Replace function body with `...` -ℹ Fix -3 3 | """foo""" # OK, strings are handled by another rule +ℹ Suggested fix +3 3 | """foo""" # OK, docstrings are handled by another rule 4 4 | 5 5 | def buzz(): 6 |- print("buzz") # ERROR PYI010 @@ -31,7 +31,7 @@ PYI010.pyi:9:5: PYI010 [*] Function body must contain only `...` | = help: Replace function body with `...` -ℹ Fix +ℹ Suggested fix 6 6 | print("buzz") # ERROR PYI010 7 7 | 8 8 | def foo2(): @@ -46,14 +46,19 @@ PYI010.pyi:12:5: PYI010 [*] Function body must contain only `...` 11 | def bizz(): 12 | x = 123 # ERROR PYI010 | ^^^^^^^ PYI010 +13 | +14 | def foo3(): | = help: Replace function body with `...` -ℹ Fix +ℹ Suggested fix 9 9 | 123 # ERROR PYI010 10 10 | 11 11 | def bizz(): 12 |- x = 123 # ERROR PYI010 12 |+ ... # ERROR PYI010 +13 13 | +14 14 | def foo3(): +15 15 | pass # OK, pass is handled by another rule diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI048_PYI048.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI048_PYI048.pyi.snap index 725d9b7b4d068..d897255d5db59 100644 --- a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI048_PYI048.pyi.snap +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI048_PYI048.pyi.snap @@ -1,17 +1,31 @@ --- source: crates/ruff/src/rules/flake8_pyi/mod.rs --- -PYI048.pyi:11:5: PYI048 Function body must contain exactly one statement +PYI048.pyi:8:5: PYI048 Function body must contain exactly one statement | -11 | def foo(): # ERROR PYI048 + 6 | """oof""" # OK + 7 | + 8 | def oof(): # ERROR PYI048 | ^^^ PYI048 -12 | """foo""" -13 | print("foo") + 9 | """oof""" +10 | print("foo") + | + +PYI048.pyi:12:5: PYI048 Function body must contain exactly one statement + | +10 | print("foo") +11 | +12 | def foo(): # ERROR PYI048 + | ^^^ PYI048 +13 | """foo""" +14 | print("foo") | PYI048.pyi:17:5: PYI048 Function body must contain exactly one statement | -17 | def buzz(): # ERROR PYI048 +15 | print("foo") +16 | +17 | def buzz(): # ERROR PYI048 | ^^^^ PYI048 18 | print("fizz") 19 | print("buzz")