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

Fix instability in trailing clause body comments #7556

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Sep 20, 2023

Summary

When we format the trailing comments on a clause body, we check if there are any newlines after the last statement; if not, we insert one.

This logic didn't take into account that the last statement could itself have trailing comments, as in:

if True:
    pass

    # comment
else:
    pass

We were thus inserting a newline after the comment, like:

if True:
    pass

    # comment

else:
    pass

In the context of function definitions, this led to an instability, since we insert a newline after a function, which would in turn lead to the bug above appearing in the second formatting pass.

Closes #7465.

Test Plan

cargo test

Small improvement in transformers, but no regressions.

Before:

project similarity index total files changed files
cpython 0.76083 1789 1631
django 0.99983 2760 36
transformers 0.99956 2587 404
twine 1.00000 33 0
typeshed 0.99983 3496 18
warehouse 0.99967 648 15
zulip 0.99972 1437 21

After:

project similarity index total files changed files
cpython 0.76083 1789 1631
django 0.99983 2760 36
transformers 0.99957 2587 402
twine 1.00000 33 0
typeshed 0.99983 3496 18
warehouse 0.99967 648 15
zulip 0.99972 1437 21

@charliermarsh charliermarsh added bug Something isn't working formatter Related to the formatter labels Sep 20, 2023
@charliermarsh charliermarsh merged commit 124d95d into main Sep 21, 2023
16 checks passed
@charliermarsh charliermarsh deleted the charlie/comment branch September 21, 2023 13:32
charliermarsh added a commit that referenced this pull request Sep 22, 2023
…rts (#7607)

## Summary

Given:

```python
# -*- coding: utf-8 -*-
import random

# Defaults for arguments are defined here
# args.threshold = None;


logger = logging.getLogger("FastProject")
```

We want to count the number of newlines after `import random`, to ensure
that there's _at least one_, but up to two.

Previously, we used the end range of the statement (then skipped
trivia); instead, we need to use the end of the _last comment_. This is
similar to #7556.

Closes #7604.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatter instability: Leading clause header comments with preceding function
3 participants