Skip to content

Commit

Permalink
Fix merging implicit multiline strings that have inline comments (#3956)
Browse files Browse the repository at this point in the history
* Fix test behaviour

* Add new test cases

* Skip merging strings that have inline comments

* Don't merge lines with multiline strings with inline comments

* Changelog entry

* Document implicit multiline string merging rules

* Fix PR number
  • Loading branch information
henriholopainen committed Oct 20, 2023
1 parent 9edba85 commit 882d879
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 5 deletions.
2 changes: 1 addition & 1 deletion CHANGES.md
Expand Up @@ -12,7 +12,7 @@

### Preview style

<!-- Changes that affect Black's preview style -->
- Fix merging implicit multiline strings that have inline comments (#3956)

### Configuration

Expand Down
64 changes: 64 additions & 0 deletions docs/the_black_code_style/future_style.md
Expand Up @@ -160,3 +160,67 @@ MULTILINE = """
foobar
""".replace("\n", "")
```

Implicit multiline strings are special, because they can have inline comments. Strings
without comments are merged, for example

```python
s = (
"An "
"implicit "
"multiline "
"string"
)
```

becomes

```python
s = "An implicit multiline string"
```

A comment on any line of the string (or between two string lines) will block the
merging, so

```python
s = (
"An " # Important comment concerning just this line
"implicit "
"multiline "
"string"
)
```

and

```python
s = (
"An "
"implicit "
# Comment in between
"multiline "
"string"
)
```

will not be merged. Having the comment after or before the string lines (but still
inside the parens) will merge the string. For example

```python
s = ( # Top comment
"An "
"implicit "
"multiline "
"string"
# Bottom comment
)
```

becomes

```python
s = ( # Top comment
"An implicit multiline string"
# Bottom comment
)
```
1 change: 1 addition & 0 deletions src/black/linegen.py
Expand Up @@ -587,6 +587,7 @@ def transform_line(
or line.contains_unsplittable_type_ignore()
)
and not (line.inside_brackets and line.contains_standalone_comments())
and not line.contains_implicit_multiline_string_with_comments()
):
# Only apply basic string preprocessing, since lines shouldn't be split here.
if Preview.string_processing in mode:
Expand Down
15 changes: 15 additions & 0 deletions src/black/lines.py
Expand Up @@ -239,6 +239,21 @@ def contains_standalone_comments(self, depth_limit: int = sys.maxsize) -> bool:

return False

def contains_implicit_multiline_string_with_comments(self) -> bool:
"""Chck if we have an implicit multiline string with comments on the line"""
for leaf_type, leaf_group_iterator in itertools.groupby(
self.leaves, lambda leaf: leaf.type
):
if leaf_type != token.STRING:
continue
leaf_list = list(leaf_group_iterator)
if len(leaf_list) == 1:
continue
for leaf in leaf_list:
if self.comments_after(leaf):
return True
return False

def contains_uncollapsable_type_comments(self) -> bool:
ignored_ids = set()
try:
Expand Down
14 changes: 13 additions & 1 deletion src/black/trans.py
Expand Up @@ -390,7 +390,19 @@ def do_match(self, line: Line) -> TMatchResult:
and is_valid_index(idx + 1)
and LL[idx + 1].type == token.STRING
):
if not is_part_of_annotation(leaf):
# Let's check if the string group contains an inline comment
# If we have a comment inline, we don't merge the strings
contains_comment = False
i = idx
while is_valid_index(i):
if LL[i].type != token.STRING:
break
if line.comments_after(LL[i]):
contains_comment = True
break
i += 1

if not is_part_of_annotation(leaf) and not contains_comment:
string_indices.append(idx)

# Advance to the next non-STRING leaf.
Expand Down
6 changes: 3 additions & 3 deletions tests/data/cases/preview_long_strings__regression.py
Expand Up @@ -210,8 +210,8 @@ def foo():

some_tuple = ("some string", "some string" " which should be joined")

some_commented_string = (
"This string is long but not so long that it needs hahahah toooooo be so greatttt" # This comment gets thrown to the top.
some_commented_string = ( # This comment stays at the top.
"This string is long but not so long that it needs hahahah toooooo be so greatttt"
" {} that I just can't think of any more good words to say about it at"
" allllllllllll".format("ha") # comments here are fine
)
Expand Down Expand Up @@ -834,7 +834,7 @@ def foo():

some_tuple = ("some string", "some string which should be joined")

some_commented_string = ( # This comment gets thrown to the top.
some_commented_string = ( # This comment stays at the top.
"This string is long but not so long that it needs hahahah toooooo be so greatttt"
" {} that I just can't think of any more good words to say about it at"
" allllllllllll".format("ha") # comments here are fine
Expand Down
28 changes: 28 additions & 0 deletions tests/data/cases/preview_multiline_strings.py
Expand Up @@ -157,6 +157,24 @@ def dastardly_default_value(
`--global-option` is reserved to flags like `--verbose` or `--quiet`.
"""

this_will_become_one_line = (
"a"
"b"
"c"
)

this_will_stay_on_three_lines = (
"a" # comment
"b"
"c"
)

this_will_also_become_one_line = ( # comment
"a"
"b"
"c"
)

# output
"""cow
say""",
Expand Down Expand Up @@ -357,3 +375,13 @@ def dastardly_default_value(
Please use `--build-option` instead,
`--global-option` is reserved to flags like `--verbose` or `--quiet`.
"""

this_will_become_one_line = "abc"

this_will_stay_on_three_lines = (
"a" # comment
"b"
"c"
)

this_will_also_become_one_line = "abc" # comment

0 comments on commit 882d879

Please sign in to comment.