Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve compatibility between multi-statement PYI rules #7024

Merged
merged 1 commit into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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