Skip to content

Commit

Permalink
Regression fix: leave R prefixes capitalization alone (#2285)
Browse files Browse the repository at this point in the history
`black.strings.get_string_prefix` used to lowercase the extracted
prefix before returning it. This is wrong because 1) it ignores the
fact we should leave R prefixes alone because of MagicPython, and 2)
there is dedicated prefix casing handling code that fixes issue 1.
`.lower` is too naive.

This was originally fixed in 20.8b0, but was reintroduced since 21.4b0.

I also added proper prefix normalization for docstrings by using the
`black.strings.normalize_string_prefix` helper.

Some more test strings were added to make sure strings with capitalized
prefixes aren't treated differently (actually happened with my original
patch, Jelle had to point it out to me).
  • Loading branch information
ichard26 committed Jun 9, 2021
1 parent a9eab85 commit 00e7e12
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Expand Up @@ -12,6 +12,7 @@
- Added `--required-version` option to require a specific version to be running (#2300)
- Fix incorrect custom breakpoint indices when string group contains fake f-strings
(#2311)
- Fix regression where `R` prefixes would be lowercased for docstrings (#2285)

## 21.5b2

Expand Down
5 changes: 3 additions & 2 deletions src/black/linegen.py
Expand Up @@ -226,8 +226,9 @@ def visit_STRING(self, leaf: Leaf) -> Iterator[Line]:
if is_docstring(leaf) and "\\\n" not in leaf.value:
# We're ignoring docstrings with backslash newline escapes because changing
# indentation of those changes the AST representation of the code.
prefix = get_string_prefix(leaf.value)
docstring = leaf.value[len(prefix) :] # Remove the prefix
docstring = normalize_string_prefix(leaf.value, self.remove_u_prefix)
prefix = get_string_prefix(docstring)
docstring = docstring[len(prefix) :] # Remove the prefix
quote_char = docstring[0]
# A natural way to remove the outer quotes is to do:
# docstring = docstring.strip(quote_char)
Expand Down
2 changes: 1 addition & 1 deletion src/black/strings.py
Expand Up @@ -87,7 +87,7 @@ def get_string_prefix(string: str) -> str:
prefix = ""
prefix_idx = 0
while string[prefix_idx] in STRING_PREFIX_CHARS:
prefix += string[prefix_idx].lower()
prefix += string[prefix_idx]
prefix_idx += 1

return prefix
Expand Down
10 changes: 5 additions & 5 deletions src/black/trans.py
Expand Up @@ -411,7 +411,7 @@ def make_naked(string: str, string_prefix: str) -> str:
and is_valid_index(next_str_idx)
and LL[next_str_idx].type == token.STRING
):
prefix = get_string_prefix(LL[next_str_idx].value)
prefix = get_string_prefix(LL[next_str_idx].value).lower()
next_str_idx += 1

# The next loop merges the string group. The final string will be
Expand All @@ -431,7 +431,7 @@ def make_naked(string: str, string_prefix: str) -> str:
num_of_strings += 1

SS = LL[next_str_idx].value
next_prefix = get_string_prefix(SS)
next_prefix = get_string_prefix(SS).lower()

# If this is an f-string group but this substring is not prefixed
# with 'f'...
Expand Down Expand Up @@ -541,7 +541,7 @@ def _validate_msg(line: Line, string_idx: int) -> TResult[None]:
return TErr("StringMerger does NOT merge multiline strings.")

num_of_strings += 1
prefix = get_string_prefix(leaf.value)
prefix = get_string_prefix(leaf.value).lower()
if "r" in prefix:
return TErr("StringMerger does NOT merge raw strings.")

Expand Down Expand Up @@ -1036,7 +1036,7 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]:
is_valid_index = is_valid_index_factory(LL)
insert_str_child = insert_str_child_factory(LL[string_idx])

prefix = get_string_prefix(LL[string_idx].value)
prefix = get_string_prefix(LL[string_idx].value).lower()

# We MAY choose to drop the 'f' prefix from substrings that don't
# contain any f-expressions, but ONLY if the original f-string
Expand Down Expand Up @@ -1290,7 +1290,7 @@ def fexpr_slices() -> Iterator[Tuple[Index, Index]]:

yield from _fexpr_slices

is_fstring = "f" in get_string_prefix(string)
is_fstring = "f" in get_string_prefix(string).lower()

def breaks_fstring_expression(i: Index) -> bool:
"""
Expand Down
42 changes: 42 additions & 0 deletions tests/data/long_strings__regression.py
Expand Up @@ -495,6 +495,26 @@ async def foo(self):
f"9. You now have a key to add to `{prefix}set api youtube api_key`"
)

# It shouldn't matter if the string prefixes are capitalized.
temp_msg = (
F"{F'{humanize_number(pos)}.': <{pound_len+2}} "
F"{balance: <{bal_len + 5}} "
F"<<{author.display_name}>>\n"
)

fstring = (
F"We have to remember to escape {braces}."
" Like {these}."
F" But not {this}."
)

welcome_to_programming = R"hello," R" world!"

fstring = F"f-strings definitely make things more {difficult} than they need to be for {{black}}. But boy they sure are handy. The problem is that some lines will need to have the 'f' whereas others do not. This {line}, for example, needs one."

x = F"This is a long string which contains an f-expr that should not split {{{[i for i in range(5)]}}}."


# output


Expand Down Expand Up @@ -1100,3 +1120,25 @@ async def foo(self):
"8. No application restrictions are needed. Click Create at the bottom."
f"9. You now have a key to add to `{prefix}set api youtube api_key`"
)

# It shouldn't matter if the string prefixes are capitalized.
temp_msg = (
f"{F'{humanize_number(pos)}.': <{pound_len+2}} "
f"{balance: <{bal_len + 5}} "
f"<<{author.display_name}>>\n"
)

fstring = f"We have to remember to escape {braces}. Like {{these}}. But not {this}."

welcome_to_programming = R"hello," R" world!"

fstring = (
f"f-strings definitely make things more {difficult} than they need to be for"
" {black}. But boy they sure are handy. The problem is that some lines will need"
f" to have the 'f' whereas others do not. This {line}, for example, needs one."
)

x = (
"This is a long string which contains an f-expr that should not split"
f" {{{[i for i in range(5)]}}}."
)
21 changes: 21 additions & 0 deletions tests/data/string_prefixes.py
Expand Up @@ -6,6 +6,17 @@
r"hello"
fR"hello"


def docstring_singleline():
R"""2020 was one hell of a year. The good news is that we were able to"""


def docstring_multiline():
R"""
clear out all of the issues opened in that time :p
"""


# output


Expand All @@ -16,3 +27,13 @@
b"hello"
r"hello"
fR"hello"


def docstring_singleline():
R"""2020 was one hell of a year. The good news is that we were able to"""


def docstring_multiline():
R"""
clear out all of the issues opened in that time :p
"""

0 comments on commit 00e7e12

Please sign in to comment.