From b73b77a9b039d5afa6ab32202bff2b2232258d45 Mon Sep 17 00:00:00 2001 From: "Yilei \"Dolee\" Yang" Date: Wed, 26 Oct 2022 18:03:10 -0700 Subject: [PATCH 1/1] Wrap concatenated strings used as function args in parens (#3307) Fixes #3292 --- CHANGES.md | 2 + src/black/__init__.py | 6 +- src/black/mode.py | 6 +- src/black/parsing.py | 8 +- src/black/trans.py | 43 ++---- 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, 169 insertions(+), 110 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 67451f7..4db9611 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,6 +15,8 @@ - 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) ### Configuration diff --git a/src/black/__init__.py b/src/black/__init__.py index d9fba41..9459227 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -497,8 +497,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 1e83f2a..e2eff23 100644 --- a/src/black/mode.py +++ b/src/black/mode.py @@ -180,8 +180,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 762934b..c37c12b 100644 --- a/src/black/parsing.py +++ b/src/black/parsing.py @@ -29,9 +29,11 @@ try: except ImportError: if 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 7e2d8e6..8893ab0 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -1058,33 +1058,19 @@ class BaseStringSplitter(StringTransformer): if LL[0].type != token.STRING: return None - matching_nodes = [ - syms.listmaker, - syms.dictsetmaker, - syms.testlist_gexp, - ] - # If the string is an immediate child of a list/set/tuple literal... - if ( - parent_type(LL[0]) in matching_nodes - or parent_type(LL[0].parent) in matching_nodes + # 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 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 ): - # 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 + return 0 return None @@ -1653,9 +1639,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. diff --git a/tests/data/preview/cantfit.py b/tests/data/preview/cantfit.py index 0849374..cade382 100644 --- a/tests/data/preview/cantfit.py +++ b/tests/data/preview/cantfit.py @@ -79,10 +79,14 @@ normal_name = ( ) # 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 6db3cfe..9288b25 100644 --- a/tests/data/preview/long_strings.py +++ b/tests/data/preview/long_strings.py @@ -297,8 +297,10 @@ x += ( 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 @@ bad_split3 = ( ) 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 @@ comment_string = ( # This comment gets thrown to the top. ) 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 @@ return ( ) 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 634db46..8b00e76 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 @@ class A: 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 @@ some_commented_string = ( ) 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 5d0175d..9256698 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -490,8 +490,10 @@ class BlackTestCase(BlackBaseTestCase): 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) @@ -500,8 +502,10 @@ class BlackTestCase(BlackBaseTestCase): 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") @@ -510,8 +514,10 @@ class BlackTestCase(BlackBaseTestCase): 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") @@ -520,8 +526,10 @@ class BlackTestCase(BlackBaseTestCase): 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) @@ -530,22 +538,28 @@ class BlackTestCase(BlackBaseTestCase): 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: @@ -587,8 +601,10 @@ class BlackTestCase(BlackBaseTestCase): 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) @@ -596,8 +612,10 @@ class BlackTestCase(BlackBaseTestCase): 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") @@ -606,8 +624,10 @@ class BlackTestCase(BlackBaseTestCase): 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") @@ -615,8 +635,10 @@ class BlackTestCase(BlackBaseTestCase): 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) @@ -624,22 +646,28 @@ class BlackTestCase(BlackBaseTestCase): 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: @@ -683,8 +711,10 @@ class BlackTestCase(BlackBaseTestCase): 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) @@ -693,8 +723,10 @@ class BlackTestCase(BlackBaseTestCase): 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") @@ -703,8 +735,10 @@ class BlackTestCase(BlackBaseTestCase): 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") @@ -712,8 +746,10 @@ class BlackTestCase(BlackBaseTestCase): 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) @@ -721,22 +757,28 @@ class BlackTestCase(BlackBaseTestCase): 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: -- 2.39.5