Skip to content

Commit

Permalink
[pycodestyle] Do not trigger E3 rules on defs following a functio…
Browse files Browse the repository at this point in the history
…n/method with a dummy body (#10704)
  • Loading branch information
hoel-bagard committed Apr 15, 2024
1 parent cbd5001 commit 670d66f
Show file tree
Hide file tree
Showing 8 changed files with 748 additions and 612 deletions.
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
# 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

0 comments on commit 670d66f

Please sign in to comment.