Improve compatibility between multi-statement PYI rules (#7024)

## Summary

This PR modifies a few of our rules related to which statements (and how
many) are allowed in function bodies within `.pyi` files, to improve
compatibility with flake8-pyi and improve the interplay dynamics between
them. Each change fixes a deviation from flake8-pyi:

- We now always trigger the multi-statement rule (PYI048) regardless of
whether one of the statements is a docstring.
- We no longer trigger the `...` rule (PYI010) if the single statement
is a docstring or a `pass` (since those are covered by other rules).
- We no longer trigger the `...` rule (PYI010) if the function body
contains multiple statements (since that's covered by PYI048).

Closes https://github.com/astral-sh/ruff/issues/7021.

## Test Plan

`cargo test`
This commit is contained in:
Charlie Marsh 2023-08-31 21:45:26 +01:00 committed by GitHub
parent f7dca3d958
commit 51d69b448c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 103 additions and 64 deletions

View file

@ -3,16 +3,20 @@ def bar():
def foo(): def foo():
"""foo""" # OK """foo""" # OK, docstrings are handled by another rule
def buzz(): def buzz():
print("buzz") # OK, not in stub file print("buzz") # ERROR PYI010
def foo2(): def foo2():
123 # OK, not in a stub file 123 # ERROR PYI010
def bizz(): def bizz():
x = 123 # OK, not in a stub file x = 123 # ERROR PYI010
def foo3():
pass # OK, pass is handled by another rule

View file

@ -1,6 +1,6 @@
def bar(): ... # OK def bar(): ... # OK
def foo(): def foo():
"""foo""" # OK, strings are handled by another rule """foo""" # OK, docstrings are handled by another rule
def buzz(): def buzz():
print("buzz") # ERROR PYI010 print("buzz") # ERROR PYI010
@ -10,3 +10,6 @@ def foo2():
def bizz(): def bizz():
x = 123 # ERROR PYI010 x = 123 # ERROR PYI010
def foo3():
pass # OK, pass is handled by another rule

View file

@ -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""" """oof"""
print("foo") print("foo")
def foo(): # Ok not in Stub file def foo(): # ERROR PYI048
"""foo""" """foo"""
print("foo") print("foo")
print("foo") print("foo")
def buzz(): # Ok not in Stub file def buzz(): # ERROR PYI048
print("fizz") print("fizz")
print("buzz") print("buzz")
print("test") print("test")

View file

@ -1,19 +1,19 @@
def bar(): ... # OK
def bar(): def bar():
... # OK pass # OK
def bar():
"""oof""" # OK
def oof(): # OK, docstrings are handled by another rule def oof(): # ERROR PYI048
"""oof""" """oof"""
print("foo") print("foo")
def foo(): # ERROR PYI048 def foo(): # ERROR PYI048
"""foo""" """foo"""
print("foo") print("foo")
print("foo") print("foo")
def buzz(): # ERROR PYI048 def buzz(): # ERROR PYI048
print("fizz") print("fizz")
print("buzz") print("buzz")

View file

@ -1,7 +1,7 @@
use ruff_python_ast::{self as ast, Constant, Expr, Stmt};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation}; 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 ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -23,18 +23,35 @@ impl AlwaysAutofixableViolation for NonEmptyStubBody {
/// PYI010 /// PYI010
pub(crate) fn non_empty_stub_body(checker: &mut Checker, body: &[Stmt]) { 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 let Expr::Constant(ast::ExprConstant { value, .. }) = value.as_ref() {
if matches!(value, Constant::Ellipsis | Constant::Str(_)) { if value.is_ellipsis() {
return; return;
} }
} }
} }
let mut diagnostic = Diagnostic::new(NonEmptyStubBody, body[0].range());
let mut diagnostic = Diagnostic::new(NonEmptyStubBody, stmt.range());
if checker.patch(Rule::NonEmptyStubBody) { if checker.patch(Rule::NonEmptyStubBody) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement( diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
format!("..."), format!("..."),
body[0].range(), stmt.range(),
))); )));
}; };
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);

View file

@ -22,17 +22,16 @@ impl AlwaysAutofixableViolation for PassStatementStubBody {
/// PYI009 /// PYI009
pub(crate) fn pass_statement_stub_body(checker: &mut Checker, body: &[Stmt]) { pub(crate) fn pass_statement_stub_body(checker: &mut Checker, body: &[Stmt]) {
let [stmt] = body else { let [Stmt::Pass(pass)] = body else {
return; return;
}; };
if stmt.is_pass_stmt() {
let mut diagnostic = Diagnostic::new(PassStatementStubBody, stmt.range()); let mut diagnostic = Diagnostic::new(PassStatementStubBody, pass.range());
if checker.patch(Rule::PassStatementStubBody) { if checker.patch(Rule::PassStatementStubBody) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement( diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
format!("..."), format!("..."),
stmt.range(), pass.range(),
))); )));
}; };
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }
}

View file

@ -1,9 +1,7 @@
use ruff_python_ast::Stmt;
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, 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::identifier::Identifier;
use ruff_python_ast::Stmt;
use crate::checkers::ast::Checker; 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]) { 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 {
if body.len() == 1 {
return;
}
// 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( checker.diagnostics.push(Diagnostic::new(
StubBodyMultipleStatements, StubBodyMultipleStatements,
stmt.identifier(), stmt.identifier(),
)); ));
} }
}

View file

@ -11,8 +11,8 @@ PYI010.pyi:6:5: PYI010 [*] Function body must contain only `...`
| |
= help: Replace function body with `...` = help: Replace function body with `...`
Fix Suggested fix
3 3 | """foo""" # OK, strings are handled by another rule 3 3 | """foo""" # OK, docstrings are handled by another rule
4 4 | 4 4 |
5 5 | def buzz(): 5 5 | def buzz():
6 |- print("buzz") # ERROR PYI010 6 |- print("buzz") # ERROR PYI010
@ -31,7 +31,7 @@ PYI010.pyi:9:5: PYI010 [*] Function body must contain only `...`
| |
= help: Replace function body with `...` = help: Replace function body with `...`
Fix Suggested fix
6 6 | print("buzz") # ERROR PYI010 6 6 | print("buzz") # ERROR PYI010
7 7 | 7 7 |
8 8 | def foo2(): 8 8 | def foo2():
@ -46,14 +46,19 @@ PYI010.pyi:12:5: PYI010 [*] Function body must contain only `...`
11 | def bizz(): 11 | def bizz():
12 | x = 123 # ERROR PYI010 12 | x = 123 # ERROR PYI010
| ^^^^^^^ PYI010 | ^^^^^^^ PYI010
13 |
14 | def foo3():
| |
= help: Replace function body with `...` = help: Replace function body with `...`
Fix Suggested fix
9 9 | 123 # ERROR PYI010 9 9 | 123 # ERROR PYI010
10 10 | 10 10 |
11 11 | def bizz(): 11 11 | def bizz():
12 |- x = 123 # ERROR PYI010 12 |- x = 123 # ERROR PYI010
12 |+ ... # ERROR PYI010 12 |+ ... # ERROR PYI010
13 13 |
14 14 | def foo3():
15 15 | pass # OK, pass is handled by another rule

View file

@ -1,16 +1,30 @@
--- ---
source: crates/ruff/src/rules/flake8_pyi/mod.rs 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 | ^^^ PYI048
12 | """foo""" 9 | """oof"""
13 | print("foo") 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 PYI048.pyi:17:5: PYI048 Function body must contain exactly one statement
| |
15 | print("foo")
16 |
17 | def buzz(): # ERROR PYI048 17 | def buzz(): # ERROR PYI048
| ^^^^ PYI048 | ^^^^ PYI048
18 | print("fizz") 18 | print("fizz")