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

[pycodestyle] Do not trigger E3 rules on defs following a function/method with a dummy body #10704

Merged
merged 8 commits into from
Apr 15, 2024
64 changes: 64 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E30.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,39 @@ def test():
# end


# no error
class Foo:
"""Demo."""

@overload
def bar(self, x: int) -> int: ...
@overload
def bar(self, x: str) -> str: ...
def bar(self, x: int | str) -> int | str:
return x
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
# end


# no error
@overload
def foo(x: int) -> int: ...
@overload
def foo(x: str) -> str: ...
def foo(x: int | str) -> int | str:
if not isinstance(x, (int, str)):
raise TypeError
return x
# end


# no error
def foo(self, x: int) -> int: ...
def bar(self, x: str) -> str: ...
def baz(self, x: int | str) -> int | str:
return x
# end


# E301
class Class(object):

Expand Down Expand Up @@ -489,6 +522,20 @@ def cls_method(cls) -> None:
# end


# E301
class Foo:
"""Demo."""

@overload
def bar(self, x: int) -> int: ...
@overload
def bar(self, x: str) -> str:
...
def bar(self, x: int | str) -> int | str:
return x
# end


# E302
"""Main module."""
def fn():
Expand Down Expand Up @@ -580,6 +627,23 @@ def method2():
# end


# E302
class A:...
class B: ...
# end


# E302
@overload
def fn(a: int) -> int: ...
@overload
def fn(a: str) -> str: ...

def fn(a: int | str) -> int | str:
...
# end


# E303
def fn():
_ = None
Expand Down
30 changes: 26 additions & 4 deletions crates/ruff_linter/src/rules/pycodestyle/rules/blank_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,11 +635,26 @@ enum Follows {
Other,
Decorator,
Def,
/// A function whose body is a dummy (...), if the ellipsis is on the same line as the def.
DummyDef,
Import,
FromImport,
Docstring,
}

impl Follows {
// Allow a function/method to follow a function/method with a dummy body.
const fn follows_def_with_dummy_body(self) -> bool {
matches!(self, Follows::DummyDef)
}
}

impl Follows {
const fn is_any_def(self) -> bool {
matches!(self, Follows::Def | Follows::DummyDef)
}
}

