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

Don't inspect the previous body for blank line rules in stub files #3861

Closed
konstin opened this issue Sep 6, 2023 · 1 comment · Fixed by #3862
Closed

Don't inspect the previous body for blank line rules in stub files #3861

konstin opened this issue Sep 6, 2023 · 1 comment · Fixed by #3862
Labels
T: style What do we want Blackened code to look like?

Comments

@konstin
Copy link
Contributor

konstin commented Sep 6, 2023

#2736 added a rule that preserves a blank line between attributes and methods. The implementation looks at the previous line instead of the previous statement, so it depends on the last statement in the (potentially arbitrarily nested) body if black inserts a blank line or not (code). #2783/#3564 fixes this for classes by always inserting a blank line, but this does not apply e.g. to function definitions. This affects e.g. typeshed, which uses if sys.version_info > (3, x): in stub files a lot.

I propose to change this to look at the type of the previous statement instead of the previous line.

Preview style

import sys

if sys.version_info > (3, 7):
    class Nested1:
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...

    class Nested2:
        def function_definition(self): ...
        assignment = 1

    def f2(self) -> str: ...

if sys.version_info > (3, 7):
    def nested1():
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...
    def nested2():
        def function_definition(self): ...
        assignment = 1
    def f2(self) -> str: ...

Desired style

(or a different set of blank line rules that don't inspect the previous function body anymore)

import sys

if sys.version_info > (3, 7):
    class Nested1:
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...

    class Nested2:
        def function_definition(self): ...
        assignment = 1

    def f2(self) -> str: ...

if sys.version_info > (3, 7):
    def nested1():
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...
    def nested2():
        def function_definition(self): ...
        assignment = 1

    def f2(self) -> str: ...
@konstin konstin added the T: style What do we want Blackened code to look like? label Sep 6, 2023
@hauntsaninja
Copy link
Collaborator

IIUC it doesn't usually matter because there's rarely any statements inside functions in stubs. Interested in opening a PR? :-)

konstin added a commit to astral-sh/ruff that referenced this issue Sep 11, 2023
## Summary

Fix all but one empty line differences with the black preview style in
typeshed. The remaining differences are breaking with type comments and
trailing commas in function definitions.

I compared the empty line differences with the preview mode of black
since stable has some oddities that would have been hard to replicate
(psf/black#3861). Additionally, it assumes the
style proposed in psf/black#3862.

An edge case that also surfaced with typeshed are newline before
trailing module comments.

**main**

| project | similarity index | total files | changed files |

|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| django | 0.99966 | 2760 | 58 |
| transformers | 0.99930 | 2587 | 447 |
| twine | 1.00000 | 33 | 0 |
| **typeshed** | 0.99978 | 3496 | **2173** |
| warehouse | 0.99825 | 648 | 22 |
| zulip | 0.99950 | 1437 | 27 |

**PR**
| project | similarity index | total files | changed files |

|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| django | 0.99966 | 2760 | 58 |
| transformers | 0.99930 | 2587 | 447 |
| twine | 1.00000 | 33 | 0 |
| **typeshed** | 0.99983 | 3496 | **18** |
| warehouse | 0.99825 | 648 | 22 |
| zulip | 0.99950 | 1437 | 27 |


Closes #6723

## Test Plan

The main driver was the typeshed diff. I added new test cases for all
kinds of possible empty line combinations in stub files, test cases for
newlines before trailing module comments.

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: style What do we want Blackened code to look like?
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants