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

Enforce a blank line after a nested class in stubs #3564

Merged
merged 4 commits into from Mar 18, 2023

Conversation

WMOkiishi
Copy link
Contributor

Description

Resolves #2783

The issue was partially resolved by #2820, but only for cases where the inner class body ends with a definition (class or def) and is followed by another definition. This change enforces a blank line after any nested class with a body (other than just ...) in stubs, particularly those ending with or followed by an annotation for an attribute.

I had to make a slight change to EmptyLineTracker.previous_defs in order to not put a blank line after the indented block of an if statement or function def: while typeshed doesn't include anything in function bodies, some of the stubs I'm maintaining include a docstring, and I wouldn't want a blank line added after each method that has one.

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@github-actions
Copy link

github-actions bot commented Feb 12, 2023

diff-shades results comparing this PR (22f6e9d) to main (a3e8247). The full diff is available in the logs under the "Generate HTML diff report" step.

╭─────────────────────── Summary ────────────────────────╮
│ 1 projects & 35 files changed / 43 changes [+43/-0]    │
│                                                        │
│ ... out of 2 418 510 lines, 11 507 files & 23 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

What is this? | Workflow run | diff-shades documentation

@AlexWaygood
Copy link
Contributor

I'm not an expert on black internals, but the typeshed changes flagged by diff-shades look like a definite improvement to me!

@JelleZijlstra JelleZijlstra self-assigned this Mar 18, 2023
@JelleZijlstra JelleZijlstra merged commit d7a28dd into psf:main Mar 18, 2023
44 checks passed
@WMOkiishi WMOkiishi deleted the stub-nested-classes branch March 18, 2023 23:59
yilei added a commit to yilei/black that referenced this pull request Mar 19, 2023
JelleZijlstra pushed a commit that referenced this pull request Mar 19, 2023
konstin added a commit to konstin/black that referenced this pull request Sep 8, 2023
The idea behind this change is that we stop looking into previous body to determine if there should be a blank before a function or class definition.

Input:

```python
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: ...
```

Stable style
```python
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: ...
```

In the stable formatting, we have a blank line sometimes, not depending on the previous statement on the same level, but on the last (potentially nested) statement in the previous body.

psf#2783/psf#3564 fixes this for classes in preview style:

```python
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: ...
```

This PR additionally fixes this for function definitions:

```python
if sys.version_info > (3, 7):
    if sys.platform == "win32":
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...
    if sys.platform != "win32":
        def function_definition(self): ...
        assignment = 1

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

if sys.version_info > (3, 8):
    if sys.platform == "win32":
        assignment = 1
        def function_definition(self): ...

    class F1: ...
    if sys.platform != "win32":
        def function_definition(self): ...
        assignment = 1

    class F2: ...
```

You can see the effect of this change on typeshed in https://github.com/konstin/typeshed/pull/1/files. As baseline, the preview mode changes without this PR are at konstin/typeshed#2.
JelleZijlstra added a commit that referenced this pull request Sep 9, 2023
The idea behind this change is that we stop looking into previous body to determine if there should be a blank before a function or class definition.

Input:

```python
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: ...
```

Stable style
```python
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: ...
```

In the stable formatting, we have a blank line sometimes, not depending on the previous statement on the same level, but on the last (potentially nested) statement in the previous body.

#2783/#3564 fixes this for classes in preview style:

```python
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: ...
```

This PR additionally fixes this for function definitions:

```python
if sys.version_info > (3, 7):
    if sys.platform == "win32":
        assignment = 1
        def function_definition(self): ...

    def f1(self) -> str: ...
    if sys.platform != "win32":
        def function_definition(self): ...
        assignment = 1

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

if sys.version_info > (3, 8):
    if sys.platform == "win32":
        assignment = 1
        def function_definition(self): ...

    class F1: ...
    if sys.platform != "win32":
        def function_definition(self): ...
        assignment = 1

    class F2: ...
```

You can see the effect of this change on typeshed in https://github.com/konstin/typeshed/pull/1/files. As baseline, the preview mode changes without this PR are at konstin/typeshed#2.

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Put blank lines between nested classes in typing stubs
3 participants