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

Add parens around implicit string concatenations where increases readability #3162

Merged
merged 7 commits into from Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Expand Up @@ -26,6 +26,8 @@
normalized as expected (#3168)
- When using `--skip-magic-trailing-comma` or `-C`, trailing commas are stripped from
subscript expressions with more than 1 element (#3209)
- Implicitly concatenated strings inside a list, set, or function call are now wrapped
inside parentheses (#3162)
- Fix a string merging/split issue when a comment is present in the middle of implicitly
concatenated strings on its own line (#3227)

Expand Down
6 changes: 4 additions & 2 deletions src/black/__init__.py
Expand Up @@ -498,8 +498,10 @@ def main( # noqa: C901
user_level_config = str(find_user_pyproject_toml())
if config == user_level_config:
out(
"Using configuration from user-level config at "
f"'{user_level_config}'.",
(
"Using configuration from user-level config at "
f"'{user_level_config}'."
),
fg="blue",
)
elif config_source in (
Expand Down
6 changes: 4 additions & 2 deletions src/black/mode.py
Expand Up @@ -178,8 +178,10 @@ class Mode:
def __post_init__(self) -> None:
if self.experimental_string_processing:
warn(
"`experimental string processing` has been included in `preview`"
" and deprecated. Use `preview` instead.",
(
"`experimental string processing` has been included in `preview`"
" and deprecated. Use `preview` instead."
),
Deprecated,
)

Expand Down
8 changes: 5 additions & 3 deletions src/black/parsing.py
Expand Up @@ -30,9 +30,11 @@
# Either our python version is too low, or we're on pypy
if sys.version_info < (3, 7) or (sys.version_info < (3, 8) and not _IS_PYPY):
print(
"The typed_ast package is required but not installed.\n"
"You can upgrade to Python 3.8+ or install typed_ast with\n"
"`python3 -m pip install typed-ast`.",
(
"The typed_ast package is required but not installed.\n"
"You can upgrade to Python 3.8+ or install typed_ast with\n"
"`python3 -m pip install typed-ast`."
),
file=sys.stderr,
)
sys.exit(1)
Expand Down
52 changes: 50 additions & 2 deletions src/black/trans.py
Expand Up @@ -1043,6 +1043,43 @@ def _get_max_string_length(self, line: Line, string_idx: int) -> int:
max_string_length = self.line_length - offset
return max_string_length

@staticmethod
def _prefer_paren_wrap_match(LL: List[Leaf]) -> Optional[int]:
"""
Returns:
string_idx such that @LL[string_idx] is equal to our target (i.e.
matched) string, if this line matches the "prefer paren wrap" statement
requirements listed in the 'Requirements' section of the StringParenWrapper
class's docstring.
OR
None, otherwise.
"""
# The line must start with a string.
if LL[0].type != token.STRING:
return None

matching_nodes = [
syms.listmaker,
syms.dictsetmaker,
syms.testlist_gexp,
syms.arglist,
]
# If the string is an immediate child of a list/set/tuple literal, or
# an argument of a function call...
if (
parent_type(LL[0]) in matching_nodes
or parent_type(LL[0].parent) in matching_nodes
):
# And the string is surrounded by commas (or is the first/last child)...
prev_sibling = LL[0].prev_sibling
next_sibling = LL[0].next_sibling
if (not prev_sibling or prev_sibling.type == token.COMMA) and (
not next_sibling or next_sibling.type == token.COMMA
):
return 0

return None


def iter_fexpr_spans(s: str) -> Iterator[Tuple[int, int]]:
"""
Expand Down Expand Up @@ -1138,6 +1175,9 @@ class StringSplitter(BaseStringSplitter, CustomSplitMapMixin):
def do_splitter_match(self, line: Line) -> TMatchResult:
LL = line.leaves

if self._prefer_paren_wrap_match(LL) is not None:
return TErr("Line needs to be wrapped in parens first.")

is_valid_index = is_valid_index_factory(LL)

idx = 0
Expand Down Expand Up @@ -1583,8 +1623,7 @@ def _get_string_operator_leaves(self, leaves: Iterable[Leaf]) -> List[Leaf]:

class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin):
"""
StringTransformer that splits non-"atom" strings (i.e. strings that do not
exist on lines by themselves).
StringTransformer that wraps strings in parens and then splits at the LPAR.

Requirements:
All of the requirements listed in BaseStringSplitter's docstring in
Expand All @@ -1604,6 +1643,11 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin):
OR
* The line is a dictionary key assignment where some valid key is being
assigned the value of some string.
OR
* The line starts with an "atom" string that prefers to be wrapped in
parens. It's preferred to be wrapped when it's is an immediate child of
a list/set/tuple literal or an argument of a function call, AND the
string is surrounded by commas (or is the first/last child).

Transformations:
The chosen string is wrapped in parentheses and then split at the LPAR.
Expand All @@ -1628,6 +1672,9 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin):
changed such that it no longer needs to be given its own line,
StringParenWrapper relies on StringParenStripper to clean up the
parentheses it created.

For "atom" strings that prefers to be wrapped in parens, it requires
StringSplitter to hold the split until the string is wrapped in parens.
"""

def do_splitter_match(self, line: Line) -> TMatchResult:
Expand All @@ -1644,6 +1691,7 @@ def do_splitter_match(self, line: Line) -> TMatchResult:
or self._assert_match(LL)
or self._assign_match(LL)
or self._dict_match(LL)
or self._prefer_paren_wrap_match(LL)
)

if string_idx is not None:
Expand Down
12 changes: 8 additions & 4 deletions tests/data/preview/cantfit.py
Expand Up @@ -79,10 +79,14 @@
)
# long arguments
normal_name = normal_function_name(
"but with super long string arguments that on their own exceed the line limit so"
" there's no way it can ever fit",
"eggs with spam and eggs and spam with eggs with spam and eggs and spam with eggs"
" with spam and eggs and spam with eggs",
(
"but with super long string arguments that on their own exceed the line limit"
" so there's no way it can ever fit"
),
(
"eggs with spam and eggs and spam with eggs with spam and eggs and spam with"
" eggs with spam and eggs and spam with eggs"
),
this_is_a_ridiculously_long_name_and_nobody_in_their_right_mind_would_use_one_like_it=0,
)
string_variable_name = "a string that is waaaaaaaayyyyyyyy too long, even in parens, there's nothing you can do" # noqa
Expand Down
46 changes: 30 additions & 16 deletions tests/data/preview/comments7.py
Expand Up @@ -226,39 +226,53 @@ class C:
# metadata_version errors.
(
{},
"None is an invalid value for Metadata-Version. Error: This field is"
" required. see"
" https://packaging.python.org/specifications/core-metadata",
(
"None is an invalid value for Metadata-Version. Error: This field"
" is required. see"
" https://packaging.python.org/specifications/core-metadata"
),
),
(
{"metadata_version": "-1"},
"'-1' is an invalid value for Metadata-Version. Error: Unknown Metadata"
" Version see"
" https://packaging.python.org/specifications/core-metadata",
(
"'-1' is an invalid value for Metadata-Version. Error: Unknown"
" Metadata Version see"
" https://packaging.python.org/specifications/core-metadata"
),
),
# name errors.
(
{"metadata_version": "1.2"},
"'' is an invalid value for Name. Error: This field is required. see"
" https://packaging.python.org/specifications/core-metadata",
(
"'' is an invalid value for Name. Error: This field is required."
" see https://packaging.python.org/specifications/core-metadata"
),
),
(
{"metadata_version": "1.2", "name": "foo-"},
"'foo-' is an invalid value for Name. Error: Must start and end with a"
" letter or numeral and contain only ascii numeric and '.', '_' and"
" '-'. see https://packaging.python.org/specifications/core-metadata",
(
"'foo-' is an invalid value for Name. Error: Must start and end"
" with a letter or numeral and contain only ascii numeric and '.',"
" '_' and '-'. see"
" https://packaging.python.org/specifications/core-metadata"
),
),
# version errors.
(
{"metadata_version": "1.2", "name": "example"},
"'' is an invalid value for Version. Error: This field is required. see"
" https://packaging.python.org/specifications/core-metadata",
(
"'' is an invalid value for Version. Error: This field is required."
" see https://packaging.python.org/specifications/core-metadata"
),
),
(
{"metadata_version": "1.2", "name": "example", "version": "dog"},
"'dog' is an invalid value for Version. Error: Must start and end with"
" a letter or numeral and contain only ascii numeric and '.', '_' and"
" '-'. see https://packaging.python.org/specifications/core-metadata",
(
"'dog' is an invalid value for Version. Error: Must start and end"
" with a letter or numeral and contain only ascii numeric and '.',"
" '_' and '-'. see"
" https://packaging.python.org/specifications/core-metadata"
),
),
],
)
Expand Down