From 53748c1fee2cb16d1592b530014a0d9bcf39475a Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Tue, 28 Feb 2023 15:46:41 -0800 Subject: [PATCH 1/6] Consistently wrap two context managers in parens. --- CHANGES.md | 2 + src/black/linegen.py | 25 ++++------- src/black/lines.py | 41 +++++++++++++++++-- src/black/nodes.py | 10 +++++ .../auto_detect/features_3_8.py | 16 ++++++++ .../targeting_py38.py | 16 ++++++++ .../targeting_py39.py | 17 ++++++++ 7 files changed, 105 insertions(+), 22 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a8b556cb7e8..f042865ab18 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,6 +16,8 @@ - Add trailing commas to collection literals even if there's a comment after the last entry (#3393) +- `with` statements that contain two context managers will be consistently wrapped in + parens (#3589) ### Configuration diff --git a/src/black/linegen.py b/src/black/linegen.py index f7d3655e962..b22304bb3d7 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -2,7 +2,6 @@ Generating lines of code. """ import sys -from dataclasses import dataclass from enum import Enum, auto from functools import partial, wraps from typing import Collection, Iterator, List, Optional, Set, Union, cast @@ -16,6 +15,7 @@ from black.comments import FMT_OFF, generate_comments, list_comments from black.lines import ( Line, + RHSResult, append_leaves, can_be_split, can_omit_invisible_parens, @@ -651,17 +651,6 @@ def left_hand_split( yield result -@dataclass -class _RHSResult: - """Intermediate split result from a right hand split.""" - - head: Line - body: Line - tail: Line - opening_bracket: Leaf - closing_bracket: Leaf - - def right_hand_split( line: Line, line_length: int, @@ -685,7 +674,7 @@ def right_hand_split( def _first_right_hand_split( line: Line, omit: Collection[LeafID] = (), -) -> _RHSResult: +) -> RHSResult: """Split the line into head, body, tail starting with the last bracket pair. Note: this function should not have side effects. It's relied upon by @@ -727,11 +716,11 @@ def _first_right_hand_split( tail_leaves, line, opening_bracket, component=_BracketSplitComponent.tail ) bracket_split_succeeded_or_raise(head, body, tail) - return _RHSResult(head, body, tail, opening_bracket, closing_bracket) + return RHSResult(head, body, tail, opening_bracket, closing_bracket) def _maybe_split_omitting_optional_parens( - rhs: _RHSResult, + rhs: RHSResult, line: Line, line_length: int, features: Collection[Feature] = (), @@ -751,11 +740,11 @@ def _maybe_split_omitting_optional_parens( # there are no standalone comments in the body and not rhs.body.contains_standalone_comments(0) # and we can actually remove the parens - and can_omit_invisible_parens(rhs.body, line_length) + and can_omit_invisible_parens(rhs, line_length) ): omit = {id(rhs.closing_bracket), *omit} try: - # The _RHSResult Omitting Optional Parens. + # The RHSResult Omitting Optional Parens. rhs_oop = _first_right_hand_split(line, omit=omit) if not ( Preview.prefer_splitting_right_hand_side_of_assignments in line.mode @@ -806,7 +795,7 @@ def _maybe_split_omitting_optional_parens( yield result -def _prefer_split_rhs_oop(rhs_oop: _RHSResult, line_length: int) -> bool: +def _prefer_split_rhs_oop(rhs_oop: RHSResult, line_length: int) -> bool: """ Returns whether we should prefer the result from a split omitting optional parens. """ diff --git a/src/black/lines.py b/src/black/lines.py index ec6ef5d9522..d6154e1822e 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -14,7 +14,7 @@ ) from black.brackets import DOT_PRIORITY, BracketTracker -from black.mode import Mode +from black.mode import Mode, Preview from black.nodes import ( BRACKETS, CLOSING_BRACKETS, @@ -26,6 +26,7 @@ is_multiline_string, is_one_sequence_between, is_type_comment, + is_with_stmt, replace_child, syms, whitespace, @@ -119,6 +120,11 @@ def is_import(self) -> bool: """Is this an import line?""" return bool(self) and is_import(self.leaves[0]) + @property + def is_with_stmt(self) -> bool: + """Is this an with_stmt line?""" + return bool(self) and is_with_stmt(self.leaves[0]) + @property def is_class(self) -> bool: """Is this line a class definition?""" @@ -446,6 +452,17 @@ def __bool__(self) -> bool: return bool(self.leaves or self.comments) +@dataclass +class RHSResult: + """Intermediate split result from a right hand split.""" + + head: Line + body: Line + tail: Line + opening_bracket: Leaf + closing_bracket: Leaf + + @dataclass class LinesBlock: """Class that holds information about a block of formatted lines. @@ -752,25 +769,41 @@ def can_be_split(line: Line) -> bool: def can_omit_invisible_parens( - line: Line, + rhs: RHSResult, line_length: int, ) -> bool: - """Does `line` have a shape safe to reformat without optional parens around it? + """Does `rhs.body` have a shape safe to reformat without optional parens around it? Returns True for only a subset of potentially nice looking formattings but the point is to not return false positives that end up producing lines that are too long. """ + line = rhs.body bt = line.bracket_tracker if not bt.delimiters: # Without delimiters the optional parentheses are useless. return True max_priority = bt.max_delimiter_priority() - if bt.delimiter_count_with_priority(max_priority) > 1: + delimiter_count = bt.delimiter_count_with_priority(max_priority) + if delimiter_count > 1: # With more than one delimiter of a kind the optional parentheses read better. return False + if delimiter_count == 1: + if ( + Preview.wrap_multiple_context_managers_in_parens in line.mode + and rhs.head.is_with_stmt + ): + # For two context manager with statements, the optional parentheses read + # better. In this case, `rhs.body` is the context managers part of + # the with statement. `rhs.head` is the `with (` part on the previous + # line. + return False + # Otherwise it may also read better, but we don't do it today and requires + # careful considerations for all possible cases. See + # https://github.com/psf/black/issues/2156. + if max_priority == DOT_PRIORITY: # A single stranded method call doesn't require optional parentheses. return True diff --git a/src/black/nodes.py b/src/black/nodes.py index a588077f4de..90728f3c87c 100644 --- a/src/black/nodes.py +++ b/src/black/nodes.py @@ -789,6 +789,16 @@ def is_import(leaf: Leaf) -> bool: ) +def is_with_stmt(leaf: Leaf) -> bool: + """Return True if the given leaf starts a with statement.""" + return bool( + leaf.type == token.NAME + and leaf.value == "with" + and leaf.parent + and leaf.parent.type == syms.with_stmt + ) + + def is_type_comment(leaf: Leaf, suffix: str = "") -> bool: """Return True if the given leaf is a special comment. Only returns true for type comments for now.""" diff --git a/tests/data/preview_context_managers/auto_detect/features_3_8.py b/tests/data/preview_context_managers/auto_detect/features_3_8.py index e05094e1421..79e438b995e 100644 --- a/tests/data/preview_context_managers/auto_detect/features_3_8.py +++ b/tests/data/preview_context_managers/auto_detect/features_3_8.py @@ -16,6 +16,14 @@ pass +with mock.patch.object( + self.my_runner, "first_method", autospec=True +) as mock_run_adb, mock.patch.object( + self.my_runner, "second_method", autospec=True, return_value="foo" +): + pass + + # output # This file doesn't use any Python 3.9+ only grammars. @@ -28,3 +36,11 @@ with make_context_manager1() as cm1, make_context_manager2() as cm2, make_context_manager3() as cm3, make_context_manager4() as cm4: pass + + +with mock.patch.object( + self.my_runner, "first_method", autospec=True +) as mock_run_adb, mock.patch.object( + self.my_runner, "second_method", autospec=True, return_value="foo" +): + pass diff --git a/tests/data/preview_context_managers/targeting_py38.py b/tests/data/preview_context_managers/targeting_py38.py index 6ec4684e441..f125cdffb8a 100644 --- a/tests/data/preview_context_managers/targeting_py38.py +++ b/tests/data/preview_context_managers/targeting_py38.py @@ -23,6 +23,14 @@ pass +with mock.patch.object( + self.my_runner, "first_method", autospec=True +) as mock_run_adb, mock.patch.object( + self.my_runner, "second_method", autospec=True, return_value="foo" +): + pass + + # output @@ -36,3 +44,11 @@ with new_new_new1() as cm1, new_new_new2(): pass + + +with mock.patch.object( + self.my_runner, "first_method", autospec=True +) as mock_run_adb, mock.patch.object( + self.my_runner, "second_method", autospec=True, return_value="foo" +): + pass diff --git a/tests/data/preview_context_managers/targeting_py39.py b/tests/data/preview_context_managers/targeting_py39.py index 64f5d09bbe8..dcdeac32412 100644 --- a/tests/data/preview_context_managers/targeting_py39.py +++ b/tests/data/preview_context_managers/targeting_py39.py @@ -49,6 +49,14 @@ pass +with mock.patch.object( + self.my_runner, "first_method", autospec=True +) as mock_run_adb, mock.patch.object( + self.my_runner, "second_method", autospec=True, return_value="foo" +): + pass + + # output @@ -102,3 +110,12 @@ ) as cm2, ): pass + + +with ( + mock.patch.object(self.my_runner, "first_method", autospec=True) as mock_run_adb, + mock.patch.object( + self.my_runner, "second_method", autospec=True, return_value="foo" + ), +): + pass From 7a044f1bf9e03265dc35fa877f2baac00b3bd43d Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Tue, 28 Feb 2023 16:01:24 -0800 Subject: [PATCH 2/6] Make sure the single delimiter is a comma so it's actually two CMs. --- src/black/lines.py | 3 ++- .../targeting_py39.py | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/black/lines.py b/src/black/lines.py index d6154e1822e..2077fe5b14d 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -13,7 +13,7 @@ cast, ) -from black.brackets import DOT_PRIORITY, BracketTracker +from black.brackets import COMMA_PRIORITY, DOT_PRIORITY, BracketTracker from black.mode import Mode, Preview from black.nodes import ( BRACKETS, @@ -793,6 +793,7 @@ def can_omit_invisible_parens( if delimiter_count == 1: if ( Preview.wrap_multiple_context_managers_in_parens in line.mode + and max_priority == COMMA_PRIORITY and rhs.head.is_with_stmt ): # For two context manager with statements, the optional parentheses read diff --git a/tests/data/preview_context_managers/targeting_py39.py b/tests/data/preview_context_managers/targeting_py39.py index dcdeac32412..643c6fd958b 100644 --- a/tests/data/preview_context_managers/targeting_py39.py +++ b/tests/data/preview_context_managers/targeting_py39.py @@ -57,6 +57,16 @@ pass +with xxxxxxxx.some_kind_of_method( + some_argument=[ + "first", + "second", + "third", + ] +).another_method() as cmd: + pass + + # output @@ -119,3 +129,13 @@ ), ): pass + + +with xxxxxxxx.some_kind_of_method( + some_argument=[ + "first", + "second", + "third", + ] +).another_method() as cmd: + pass From 4be75b03aa0a2bfed72e945f9fd3c96b5fa85531 Mon Sep 17 00:00:00 2001 From: "Yilei \"Dolee\" Yang" Date: Tue, 28 Feb 2023 21:12:17 -0800 Subject: [PATCH 3/6] Update CHANGES.md Co-authored-by: Jelle Zijlstra --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index f042865ab18..f680c979d25 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,7 +17,7 @@ - Add trailing commas to collection literals even if there's a comment after the last entry (#3393) - `with` statements that contain two context managers will be consistently wrapped in - parens (#3589) + parentheses (#3589) ### Configuration From 89ed5dfa2284cf37488c15c8dd270c28b6dafbe0 Mon Sep 17 00:00:00 2001 From: "Yilei \"Dolee\" Yang" Date: Tue, 28 Feb 2023 21:12:28 -0800 Subject: [PATCH 4/6] Update src/black/lines.py Co-authored-by: Jelle Zijlstra --- src/black/lines.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/lines.py b/src/black/lines.py index 2077fe5b14d..b5d28c606e0 100644 --- a/src/black/lines.py +++ b/src/black/lines.py @@ -122,7 +122,7 @@ def is_import(self) -> bool: @property def is_with_stmt(self) -> bool: - """Is this an with_stmt line?""" + """Is this a with_stmt line?""" return bool(self) and is_with_stmt(self.leaves[0]) @property From e702b0b013b50bc8a1c1af5e4900b297b9c6124c Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 2 Mar 2023 22:43:25 -0800 Subject: [PATCH 5/6] Empty commit to try trigger CI From e8f81c941419355cb4cea087570978d313599b08 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 9 Mar 2023 07:32:07 -0800 Subject: [PATCH 6/6] Fix merge. --- src/black/linegen.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/black/linegen.py b/src/black/linegen.py index 2d13606df13..6f67799e717 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -2,6 +2,7 @@ Generating lines of code. """ import sys +from dataclasses import replace from enum import Enum, auto from functools import partial, wraps from typing import Collection, Iterator, List, Optional, Set, Union, cast