From 23c77ee1f8fad6f96251ed627142f7263a504264 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Mon, 13 Mar 2023 09:17:25 -0700 Subject: [PATCH] Rebase Pyink to https://github.com/psf/black/commit/d16a1dbd05832632d7f3be28f0a4b6e9b208807c. PiperOrigin-RevId: 516230302 --- patches/pyink.patch | 76 +++++++++---------- src/pyink/linegen.py | 26 ++----- src/pyink/lines.py | 42 +++++++++- src/pyink/nodes.py | 10 +++ .../auto_detect/features_3_8.py | 16 ++++ .../targeting_py38.py | 16 ++++ .../targeting_py39.py | 37 +++++++++ 7 files changed, 163 insertions(+), 60 deletions(-) diff --git a/patches/pyink.patch b/patches/pyink.patch index 5f22b036b9e..0edc6db695b 100644 --- a/patches/pyink.patch +++ b/patches/pyink.patch @@ -349,7 +349,7 @@ --- a/linegen.py +++ b/linegen.py @@ -5,7 +5,7 @@ import sys - from dataclasses import dataclass, replace + from dataclasses import replace from enum import Enum, auto from functools import partial, wraps -from typing import Collection, Iterator, List, Optional, Set, Union, cast @@ -363,9 +363,9 @@ from pyink.lines import ( + Indentation, Line, + RHSResult, append_leaves, - can_be_split, -@@ -80,6 +81,15 @@ LeafID = int +@@ -81,6 +82,15 @@ LeafID = int LN = Union[Leaf, Node] @@ -381,7 +381,7 @@ class CannotSplit(CannotTransform): """A readable split that fits the allotted line length is impossible.""" -@@ -99,7 +109,9 @@ class LineGenerator(Visitor[Line]): +@@ -100,7 +110,9 @@ class LineGenerator(Visitor[Line]): self.current_line: Line self.__post_init__() @@ -392,7 +392,7 @@ """Generate a line. If the line is empty, only emit if it makes sense. -@@ -108,11 +120,20 @@ class LineGenerator(Visitor[Line]): +@@ -109,11 +121,20 @@ class LineGenerator(Visitor[Line]): If any lines were generated, set up a new current_line. """ if not self.current_line: @@ -415,7 +415,7 @@ yield complete_line def visit_default(self, node: LN) -> Iterator[Line]: -@@ -138,7 +159,9 @@ class LineGenerator(Visitor[Line]): +@@ -139,7 +160,9 @@ class LineGenerator(Visitor[Line]): normalize_prefix(node, inside_brackets=any_open_brackets) if self.mode.string_normalization and node.type == token.STRING: node.value = normalize_string_prefix(node.value) @@ -426,7 +426,7 @@ if node.type == token.NUMBER: normalize_numeric_literal(node) if node.type not in WHITESPACE: -@@ -148,7 +171,10 @@ class LineGenerator(Visitor[Line]): +@@ -149,7 +172,10 @@ class LineGenerator(Visitor[Line]): def visit_test(self, node: Node) -> Iterator[Line]: """Visit an `x if y else z` test""" @@ -438,7 +438,7 @@ already_parenthesized = ( node.prev_sibling and node.prev_sibling.type == token.LPAR ) -@@ -164,7 +190,7 @@ class LineGenerator(Visitor[Line]): +@@ -165,7 +191,7 @@ class LineGenerator(Visitor[Line]): def visit_INDENT(self, node: Leaf) -> Iterator[Line]: """Increase indentation level, maybe yield a line.""" # In blib2to3 INDENT never holds comments. @@ -447,7 +447,7 @@ yield from self.visit_default(node) def visit_DEDENT(self, node: Leaf) -> Iterator[Line]: -@@ -179,7 +205,7 @@ class LineGenerator(Visitor[Line]): +@@ -180,7 +206,7 @@ class LineGenerator(Visitor[Line]): yield from self.visit_default(node) # Finally, emit the dedent. @@ -456,7 +456,7 @@ def visit_stmt( self, node: Node, keywords: Set[str], parens: Set[str] -@@ -275,9 +301,9 @@ class LineGenerator(Visitor[Line]): +@@ -276,9 +302,9 @@ class LineGenerator(Visitor[Line]): if self.mode.is_pyi and is_stub_body(node): yield from self.visit_default(node) else: @@ -468,7 +468,7 @@ else: if ( -@@ -367,10 +393,13 @@ class LineGenerator(Visitor[Line]): +@@ -368,10 +394,13 @@ class LineGenerator(Visitor[Line]): yield from self.visit_default(node) def visit_STRING(self, leaf: Leaf) -> Iterator[Line]: @@ -484,7 +484,7 @@ # We're ignoring docstrings with backslash newline escapes because changing # indentation of those changes the AST representation of the code. if self.mode.string_normalization: -@@ -381,7 +410,9 @@ class LineGenerator(Visitor[Line]): +@@ -382,7 +411,9 @@ class LineGenerator(Visitor[Line]): # formatting as visit_default() is called *after*. To avoid a # situation where this function formats a docstring differently on # the second pass, normalize it early. @@ -495,7 +495,7 @@ else: docstring = leaf.value prefix = get_string_prefix(docstring) -@@ -395,7 +426,7 @@ class LineGenerator(Visitor[Line]): +@@ -396,7 +427,7 @@ class LineGenerator(Visitor[Line]): quote_len = 1 if docstring[1] != quote_char else 3 docstring = docstring[quote_len:-quote_len] docstring_started_empty = not docstring @@ -504,7 +504,7 @@ if is_multiline_string(leaf): docstring = fix_docstring(docstring, indent) -@@ -437,7 +468,11 @@ class LineGenerator(Visitor[Line]): +@@ -438,7 +469,11 @@ class LineGenerator(Visitor[Line]): # If docstring is one line, we don't put the closing quotes on a # separate line because it looks ugly (#3320). lines = docstring.splitlines() @@ -517,7 +517,7 @@ # If adding closing quotes would cause the last line to exceed # the maximum line length then put a line break before the -@@ -476,7 +511,8 @@ class LineGenerator(Visitor[Line]): +@@ -477,7 +512,8 @@ class LineGenerator(Visitor[Line]): self.visit_classdef = partial(v, keywords={"class"}, parens=Ø) self.visit_expr_stmt = partial(v, keywords=Ø, parens=ASSIGNMENTS) self.visit_return_stmt = partial(v, keywords={"return"}, parens={"return"}) @@ -527,7 +527,7 @@ self.visit_del_stmt = partial(v, keywords=Ø, parens={"del"}) self.visit_async_funcdef = self.visit_async_stmt self.visit_decorated = self.visit_decorators -@@ -503,10 +539,11 @@ def transform_line( +@@ -504,10 +540,11 @@ def transform_line( ll = mode.line_length sn = mode.string_normalization @@ -543,9 +543,9 @@ transformers: List[Transformer] if ( -@@ -692,8 +729,7 @@ def _first_right_hand_split( +@@ -682,8 +719,7 @@ def _first_right_hand_split( 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 @@ -553,7 +553,7 @@ _maybe_split_omitting_optional_parens to get an opinion whether to prefer splitting on the right side of an assignment statement. """ -@@ -722,12 +758,53 @@ def _first_right_hand_split( +@@ -712,12 +748,53 @@ def _first_right_hand_split( tail_leaves.reverse() body_leaves.reverse() head_leaves.reverse() @@ -610,7 +610,7 @@ tail = bracket_split_build_line( tail_leaves, line, opening_bracket, component=_BracketSplitComponent.tail ) -@@ -886,7 +963,7 @@ def bracket_split_build_line( +@@ -876,7 +953,7 @@ def bracket_split_build_line( result = Line(mode=original.mode, depth=original.depth) if component is _BracketSplitComponent.body: result.inside_brackets = True @@ -619,7 +619,7 @@ if leaves: # Since body is a new indent level, remove spurious leading whitespace. normalize_prefix(leaves[0], inside_brackets=True) -@@ -1444,7 +1521,7 @@ def generate_trailers_to_omit(line: Line +@@ -1434,7 +1511,7 @@ def generate_trailers_to_omit(line: Line if not line.magic_trailing_comma: yield omit @@ -638,7 +638,7 @@ import sys from dataclasses import dataclass, field from typing import ( -@@ -41,13 +43,28 @@ Index = int +@@ -42,13 +44,28 @@ Index = int LeafID = int LN = Union[Leaf, Node] @@ -668,7 +668,7 @@ leaves: List[Leaf] = field(default_factory=list) # keys ordered like `leaves` comments: Dict[LeafID, List[Leaf]] = field(default_factory=dict) -@@ -56,6 +73,9 @@ class Line: +@@ -57,6 +74,9 @@ class Line: should_split_rhs: bool = False magic_trailing_comma: Optional[Leaf] = None @@ -678,7 +678,7 @@ def append( self, leaf: Leaf, preformatted: bool = False, track_bracket: bool = False ) -> None: -@@ -96,7 +116,7 @@ class Line: +@@ -97,7 +117,7 @@ class Line: Raises ValueError when any `leaf` is appended after a standalone comment or when a standalone comment is not the first leaf on the line. """ @@ -687,7 +687,7 @@ if self.is_comment: raise ValueError("cannot append to standalone comments") -@@ -266,6 +286,20 @@ class Line: +@@ -272,6 +292,20 @@ class Line: return False @@ -708,7 +708,7 @@ def contains_multiline_strings(self) -> bool: return any(is_multiline_string(leaf) for leaf in self.leaves) -@@ -433,7 +467,7 @@ class Line: +@@ -439,7 +473,7 @@ class Line: if not self: return "\n" @@ -717,7 +717,7 @@ leaves = iter(self.leaves) first = next(leaves) res = f"{first.prefix}{indent}{first.value}" -@@ -484,7 +518,7 @@ class EmptyLineTracker: +@@ -501,7 +535,7 @@ class EmptyLineTracker: mode: Mode previous_line: Optional[Line] = None previous_block: Optional[LinesBlock] = None @@ -726,7 +726,7 @@ semantic_leading_comment: Optional[LinesBlock] = None def maybe_empty_lines(self, current_line: Line) -> LinesBlock: -@@ -529,7 +563,7 @@ class EmptyLineTracker: +@@ -546,7 +580,7 @@ class EmptyLineTracker: def _maybe_empty_lines(self, current_line: Line) -> Tuple[int, int]: max_allowed = 1 @@ -735,7 +735,7 @@ max_allowed = 1 if self.mode.is_pyi else 2 if current_line.leaves: # Consume the first leaf's extra newlines. -@@ -540,7 +574,7 @@ class EmptyLineTracker: +@@ -557,7 +591,7 @@ class EmptyLineTracker: else: before = 0 depth = current_line.depth @@ -744,7 +744,7 @@ if self.mode.is_pyi: assert self.previous_line is not None if depth and not current_line.is_def and self.previous_line.is_def: -@@ -579,8 +613,9 @@ class EmptyLineTracker: +@@ -596,8 +630,9 @@ class EmptyLineTracker: if ( self.previous_line and self.previous_line.is_import @@ -756,7 +756,7 @@ ): return (before or 1), 0 -@@ -591,6 +626,14 @@ class EmptyLineTracker: +@@ -608,6 +643,14 @@ class EmptyLineTracker: ): return before, 1 @@ -771,7 +771,7 @@ if self.previous_line and self.previous_line.opens_block: return 0, 0 return before, 0 -@@ -611,15 +654,16 @@ class EmptyLineTracker: +@@ -628,15 +671,16 @@ class EmptyLineTracker: return 0, 0 @@ -791,7 +791,7 @@ and before == 0 ): slc = self.semantic_leading_comment -@@ -636,9 +680,9 @@ class EmptyLineTracker: +@@ -653,9 +697,9 @@ class EmptyLineTracker: if self.mode.is_pyi: if current_line.is_class or self.previous_line.is_class: @@ -803,7 +803,7 @@ newlines = 1 elif current_line.is_stub_class and self.previous_line.is_stub_class: # No blank line between classes with an empty body -@@ -656,7 +700,7 @@ class EmptyLineTracker: +@@ -673,7 +717,7 @@ class EmptyLineTracker: # Blank line between a block of functions (maybe with preceding # decorators) and a block of non-functions newlines = 1 @@ -812,7 +812,7 @@ newlines = 1 else: newlines = 0 -@@ -714,10 +758,14 @@ def is_line_short_enough( # noqa: C901 +@@ -731,10 +775,14 @@ def is_line_short_enough( # noqa: C901 """ if not line_str: line_str = line_to_string(line) @@ -828,7 +828,7 @@ and "\n" not in line_str # multiline strings and not line.contains_standalone_comments() ) -@@ -726,7 +774,7 @@ def is_line_short_enough( # noqa: C901 +@@ -743,7 +791,7 @@ def is_line_short_enough( # noqa: C901 return False if "\n" not in line_str: # No multiline strings (MLS) present @@ -837,7 +837,7 @@ first, *_, last = line_str.split("\n") if len(first) > mode.line_length or len(last) > mode.line_length: -@@ -899,7 +947,7 @@ def can_omit_invisible_parens( +@@ -933,7 +981,7 @@ def can_omit_invisible_parens( def _can_omit_opening_paren(line: Line, *, first: Leaf, line_length: int) -> bool: """See `can_omit_invisible_parens`.""" remainder = False @@ -846,7 +846,7 @@ _index = -1 for _index, leaf, leaf_length in line.enumerate_with_length(): if leaf.type in CLOSING_BRACKETS and leaf.opening_bracket is first: -@@ -923,7 +971,7 @@ def _can_omit_opening_paren(line: Line, +@@ -957,7 +1005,7 @@ def _can_omit_opening_paren(line: Line, def _can_omit_closing_paren(line: Line, *, last: Leaf, line_length: int) -> bool: """See `can_omit_invisible_parens`.""" diff --git a/src/pyink/linegen.py b/src/pyink/linegen.py index 84e2347946a..d5518198274 100644 --- a/src/pyink/linegen.py +++ b/src/pyink/linegen.py @@ -2,7 +2,7 @@ Generating lines of code. """ import sys -from dataclasses import dataclass, replace +from dataclasses import replace from enum import Enum, auto from functools import partial, wraps from typing import Collection, Iterator, List, Literal, Optional, Set, Union, cast @@ -17,6 +17,7 @@ from pyink.lines import ( Indentation, Line, + RHSResult, append_leaves, can_be_split, can_omit_invisible_parens, @@ -684,17 +685,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, mode: Mode, @@ -718,7 +708,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 replied upon by _maybe_split_omitting_optional_parens to get an opinion whether to prefer @@ -800,11 +790,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, mode: Mode, features: Collection[Feature] = (), @@ -824,11 +814,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, mode.line_length) + and can_omit_invisible_parens(rhs, mode.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 @@ -880,7 +870,7 @@ def _maybe_split_omitting_optional_parens( yield result -def _prefer_split_rhs_oop(rhs_oop: _RHSResult, mode: Mode) -> bool: +def _prefer_split_rhs_oop(rhs_oop: RHSResult, mode: Mode) -> bool: """ Returns whether we should prefer the result from a split omitting optional parens. """ diff --git a/src/pyink/lines.py b/src/pyink/lines.py index de0e0181c5c..a39c211681b 100644 --- a/src/pyink/lines.py +++ b/src/pyink/lines.py @@ -17,7 +17,7 @@ cast, ) -from pyink.brackets import DOT_PRIORITY, BracketTracker +from pyink.brackets import COMMA_PRIORITY, DOT_PRIORITY, BracketTracker from pyink.mode import Mode, Preview from pyink.nodes import ( BRACKETS, @@ -30,6 +30,7 @@ is_multiline_string, is_one_sequence_between, is_type_comment, + is_with_stmt, replace_child, syms, whitespace, @@ -142,6 +143,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 a 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?""" @@ -483,6 +489,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. @@ -878,25 +895,42 @@ 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 max_priority == COMMA_PRIORITY + 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/pyink/nodes.py b/src/pyink/nodes.py index fd89246616d..1256d890b8f 100644 --- a/src/pyink/nodes.py +++ b/src/pyink/nodes.py @@ -799,6 +799,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..643c6fd958b 100644 --- a/tests/data/preview_context_managers/targeting_py39.py +++ b/tests/data/preview_context_managers/targeting_py39.py @@ -49,6 +49,24 @@ 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 + + +with xxxxxxxx.some_kind_of_method( + some_argument=[ + "first", + "second", + "third", + ] +).another_method() as cmd: + pass + + # output @@ -102,3 +120,22 @@ ) 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 + + +with xxxxxxxx.some_kind_of_method( + some_argument=[ + "first", + "second", + "third", + ] +).another_method() as cmd: + pass