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

More concise formatting for dummy implementations #3796

Merged
merged 9 commits into from Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Expand Up @@ -14,6 +14,8 @@

<!-- Changes that affect Black's preview style -->

- More concise formatting for dummy implementations (#3796)

### Configuration

<!-- Changes to how Black can be configured -->
Expand Down
10 changes: 7 additions & 3 deletions src/black/linegen.py
Expand Up @@ -281,7 +281,9 @@ def visit_match_case(self, node: Node) -> Iterator[Line]:

def visit_suite(self, node: Node) -> Iterator[Line]:
"""Visit a suite."""
if self.mode.is_pyi and is_stub_suite(node):
if (
self.mode.is_pyi or Preview.dummy_implementations in self.mode
) and is_stub_suite(node):
yield from self.visit(node.children[2])
else:
yield from self.visit_default(node)
Expand All @@ -296,7 +298,9 @@ def visit_simple_stmt(self, node: Node) -> Iterator[Line]:

is_suite_like = node.parent and node.parent.type in STATEMENT
if is_suite_like:
if self.mode.is_pyi and is_stub_body(node):
if (
self.mode.is_pyi or Preview.dummy_implementations in self.mode
) and is_stub_body(node):
yield from self.visit_default(node)
else:
yield from self.line(+1)
Expand All @@ -305,7 +309,7 @@ def visit_simple_stmt(self, node: Node) -> Iterator[Line]:

else:
if (
not self.mode.is_pyi
not (self.mode.is_pyi or Preview.dummy_implementations in self.mode)
or not node.parent
or not is_stub_suite(node.parent)
):
Expand Down
26 changes: 22 additions & 4 deletions src/black/lines.py
Expand Up @@ -165,6 +165,13 @@ def is_def(self) -> bool:
and second_leaf.value == "def"
)

@property
def is_stub_def(self) -> bool:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mirrors is_stub_class from a couple lines above

"""Is this line a function definition with a body consisting only of "..."?"""
return self.is_def and self.leaves[-3:] == [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this also match something like def f(x): y = ...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is true (and true for the is is_stub_class heuristic above as well). I've made the heuristic slightly better by including the colon. I didn't see anything adverse in the diff shades output.

Also note some of the pyi code we now use in linegen.py will trigger in some cases involving other statements, although it's not unreasonable when it happens.

Leaf(token.DOT, ".") for _ in range(3)
]

@property
def is_class_paren_empty(self) -> bool:
"""Is this a class with no base classes but using parentheses?
Expand Down Expand Up @@ -578,6 +585,8 @@ def _maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]:
first_leaf.prefix = ""
else:
before = 0

user_hint_before = before
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
depth = current_line.depth

previous_def = None
Expand Down Expand Up @@ -624,7 +633,9 @@ def _maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]:
before = 2

if current_line.is_decorator or current_line.is_def or current_line.is_class:
return self._maybe_empty_lines_for_class_or_def(current_line, before)
return self._maybe_empty_lines_for_class_or_def(
current_line, before, user_hint_before
)

if (
self.previous_line
Expand All @@ -648,8 +659,8 @@ def _maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]:
return 0, 0
return before, 0

def _maybe_empty_lines_for_class_or_def(
self, current_line: Line, before: int
def _maybe_empty_lines_for_class_or_def( # noqa: C901
self, current_line: Line, before: int, user_hint_before: int
) -> Tuple[int, int]:
if not current_line.is_decorator:
self.previous_defs.append(current_line)
Expand Down Expand Up @@ -714,7 +725,14 @@ def _maybe_empty_lines_for_class_or_def(
else:
newlines = 0
else:
newlines = 1 if current_line.depth else 2
if (
Preview.dummy_implementations in self.mode
and self.previous_line.is_stub_def
and (current_line.is_stub_def or current_line.is_decorator)
):
newlines = user_hint_before
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perhaps controversial, but it allows users to group things as they want (e.g. in the test case, dummy and other are grouped separately) and solves some lookahead problems when dealing with decorators

else:
newlines = 1 if current_line.depth else 2
if comment_to_add_newlines is not None:
previous_block = comment_to_add_newlines.previous_block
if previous_block is not None:
Expand Down
1 change: 1 addition & 0 deletions src/black/mode.py
Expand Up @@ -182,6 +182,7 @@ class Preview(Enum):
skip_magic_trailing_comma_in_subscript = auto()
wrap_long_dict_values_in_parens = auto()
wrap_multiple_context_managers_in_parens = auto()
dummy_implementations = auto()


class Deprecated(UserWarning):
Expand Down
4 changes: 2 additions & 2 deletions tests/data/miscellaneous/force_py36.py
@@ -1,6 +1,6 @@
# The input source must not contain any Py36-specific syntax (e.g. argument type
# annotations, trailing comma after *rest) or this test becomes invalid.
def long_function_name(argument_one, argument_two, argument_three, argument_four, argument_five, argument_six, *rest): ...
def long_function_name(argument_one, argument_two, argument_three, argument_four, argument_five, argument_six, *rest): pass
# output
# The input source must not contain any Py36-specific syntax (e.g. argument type
# annotations, trailing comma after *rest) or this test becomes invalid.
Expand All @@ -13,4 +13,4 @@ def long_function_name(
argument_six,
*rest,
):
...
pass
3 changes: 1 addition & 2 deletions tests/data/preview/comments7.py
Expand Up @@ -278,8 +278,7 @@ class C:
)
def test_fails_invalid_post_data(
self, pyramid_config, db_request, post_data, message
):
...
): ...


square = Square(4) # type: Optional[Square]
71 changes: 71 additions & 0 deletions tests/data/preview/dummy_implementations.py
@@ -0,0 +1,71 @@
from typing import NoReturn, Protocol, Union, overload


def dummy(a): ...
def other(b): ...


@overload
def a(arg: int) -> int: ...
@overload
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
def a(arg: str) -> str: ...
@overload
def a(arg: object) -> NoReturn: ...
def a(arg: Union[int, str, object]) -> Union[int, str]:
if not isinstance(arg, (int, str)):
raise TypeError
return arg

class Proto(Protocol):
def foo(self, a: int) -> int:
...

def bar(self, b: str) -> str: ...
def baz(self, c: bytes) -> str:
...


def dummy_two():
...
@dummy
def dummy_three():
...

def dummy_four():
...

# output

from typing import NoReturn, Protocol, Union, overload


def dummy(a): ...
def other(b): ...


@overload
def a(arg: int) -> int: ...
@overload
def a(arg: str) -> str: ...
@overload
def a(arg: object) -> NoReturn: ...


def a(arg: Union[int, str, object]) -> Union[int, str]:
if not isinstance(arg, (int, str)):
raise TypeError
return arg


class Proto(Protocol):
def foo(self, a: int) -> int: ...

def bar(self, b: str) -> str: ...
def baz(self, c: bytes) -> str: ...


def dummy_two(): ...
@dummy
def dummy_three(): ...

def dummy_four(): ...