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

Line break not occuring on type annotations whereas black does #11791

Open
jonathan-hill-visasq opened this issue Jun 7, 2024 · 3 comments
Open
Labels
accepted Ready for implementation breaking Breaking API change formatter Related to the formatter

Comments

@jonathan-hill-visasq
Copy link

Issue Description

I've noticed a deviation from the black formatter, which I can't find accounted for in either the known deviations page, or the issue tracker.

It occurs on assignments where

  • there is a type annotation
  • the maximum line length is exceeded in the middle of the type annotation.

Black (24.4.2), will parenthesize the type annotation and break the line, ensuring no lines exceed the line length.
Ruff (0.4.7) however keeps the assignment all one line. Even if the file is formatted by black, if ruff format is run after, it removes the parenthesis and puts it all back on one (too long) line.

Black started parenthesizing and line breaking on type annotations as of version 24.1.0, specifically PR 3899.

Example

Example (default line-length of 88) - script.py(extract):

# input
a_member_with_long_long_name: TypeNameWhichIsLongEnoughToCauseLineLengthToBeOverLimit = 12345

# ruff-format output
a_member_with_long_long_name: TypeNameWhichIsLongEnoughToCauseLineLengthToBeOverLimit = 12345

# black output
a_member_with_long_long_name: (
    TypeNameWhichIsLongEnoughToCauseLineLengthToBeOverLimit
) = 12345

commands used:

  • black script.py ruff-format script.py

Related Links/Discussions

The original issue that led to the black change (v24.1.0, PR#3899) , only seems to specify type compound type annotations- ie int | str, rather than int or str. When I try a compound type annotation with ruff, it does indeed parenthesize and line-break the same as black.
So the deviation only occurs when the type annotation is comprised of a single type only.

Is this deviation intentional? Given that the original black issue only seems to reference compound type annotations, I guess it could be argued that the ruff output is now "correct" - but personally I would strongly prefer having the annotation parenthesized and line-broken

@MichaReiser MichaReiser added the formatter Related to the formatter label Jun 7, 2024
@MichaReiser
Copy link
Member

MichaReiser commented Jun 7, 2024

Hi, thanks for the very detailed write up and linking to all relevant issues!

A few more observations to the deviation:

For the given example, the variable name and the type annotation exceed the line length limit. This is even before the value begins.

Both Ruff and Black format the assignment the same if the variable name and type annotation fit into the line length

a_member_with_long_long_name: TypeNameWhichIsLongEnoughToCauseLineLengthToBeOdddd = (
    12345
)

Both Ruff and Black collapse the value when parenthesizing it doesn't help fitting it in the configured line length

a_member_with_long_long_name: TypeNameWhichIsLongEnoughToCauseLineLengthToBeOddd = 12345

Arguably Black's formatting is more consistent. Let's say we start with

a_member_with_long_long_name: TypeNameWhichIsLongEnoughToCauseLineLengthToBeOddddddddddddddd 

Ruff and Black both parenthesize the type annotation because it exceeds the configured line length

a_member_with_long_long_name: (
    TypeNameWhichIsLongEnoughToCauseLineLengthToBeOddddddddddddddd
)

Black keeps the parentheses when we add a value, but ruff removes them, leading to a larger diff than necessary. That's why I think should fix this, but I don't think we can before the next stable style.

I'm not sure how easy this fix is, considering that the assignment expression is already fairly complicated and Black has a lot of special casing. For example, Black formats

a_member_with_long_long_name: TypeNameWhichIsLongEnoughToCauseLineLengthToBeOddddddddddddddd # comment

to

a_member_with_long_long_name: (
    TypeNameWhichIsLongEnoughToCauseLineLengthToBeOddddddddddddddd  # comment
)

which Ruff supports too. But I guess this behavior is not relevant when there's a value?

} else {
annotation
.format()
.with_options(Parentheses::Never)
.fmt(f)?;
}

My simple fix of replacing these lines with

 else {
      optional_parentheses(&annotation.format().with_options(Parentheses::Never))
          .fmt(f)?;
  }

Doesn't work because the annotation now splits before the value. So I fear we would need to integrate this into left_to_right somehow.

@MichaReiser MichaReiser added breaking Breaking API change accepted Ready for implementation labels Jun 7, 2024
@jonathan-hill-visasq
Copy link
Author

@MichaReiser Thanks for the quick response!
Unfortunately I'm not familiar with the ruff code base, or even rust myself, so trying to fix this would be too big of a commitment for me to take on now... but it's good to know it is a genuine deviation.

@MichaReiser
Copy link
Member

@jonathan-hill-visasq yeah sorry. It wasn't my intention to imply that you should be fixing it. I mainly added the notes to know the complexity and where to start if I get back to the issue (or someone else who's interested). But thanks for considering it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation breaking Breaking API change formatter Related to the formatter
Projects
None yet
Development

No branches or pull requests

2 participants