impl Follows {
const fn is_any_import(self) -> bool {
matches!(self, Follows::Import | Follows::FromImport)
Expand Down Expand Up @@ -755,7 +770,11 @@ impl<'a> BlankLinesChecker<'a> {
if matches!(state.fn_status, Status::Outside) {
state.fn_status = Status::Inside(logical_line.indent_length);
}
state.follows = Follows::Def;
state.follows = if logical_line.last_token == TokenKind::Ellipsis {
Follows::DummyDef
} else {
Follows::Def
};
}
LogicalLineKind::Comment => {}
LogicalLineKind::Import => {
Expand Down Expand Up @@ -801,7 +820,8 @@ impl<'a> BlankLinesChecker<'a> {
// Only applies to methods.
&& matches!(line.kind, LogicalLineKind::Function | LogicalLineKind::Decorator)
// Allow groups of one-liners.
&& !(matches!(state.follows, Follows::Def) && !matches!(line.last_token, TokenKind::Colon))
&& !(state.follows.is_any_def() && line.last_token != TokenKind::Colon)
&& !state.follows.follows_def_with_dummy_body()
&& matches!(state.class_status, Status::Inside(_))
// The class/parent method's docstring can directly precede the def.
// Allow following a decorator (if there is an error it will be triggered on the first decorator).
Expand Down Expand Up @@ -852,7 +872,8 @@ impl<'a> BlankLinesChecker<'a> {
// Allow following a decorator (if there is an error it will be triggered on the first decorator).
&& !matches!(state.follows, Follows::Decorator)
// Allow groups of one-liners.
&& !(matches!(state.follows, Follows::Def) && !matches!(line.last_token, TokenKind::Colon))
&& !(state.follows.is_any_def() && line.last_token != TokenKind::Colon)
&& !(state.follows.follows_def_with_dummy_body() && line.preceding_blank_lines == 0)
// Only trigger on non-indented classes and functions (for example functions within an if are ignored)
&& line.indent_length == 0
// Only apply to functions or classes.
Expand Down Expand Up @@ -1018,7 +1039,8 @@ impl<'a> BlankLinesChecker<'a> {
// Do not trigger when the def/class follows an "indenting token" (if/while/etc...).
&& prev_indent_length.is_some_and(|prev_indent_length| prev_indent_length >= line.indent_length)
// Allow groups of one-liners.
&& !(matches!(state.follows, Follows::Def) && line.last_token != TokenKind::Colon)
&& !(state.follows.is_any_def() && line.last_token != TokenKind::Colon)
&& !state.follows.follows_def_with_dummy_body()
// Blank lines in stub files are only used for grouping. Don't enforce blank lines.
&& !self.source_type.is_stub()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,83 +1,101 @@
---
source: crates/ruff_linter/src/rules/pycodestyle/mod.rs
---
E30.py:453:5: E301 [*] Expected 1 blank line, found 0
E30.py:486:5: E301 [*] Expected 1 blank line, found 0
|
451 | def func1():
452 | pass
453 | def func2():
484 | def func1():
485 | pass
486 | def func2():
| ^^^ E301
454 | pass
455 | # end
487 | pass
488 | # end
|
= help: Add missing blank line

ℹ Safe fix
450 450 |
451 451 | def func1():
452 452 | pass
453 |+
453 454 | def func2():
454 455 | pass
455 456 | # end
483 483 |
484 484 | def func1():
485 485 | pass
486 |+
486 487 | def func2():
487 488 | pass
488 489 | # end

E30.py:464:5: E301 [*] Expected 1 blank line, found 0
E30.py:497:5: E301 [*] Expected 1 blank line, found 0
|
462 | pass
463 | # comment
464 | def fn2():
495 | pass
496 | # comment
497 | def fn2():
| ^^^ E301
465 | pass
466 | # end
498 | pass
499 | # end
|
= help: Add missing blank line

ℹ Safe fix
460 460 |
461 461 | def fn1():
462 462 | pass
463 |+
463 464 | # comment
464 465 | def fn2():
465 466 | pass
493 493 |
494 494 | def fn1():
495 495 | pass
496 |+
496 497 | # comment
497 498 | def fn2():
498 499 | pass

E30.py:474:5: E301 [*] Expected 1 blank line, found 0
E30.py:507:5: E301 [*] Expected 1 blank line, found 0
|
473 | columns = []
474 | @classmethod
506 | columns = []
507 | @classmethod
| ^ E301
475 | def cls_method(cls) -> None:
476 | pass
508 | def cls_method(cls) -> None:
509 | pass
|
= help: Add missing blank line

ℹ Safe fix
471 471 | """Class for minimal repo."""
472 472 |
473 473 | columns = []
474 |+
474 475 | @classmethod
475 476 | def cls_method(cls) -> None:
476 477 | pass
504 504 | """Class for minimal repo."""
505 505 |
506 506 | columns = []
507 |+
507 508 | @classmethod
508 509 | def cls_method(cls) -> None:
509 510 | pass

E30.py:486:5: E301 [*] Expected 1 blank line, found 0
E30.py:519:5: E301 [*] Expected 1 blank line, found 0
|
484 | def method(cls) -> None:
485 | pass
486 | @classmethod
517 | def method(cls) -> None:
518 | pass
519 | @classmethod
| ^ E301
487 | def cls_method(cls) -> None:
488 | pass
520 | def cls_method(cls) -> None:
521 | pass
|
= help: Add missing blank line

ℹ Safe fix
483 483 |
484 484 | def method(cls) -> None:
485 485 | pass
486 |+
486 487 | @classmethod
487 488 | def cls_method(cls) -> None:
488 489 | pass
516 516 |
517 517 | def method(cls) -> None:
518 518 | pass
519 |+
519 520 | @classmethod
520 521 | def cls_method(cls) -> None:
521 522 | pass

E30.py:534:5: E301 [*] Expected 1 blank line, found 0
|
532 | def bar(self, x: str) -> str:
533 | ...
534 | def bar(self, x: int | str) -> int | str:
| ^^^ E301
535 | return x
536 | # end
|
= help: Add missing blank line

ℹ Safe fix
531 531 | @overload
532 532 | def bar(self, x: str) -> str:
533 533 | ...
534 |+
534 535 | def bar(self, x: int | str) -> int | str:
535 536 | return x
536 537 | # end