Skip to content

Commit

Permalink
Improve compatibility between multi-statement PYI rules (#7024)
Browse files Browse the repository at this point in the history
## 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 #7021.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Aug 31, 2023
1 parent f7dca3d commit 51d69b4
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 64 deletions.
12 changes: 8 additions & 4 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion crates/ruff/resources/test/fixtures/flake8_pyi/PYI010.pyi
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -10,3 +10,6 @@ def foo2():

def bizz():
x = 123 # ERROR PYI010

def foo3():
pass # OK, pass is handled by another rule
18 changes: 13 additions & 5 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.py
Original file line number Diff line number Diff line change
@@ -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")
18 changes: 9 additions & 9 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI048.pyi
Original file line number Diff line number Diff line change
@@ -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")
31 changes: 24 additions & 7 deletions crates/ruff/src/rules/flake8_pyi/rules/non_empty_stub_body.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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);
Expand Down
21 changes: 10 additions & 11 deletions crates/ruff/src/rules/flake8_pyi/rules/pass_statement_stub_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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(),
));
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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():
Expand All @@ -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


Original file line number Diff line number Diff line change
@@ -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")
Expand Down

0 comments on commit 51d69b4

Please sign in to comment.