String processing: wrap concatenated strings in parens in all cases

[yilei]

Exending the dicussions in #3260 and #2553, I'd like to once more try to bring up the topic of how black should format concatenated strings.

After a lot of reading and experimenting, I'm now in favor of always wrapping concatenated strings in parens, i.e. extending the cases from #3159 to all. Examples:

# Unformatted:
function_call(
    " lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua Ut enim ad minim",
    " veniam quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo",
)
some_list = [
    " lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua Ut enim ad minim",
    " veniam quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo",
]

# v22.8.0 --preview:
function_call(
    " lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor"
    " incididunt ut labore et dolore magna aliqua Ut enim ad minim",
    " veniam quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo",
)
some_list = [
    (
        " lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor"
        " incididunt ut labore et dolore magna aliqua Ut enim ad minim"
    ),
    " veniam quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo",
]

# Proposed:
function_call(
    (
        " lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor"
        " incididunt ut labore et dolore magna aliqua Ut enim ad minim"
    ),
    " veniam quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo",
)
some_list = [
    (
        " lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor"
        " incididunt ut labore et dolore magna aliqua Ut enim ad minim"
    ),
    " veniam quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo",
]

This in turn makes #3260 and #2553 redundant/obsolete.

Pros

1. Code is more readable since the extra indentation makes the scope more clear.

2. Avoids common coding errors when commas are accidentally left-out or missed in function calls, and especially list/tuple/set literals.

3. It's consistent no matter where the string appears. --preview (even without Add parens around implicit string concatenations where increases readability #3162) already does this in a few places:

   def function():
       value = "a very long string a very long string a very long string a very long string a very long string a very long string"
       dct = {"key": "a very long string a very long string a very long string a very long string a very long string a very long string"}
       return (
           "a very long string a very long string a very long string a very long string a"
           " very long string a very long string"
       )
   
   # output:
   def function():
       value = (
           "a very long string a very long string a very long string a very long string a"
           " very long string a very long string"
       )
       dct = {
           "key": (
               "a very long string a very long string a very long string a very long"
               " string a very long string a very long string"
           )
       }
       return (
           "a very long string a very long string a very long string a very long string a"
           " very long string a very long string"
       )

4. It's much easier to explain Black's behavior: if a string (after merging implicitly or explicitly concated parts)
   exceeds the line length, it's wrapped in parens then split. (FWIW, this is the point that finally convinces me we should adopt this approach.)

Cons

1. It can introduce decent amount of diffs

How about explicit concatenation without parens?

It introduces somewhat less amount of diffs, but the formatting is less readable some cases:

some_list = [
    " lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor"
    + " incididunt ut labore et dolore magna aliqua Ut enim ad minim",
    " veniam quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo",
]

## Modified example from https://github.com/psf/black/blob/main/tests/data/preview/long_strings__regression.py
class A:
    def append(self):
        if True:
            xxxx.xxxxxxx.xxxxx(
                "xxxxxxxxxx xxxx xx xxxxxx xxxxxxxxxx xxxx xx xxxxxx xxxxxxxxxx xxxx xx"
                + " xxxxxx (%x) xx %x xxxx xx xxx %x.xx"
                % (len(self) + 1, xxxx.xxxxxxxxxx, xxxx.xxxxxxxxxx)
                + " %.3f (%s) to %.3f (%s).\n"
                % (
                    xxxx.xxxxxxxxx,
                    xxxx.xxxxxxxxxxxxxx(xxxx.xxxxxxxxx),
                    x,
                    xxxx.xxxxxxxxxxxxxx(xx),
                )
            )

How about explicit concatenation with parens?

Since it's already wrapped in parens, the explicit concatenation is redundant and doesn't
increase readability (and also occupies two extra columns).

How about keep organising parens added by users?

Since Black removes parens if the string fits on a single line, keeping organising parens
has the similar issue as the magic-trailing-comma: #2237 (comment).

cc @felix-hilden @JelleZijlstra @ichard26

[felix-hilden]

Thank you for writing a fleshed out proposal! It's a Yay from me 👍

[JelleZijlstra]

Thanks for the writeup! I agree there's definite benefits to wrapping these strings in parentheses. My only reservation is that sometimes the parentheses feel redundant and verbose. For example, I don't love the change in https://github.com/psf/black/pull/3307/files#diff-d23a6917d7262ca9815093bffb7148299e516575251959ed9b405b648e76aa29. On the other hand, the change in https://github.com/psf/black/pull/3307/files#diff-116e83ac321f88d78cac1d0b7058473a5853b7753b37e121e3674e282038f763 is definitely an improvement: the old code is not at all clear that there are two string arguments.

I think on balance it's more important to prevent bugs from implicitly concatenated strings, so we should land the change. I'd like to hear opinions from other maintainers though.

[yilei]

Are we still waiting for other maintainers' opinions? Who should I tag here?

[felix-hilden]

Apart from Rich maybe @cooperlees, but we should be good unless you want to be extra sure to not waste effort 😄

[yilei]

@felix-hilden this is implemented in #3307, please take a look when you get a chance. Thanks 🙏🏼