From 7e4088a13b030b004571e40ebcbd1d20a3df566d Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Mon, 3 Oct 2022 12:56:34 -0700 Subject: [PATCH 1/4] Wrap concatenated strings used as function args in parens. --- CHANGES.md | 2 + src/black/__init__.py | 6 +- src/black/mode.py | 6 +- src/black/parsing.py | 8 +- src/black/trans.py | 1 + tests/data/preview/cantfit.py | 12 +- tests/data/preview/long_strings.py | 54 +++++--- .../data/preview/long_strings__regression.py | 22 +-- tests/test_black.py | 126 ++++++++++++------ 9 files changed, 156 insertions(+), 81 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d3ba53fd05f..d0e7c634a9f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -22,6 +22,8 @@ - Fix a crash when formatting some dicts with parenthesis-wrapped long string keys (#3262) +- Implicitly concatenated strings used as function args are now wrapped inside + parentheses (#3307) ### Configuration diff --git a/src/black/__init__.py b/src/black/__init__.py index 5b8c9749119..2b43de4fede 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -490,8 +490,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 ( diff --git a/src/black/mode.py b/src/black/mode.py index 6c0847e8bcc..1fdc90c05f4 100644 --- a/src/black/mode.py +++ b/src/black/mode.py @@ -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, ) diff --git a/src/black/parsing.py b/src/black/parsing.py index 64c0b1e3018..57bee330240 100644 --- a/src/black/parsing.py +++ b/src/black/parsing.py @@ -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) diff --git a/src/black/trans.py b/src/black/trans.py index 7e2d8e67c1a..b71f60341b2 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -1062,6 +1062,7 @@ def _prefer_paren_wrap_match(LL: List[Leaf]) -> Optional[int]: syms.listmaker, syms.dictsetmaker, syms.testlist_gexp, + syms.arglist, ] # If the string is an immediate child of a list/set/tuple literal... if ( diff --git a/tests/data/preview/cantfit.py b/tests/data/preview/cantfit.py index 0849374f776..cade382e30d 100644 --- a/tests/data/preview/cantfit.py +++ b/tests/data/preview/cantfit.py @@ -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 diff --git a/tests/data/preview/long_strings.py b/tests/data/preview/long_strings.py index 6db3cfed9a9..9288b253b60 100644 --- a/tests/data/preview/long_strings.py +++ b/tests/data/preview/long_strings.py @@ -297,8 +297,10 @@ def foo(): y = "Short string" print( - "This is a really long string inside of a print statement with extra arguments" - " attached at the end of it.", + ( + "This is a really long string inside of a print statement with extra arguments" + " attached at the end of it." + ), x, y, z, @@ -474,13 +476,15 @@ def foo(): ) bad_split_func1( - "But what should happen when code has already " - "been formatted but in the wrong way? Like " - "with a space at the end instead of the " - "beginning. Or what about when it is split too " - "soon? In the case of a split that is too " - "short, black will try to honer the custom " - "split.", + ( + "But what should happen when code has already " + "been formatted but in the wrong way? Like " + "with a space at the end instead of the " + "beginning. Or what about when it is split too " + "soon? In the case of a split that is too " + "short, black will try to honer the custom " + "split." + ), xxx, yyy, zzz, @@ -583,9 +587,11 @@ def foo(): ) arg_comment_string = print( - "Long lines with inline comments which are apart of (and not the only member of) an" - " argument list should have their comments appended to the reformatted string's" - " enclosing left parentheses.", # This comment gets thrown to the top. + ( # This comment gets thrown to the top. + "Long lines with inline comments which are apart of (and not the only member" + " of) an argument list should have their comments appended to the reformatted" + " string's enclosing left parentheses." + ), "Arg #2", "Arg #3", "Arg #4", @@ -645,23 +651,31 @@ def foo(): ) func_with_bad_comma( - "This is a really long string argument to a function that has a trailing comma" - " which should NOT be there.", + ( + "This is a really long string argument to a function that has a trailing comma" + " which should NOT be there." + ), ) func_with_bad_comma( - "This is a really long string argument to a function that has a trailing comma" - " which should NOT be there.", # comment after comma + ( # comment after comma + "This is a really long string argument to a function that has a trailing comma" + " which should NOT be there." + ), ) func_with_bad_comma( - "This is a really long string argument to a function that has a trailing comma" - " which should NOT be there.", + ( + "This is a really long string argument to a function that has a trailing comma" + " which should NOT be there." + ), ) func_with_bad_comma( - "This is a really long string argument to a function that has a trailing comma" - " which should NOT be there.", # comment after comma + ( # comment after comma + "This is a really long string argument to a function that has a trailing comma" + " which should NOT be there." + ), ) func_with_bad_parens_that_wont_fit_in_one_line( diff --git a/tests/data/preview/long_strings__regression.py b/tests/data/preview/long_strings__regression.py index 634db46a5e0..8b00e76f40e 100644 --- a/tests/data/preview/long_strings__regression.py +++ b/tests/data/preview/long_strings__regression.py @@ -679,9 +679,11 @@ class A: def foo(): some_func_call( "xxxxxxxxxx", - "xx {xxxxxxxxxxx}/xxxxxxxxxxx.xxx xxxx.xxx && xxxxxx -x " - '"xxxx xxxxxxx xxxxxx xxxx; xxxx xxxxxx_xxxxx xxxxxx xxxx; ' - "xxxx.xxxx_xxxxxx(['xxxx.xxx'], xxxx.xxxxxxx().xxxxxxxxxx)\" ", + ( + "xx {xxxxxxxxxxx}/xxxxxxxxxxx.xxx xxxx.xxx && xxxxxx -x " + '"xxxx xxxxxxx xxxxxx xxxx; xxxx xxxxxx_xxxxx xxxxxx xxxx; ' + "xxxx.xxxx_xxxxxx(['xxxx.xxx'], xxxx.xxxxxxx().xxxxxxxxxx)\" " + ), None, ("xxxxxxxxxxx",), ), @@ -690,9 +692,11 @@ def foo(): class A: def foo(): some_func_call( - "xx {xxxxxxxxxxx}/xxxxxxxxxxx.xxx xxxx.xxx && xxxxxx -x " - "xxxx, ('xxxxxxx xxxxxx xxxx, xxxx') xxxxxx_xxxxx xxxxxx xxxx; " - "xxxx.xxxx_xxxxxx(['xxxx.xxx'], xxxx.xxxxxxx().xxxxxxxxxx)\" ", + ( + "xx {xxxxxxxxxxx}/xxxxxxxxxxx.xxx xxxx.xxx && xxxxxx -x " + "xxxx, ('xxxxxxx xxxxxx xxxx, xxxx') xxxxxx_xxxxx xxxxxx xxxx; " + "xxxx.xxxx_xxxxxx(['xxxx.xxx'], xxxx.xxxxxxx().xxxxxxxxxx)\" " + ), None, ("xxxxxxxxxxx",), ), @@ -810,8 +814,10 @@ def foo(): ) lpar_and_rpar_have_comments = func_call( # LPAR Comment - "Long really ridiculous type of string that shouldn't really even exist at all. I" - " mean commmme onnn!!!", # Comma Comment + ( # Comma Comment + "Long really ridiculous type of string that shouldn't really even exist at all." + " I mean commmme onnn!!!" + ), ) # RPAR Comment cmd_fstring = ( diff --git a/tests/test_black.py b/tests/test_black.py index abd4d00b8e8..7a4e9851bdf 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -466,8 +466,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(err_lines[-1], "error: cannot format e1: boom") self.assertEqual( unstyle(str(report)), - "1 file reformatted, 2 files left unchanged, 1 file failed to" - " reformat.", + ( + "1 file reformatted, 2 files left unchanged, 1 file failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.done(Path("f3"), black.Changed.YES) @@ -476,8 +478,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(out_lines[-1], "reformatted f3") self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 1 file failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 1 file failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.failed(Path("e2"), "boom") @@ -486,8 +490,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(err_lines[-1], "error: cannot format e2: boom") self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.path_ignored(Path("wat"), "no match") @@ -496,8 +502,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(out_lines[-1], "wat ignored: no match") self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.done(Path("f4"), black.Changed.NO) @@ -506,22 +514,28 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(out_lines[-1], "f4 already well formatted, good job.") self.assertEqual( unstyle(str(report)), - "2 files reformatted, 3 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 3 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.check = True self.assertEqual( unstyle(str(report)), - "2 files would be reformatted, 3 files would be left unchanged, 2" - " files would fail to reformat.", + ( + "2 files would be reformatted, 3 files would be left unchanged, 2" + " files would fail to reformat." + ), ) report.check = False report.diff = True self.assertEqual( unstyle(str(report)), - "2 files would be reformatted, 3 files would be left unchanged, 2" - " files would fail to reformat.", + ( + "2 files would be reformatted, 3 files would be left unchanged, 2" + " files would fail to reformat." + ), ) def test_report_quiet(self) -> None: @@ -563,8 +577,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(err_lines[-1], "error: cannot format e1: boom") self.assertEqual( unstyle(str(report)), - "1 file reformatted, 2 files left unchanged, 1 file failed to" - " reformat.", + ( + "1 file reformatted, 2 files left unchanged, 1 file failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.done(Path("f3"), black.Changed.YES) @@ -572,8 +588,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(len(err_lines), 1) self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 1 file failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 1 file failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.failed(Path("e2"), "boom") @@ -582,8 +600,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(err_lines[-1], "error: cannot format e2: boom") self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.path_ignored(Path("wat"), "no match") @@ -591,8 +611,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(len(err_lines), 2) self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.done(Path("f4"), black.Changed.NO) @@ -600,22 +622,28 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(len(err_lines), 2) self.assertEqual( unstyle(str(report)), - "2 files reformatted, 3 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 3 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.check = True self.assertEqual( unstyle(str(report)), - "2 files would be reformatted, 3 files would be left unchanged, 2" - " files would fail to reformat.", + ( + "2 files would be reformatted, 3 files would be left unchanged, 2" + " files would fail to reformat." + ), ) report.check = False report.diff = True self.assertEqual( unstyle(str(report)), - "2 files would be reformatted, 3 files would be left unchanged, 2" - " files would fail to reformat.", + ( + "2 files would be reformatted, 3 files would be left unchanged, 2" + " files would fail to reformat." + ), ) def test_report_normal(self) -> None: @@ -659,8 +687,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(err_lines[-1], "error: cannot format e1: boom") self.assertEqual( unstyle(str(report)), - "1 file reformatted, 2 files left unchanged, 1 file failed to" - " reformat.", + ( + "1 file reformatted, 2 files left unchanged, 1 file failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.done(Path("f3"), black.Changed.YES) @@ -669,8 +699,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(out_lines[-1], "reformatted f3") self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 1 file failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 1 file failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.failed(Path("e2"), "boom") @@ -679,8 +711,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(err_lines[-1], "error: cannot format e2: boom") self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.path_ignored(Path("wat"), "no match") @@ -688,8 +722,10 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(len(err_lines), 2) self.assertEqual( unstyle(str(report)), - "2 files reformatted, 2 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 2 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.done(Path("f4"), black.Changed.NO) @@ -697,22 +733,28 @@ def err(msg: str, **kwargs: Any) -> None: self.assertEqual(len(err_lines), 2) self.assertEqual( unstyle(str(report)), - "2 files reformatted, 3 files left unchanged, 2 files failed to" - " reformat.", + ( + "2 files reformatted, 3 files left unchanged, 2 files failed to" + " reformat." + ), ) self.assertEqual(report.return_code, 123) report.check = True self.assertEqual( unstyle(str(report)), - "2 files would be reformatted, 3 files would be left unchanged, 2" - " files would fail to reformat.", + ( + "2 files would be reformatted, 3 files would be left unchanged, 2" + " files would fail to reformat." + ), ) report.check = False report.diff = True self.assertEqual( unstyle(str(report)), - "2 files would be reformatted, 3 files would be left unchanged, 2" - " files would fail to reformat.", + ( + "2 files would be reformatted, 3 files would be left unchanged, 2" + " files would fail to reformat." + ), ) def test_lib2to3_parse(self) -> None: From 708eec4eff7a6a5160dff24180c382da8b96f192 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Tue, 25 Oct 2022 18:39:47 -0700 Subject: [PATCH 2/4] Update CHANGES.md --- CHANGES.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 4a04821917a..4db961149bf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,8 +14,6 @@ -- Fix a crash when formatting some dicts with parenthesis-wrapped long string keys - (#3262) - Enforce empty lines before classes and functions with sticky leading comments (#3302) - Implicitly concatenated strings used as function args are now wrapped inside parentheses (#3307) From 2e37145673a3b8354441e7c61cad0395709974ed Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Wed, 26 Oct 2022 14:04:13 -0700 Subject: [PATCH 3/4] Update docstring & comment, also remove the unnecessary matching_nodes check. --- src/black/trans.py | 46 +++++++++++++++++----------------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/src/black/trans.py b/src/black/trans.py index b71f60341b2..384eaee2693 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -1058,34 +1058,23 @@ def _prefer_paren_wrap_match(LL: List[Leaf]) -> Optional[int]: 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... + # If 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 ( - parent_type(LL[0]) in matching_nodes - or parent_type(LL[0].parent) in matching_nodes + not prev_sibling + and not next_sibling + and parent_type(LL[0]) == syms.atom ): - # 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 - and not next_sibling - and parent_type(LL[0]) == syms.atom - ): - # If it's an atom string, we need to check the parent atom's siblings. - parent = LL[0].parent - assert parent is not None # For type checkers. - prev_sibling = parent.prev_sibling - next_sibling = parent.next_sibling - if (not prev_sibling or prev_sibling.type == token.COMMA) and ( - not next_sibling or next_sibling.type == token.COMMA - ): - return 0 + # If it's an atom string, we need to check the parent atom's siblings. + parent = LL[0].parent + assert parent is not None # For type checkers. + prev_sibling = parent.prev_sibling + next_sibling = parent.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 @@ -1654,9 +1643,8 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin): 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, AND the string is surrounded by commas (or is - the first/last child). + parens. It's preferred to be wrapped when 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. From 5f4e8105abc2123932937f629b454a2a030e4de7 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Wed, 26 Oct 2022 14:08:07 -0700 Subject: [PATCH 4/4] Format outself. --- src/black/trans.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/black/trans.py b/src/black/trans.py index 384eaee2693..8893ab02aab 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -1061,11 +1061,7 @@ def _prefer_paren_wrap_match(LL: List[Leaf]) -> Optional[int]: # If 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 - and not next_sibling - and parent_type(LL[0]) == syms.atom - ): + if not prev_sibling and not next_sibling and parent_type(LL[0]) == syms.atom: # If it's an atom string, we need to check the parent atom's siblings. parent = LL[0].parent assert parent is not None # For type checkers.