Skip to content

Comments

Fix placement of inline parameter comments#13379

Merged
MichaReiser merged 3 commits intomainfrom
micha/fix-parameter-comment-placement
Sep 18, 2024
Merged

Fix placement of inline parameter comments#13379
MichaReiser merged 3 commits intomainfrom
micha/fix-parameter-comment-placement

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Sep 17, 2024 •
edited
Loading

Summary

Fixes #13369

Before: The formatter made all parameter comments leading comments unless the parameter had type annotations.
Now: Only make comments leading comments if they are between * and ** and args/kwargs.

This bug fix doesn't require a new style guide because it doesn't change the placement of comments that were incorrectly made leading comments.
It only changes the formatting for unformatted code.

Test Plan

Added tests, verified the black snapshot.

Black

Black preserves the placement of all comments.

def args_many_comments_with_type_annotation(
    # before
    *
    # between * and name
    args  # trailing args
    # before colon
    :  # after colon
    # before type
    int,  # trailing type
    # after type
):
    pass

I don't think it's worth the complexity, considering that we haven't received any user reports that the comments were moved, and comments between * and the name and the name and : seem undesirable.

@MichaReiser MichaReiser added bug Something isn't working formatter Related to the formatter labels Sep 17, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Sep 17, 2024 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser
Copy link
Member Author

MichaReiser commented Sep 17, 2024

There are a few more parameter cases that we aren't handling correctly. Putting back in draft

@MichaReiser MichaReiser marked this pull request as draft September 17, 2024 10:04
@MichaReiser
Copy link
Member Author

MichaReiser commented Sep 17, 2024

CC: @carljm because you once mentioned that you're interested in how our formatter works.

@MichaReiser MichaReiser force-pushed the micha/fix-parameter-comment-placement branch from 869807f to 3a07d72 Compare September 17, 2024 10:39
@MichaReiser MichaReiser marked this pull request as ready for review September 17, 2024 10:40
charliermarsh
charliermarsh approved these changes Sep 17, 2024
View reviewed changes
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Wow been a while here. Looks reasonable though.

@MichaReiser
Copy link
Member Author

MichaReiser commented Sep 17, 2024

Wow been a while here. Looks reasonable though.

Haha. That was my reaction too when I started working on this 🙈

@MichaReiser MichaReiser merged commit 6ac61d7 into main Sep 18, 2024
@MichaReiser MichaReiser deleted the micha/fix-parameter-comment-placement branch September 18, 2024 06:26
@BrewTestBot BrewTestBot mentioned this pull request Sep 20, 2024
Merged
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 7, 2024
@0323pin
devel/py-ruff: update to 0.6.9
3842c32 
## 0.6.9

### Preview features

- Fix codeblock dynamic line length calculation for indented docstring examples ([#13523](astral-sh/ruff#13523))
- \[`refurb`\] Mark `FURB118` fix as unsafe ([#13613](astral-sh/ruff#13613))

### Rule changes

- \[`pydocstyle`\] Don't raise `D208` when last line is non-empty ([#13372](astral-sh/ruff#13372))
- \[`pylint`\] Preserve trivia (i.e. comments) in `PLR5501` autofix ([#13573](astral-sh/ruff#13573))

### Configuration

- \[`pyflakes`\] Add `allow-unused-imports` setting for `unused-import` rule (`F401`) ([#13601](astral-sh/ruff#13601))

### Bug fixes

- Support ruff discovery in pip build environments ([#13591](astral-sh/ruff#13591))
- \[`flake8-bugbear`\] Avoid short circuiting `B017` for multiple context managers ([#13609](astral-sh/ruff#13609))
- \[`pylint`\] Do not offer an invalid fix for `PLR1716` when the comparisons contain parenthesis ([#13527](astral-sh/ruff#13527))
- \[`pyupgrade`\] Fix `UP043` to apply to `collections.abc.Generator` and `collections.abc.AsyncGenerator` ([#13611](astral-sh/ruff#13611))
- \[`refurb`\] Fix handling of slices in tuples for `FURB118`, e.g., `x[:, 1]` ([#13518](astral-sh/ruff#13518))

### Documentation

- Update GitHub Action link to `astral-sh/ruff-action` ([#13551](astral-sh/ruff#13551))

## 0.6.8

### Preview features

- Remove unnecessary parentheses around `match case` clauses ([#13510](astral-sh/ruff#13510))
- Parenthesize overlong `if` guards in `match..case` clauses ([#13513](astral-sh/ruff#13513))
- Detect basic wildcard imports in `ruff analyze graph` ([#13486](astral-sh/ruff#13486))
- \[`pylint`\] Implement `boolean-chained-comparison` (`R1716`) ([#13435](astral-sh/ruff#13435))

### Rule changes

- \[`lake8-simplify`\] Detect `SIM910` when using variadic keyword arguments, i.e., `**kwargs` ([#13503](astral-sh/ruff#13503))
- \[`pyupgrade`\] Avoid false negatives with non-reference shadowed bindings of loop variables (`UP028`) ([#13504](astral-sh/ruff#13504))

### Bug fixes

- Detect tuples bound to variadic positional arguments i.e. `*args` ([#13512](astral-sh/ruff#13512))
- Exit gracefully on broken pipe errors ([#13485](astral-sh/ruff#13485))
- Avoid panic when analyze graph hits broken pipe ([#13484](astral-sh/ruff#13484))

### Performance

- Reuse `BTreeSets` in module resolver ([#13440](astral-sh/ruff#13440))
- Skip traversal for non-compound statements ([#13441](astral-sh/ruff#13441))

## 0.6.7

### Preview features

- Add Python version support to ruff analyze CLI ([#13426](astral-sh/ruff#13426))
- Add `exclude` support to `ruff analyze` ([#13425](astral-sh/ruff#13425))
- Fix parentheses around return type annotations ([#13381](astral-sh/ruff#13381))

### Rule changes

- \[`pycodestyle`\] Fix: Don't autofix if the first line ends in a question mark? (D400) ([#13399](astral-sh/ruff#13399))

### Bug fixes

- Respect `lint.exclude` in ruff check `--add-noqa` ([#13427](astral-sh/ruff#13427))

### Performance

- Avoid tracking module resolver files in Salsa ([#13437](astral-sh/ruff#13437))
- Use `forget` for module resolver database ([#13438](astral-sh/ruff#13438))

## 0.6.6

### Preview features

- \[`refurb`\] Skip `slice-to-remove-prefix-or-suffix` (`FURB188`) when non-trivial slice steps are present ([#13405](astral-sh/ruff#13405))
- Add a subcommand to generate dependency graphs ([#13402](astral-sh/ruff#13402))

### Formatter

- Fix placement of inline parameter comments ([#13379](astral-sh/ruff#13379))

### Server

- Fix off-by one error in the `LineIndex::offset` calculation ([#13407](astral-sh/ruff#13407))

### Bug fixes

- \[`fastapi`\] Respect FastAPI aliases in route definitions ([#13394](astral-sh/ruff#13394))
- \[`pydocstyle`\] Respect word boundaries when detecting function signature in docs ([#13388](astral-sh/ruff#13388))

### Documentation

- Add backlinks to rule overview linter ([#13368](astral-sh/ruff#13368))
- Fix documentation for editor vim plugin ALE ([#13348](astral-sh/ruff#13348))
- Fix rendering of `FURB188` docs ([#13406](astral-sh/ruff#13406))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees

No one assigned

Labels

bug Something isn't working formatter Related to the formatter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Trailing parenthesized function return type comment becomes leading comment

2 participants

@MichaReiser @charliermarsh