From 35bdcfe50d498c6c64a8426eca095d88da96d427 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 22 Sep 2022 16:54:06 -0700 Subject: [PATCH 1/5] Fix a crash when `# fmt: on` is used on a different block level as `# fmt: off` --- CHANGES.md | 3 + src/black/comments.py | 32 +++---- src/black/nodes.py | 14 +-- tests/data/simple_cases/fmtonoff5.py | 122 +++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 22 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 147100c3012..5f3a79c62b7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,9 @@ +- Fix a crash when `# fmt: on` is used on a different block level as `# fmt: off` + (#3281) + ### Preview style diff --git a/src/black/comments.py b/src/black/comments.py index dc58934f9d3..b7a3ae777f5 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -14,11 +14,12 @@ STANDALONE_COMMENT, WHITESPACE, container_of, - first_leaf_column, + first_leaf_of, preceding_leaf, + syms, ) from blib2to3.pgen2 import token -from blib2to3.pytree import Leaf, Node, type_repr +from blib2to3.pytree import Leaf, Node # types LN = Union[Leaf, Node] @@ -230,7 +231,7 @@ def generate_ignored_nodes( return # fix for fmt: on in children - if contains_fmt_on_at_column(container, leaf.column, preview=preview): + if children_contains_fmt_on(container, preview=preview): for child in container.children: if isinstance(child, Leaf) and is_fmt_on(child, preview=preview): if child.type in CLOSING_BRACKETS: @@ -240,10 +241,14 @@ def generate_ignored_nodes( # The alternative is to fail the formatting. yield child return - if contains_fmt_on_at_column(child, leaf.column, preview=preview): + if children_contains_fmt_on(child, preview=preview): return yield child else: + if container.type == token.DEDENT and container.next_sibling is None: + # This can happen when there is no matching `# fmt: on` comment at the + # same level as `# fmt: on`. We need to keep this DEDENT. + return yield container container = container.next_sibling @@ -269,7 +274,7 @@ def _generate_ignored_nodes_from_fmt_skip( yield sibling elif ( parent is not None - and type_repr(parent.type) == "suite" + and parent.type == syms.suite and leaf.type == token.NEWLINE ): # The `# fmt: skip` is on the colon line of the if/while/def/class/... @@ -278,7 +283,7 @@ def _generate_ignored_nodes_from_fmt_skip( leaf.prefix = "" ignored_nodes: List[LN] = [] parent_sibling = parent.prev_sibling - while parent_sibling is not None and type_repr(parent_sibling.type) != "suite": + while parent_sibling is not None and parent_sibling.type != syms.suite: ignored_nodes.insert(0, parent_sibling) parent_sibling = parent_sibling.prev_sibling # Special case for `async_stmt` where the ASYNC token is on the @@ -306,17 +311,12 @@ def is_fmt_on(container: LN, preview: bool) -> bool: return fmt_on -def contains_fmt_on_at_column(container: LN, column: int, *, preview: bool) -> bool: - """Determine if children at a given column have formatting switched on.""" +def children_contains_fmt_on(container: LN, *, preview: bool) -> bool: + """Determine if children have formatting switched on.""" for child in container.children: - if ( - isinstance(child, Node) - and first_leaf_column(child) == column - or isinstance(child, Leaf) - and child.column == column - ): - if is_fmt_on(child, preview=preview): - return True + leaf = first_leaf_of(child) + if leaf is not None and is_fmt_on(leaf, preview=preview): + return True return False diff --git a/src/black/nodes.py b/src/black/nodes.py index 8f341ab35d6..aeb2be389c8 100644 --- a/src/black/nodes.py +++ b/src/black/nodes.py @@ -502,12 +502,14 @@ def container_of(leaf: Leaf) -> LN: return container -def first_leaf_column(node: Node) -> Optional[int]: - """Returns the column of the first leaf child of a node.""" - for child in node.children: - if isinstance(child, Leaf): - return child.column - return None +def first_leaf_of(node: LN) -> Optional[Leaf]: + """Returns the first leaf of the node tree.""" + if isinstance(node, Leaf): + return node + if node.children: + return first_leaf_of(node.children[0]) + else: + return None def is_arith_like(node: LN) -> bool: diff --git a/tests/data/simple_cases/fmtonoff5.py b/tests/data/simple_cases/fmtonoff5.py index 746aa41f4e4..d16d24dd936 100644 --- a/tests/data/simple_cases/fmtonoff5.py +++ b/tests/data/simple_cases/fmtonoff5.py @@ -34,3 +34,125 @@ def test_func(): return True return False + + +# Regression test for https://github.com/psf/black/issues/2567. +if True: + # fmt: off + for _ in range(1): + # fmt: on + print ( "This won't be formatted" ) + print ( "This won't be formatted either" ) +else: + print ( "This will be formatted" ) + + +# Regression test for https://github.com/psf/black/issues/3184. +class A: + async def call(param): + if param: + # fmt: off + if param[0:4] in ( + "ABCD", "EFGH" + ) : + # fmt: on + print ( "This won't be formatted" ) + + elif param[0:4] in ("ZZZZ",): + print ( "This won't be formatted either" ) + + print ( "This will be formatted" ) + + +# Regression test for https://github.com/psf/black/issues/2985 +class Named(t.Protocol): + # fmt: off + @property + def this_wont_be_formatted ( self ) -> str: ... + +class Factory(t.Protocol): + def this_will_be_formatted ( self, **kwargs ) -> Named: ... + # fmt: on + + +# output + + +# Regression test for https://github.com/psf/black/issues/3129. +setup( + entry_points={ + # fmt: off + "console_scripts": [ + "foo-bar" + "=foo.bar.:main", + # fmt: on + ] # Includes an formatted indentation. + }, +) + + +# Regression test for https://github.com/psf/black/issues/2015. +run( + # fmt: off + [ + "ls", + "-la", + ] + # fmt: on + + path, + check=True, +) + + +# Regression test for https://github.com/psf/black/issues/3026. +def test_func(): + # yapf: disable + if unformatted( args ): + return True + # yapf: enable + elif b: + return True + + return False + + +# Regression test for https://github.com/psf/black/issues/2567. +if True: + # fmt: off + for _ in range(1): + # fmt: on + print ( "This won't be formatted" ) + print ( "This won't be formatted either" ) +else: + print("This will be formatted") + + +# Regression test for https://github.com/psf/black/issues/3184. +class A: + async def call(param): + if param: + # fmt: off + if param[0:4] in ( + "ABCD", "EFGH" + ) : + # fmt: on + print ( "This won't be formatted" ) + + elif param[0:4] in ("ZZZZ",): + print ( "This won't be formatted either" ) + + print("This will be formatted") + + +# Regression test for https://github.com/psf/black/issues/2985 +class Named(t.Protocol): + # fmt: off + @property + def this_wont_be_formatted ( self ) -> str: ... + + +class Factory(t.Protocol): + def this_will_be_formatted(self, **kwargs) -> Named: + ... + + # fmt: on From 0b2da3d212c930915775a82ee86c3f3dafedde3e Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 22 Sep 2022 17:07:18 -0700 Subject: [PATCH 2/5] Format file. --- src/black/comments.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index b7a3ae777f5..2a4c254ecd9 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -273,9 +273,7 @@ def _generate_ignored_nodes_from_fmt_skip( for sibling in siblings: yield sibling elif ( - parent is not None - and parent.type == syms.suite - and leaf.type == token.NEWLINE + parent is not None and parent.type == syms.suite and leaf.type == token.NEWLINE ): # The `# fmt: skip` is on the colon line of the if/while/def/class/... # statements. The ignored nodes should be previous siblings of the From 8dec50e418a7e543a70e0e2f44dde74355e7494b Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 22 Sep 2022 17:09:46 -0700 Subject: [PATCH 3/5] Update docs. --- docs/contributing/reference/reference_functions.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/contributing/reference/reference_functions.rst b/docs/contributing/reference/reference_functions.rst index 50eaeb31e15..3bda5de1774 100644 --- a/docs/contributing/reference/reference_functions.rst +++ b/docs/contributing/reference/reference_functions.rst @@ -137,9 +137,9 @@ Utilities .. autofunction:: black.comments.is_fmt_on -.. autofunction:: black.comments.contains_fmt_on_at_column +.. autofunction:: black.comments.children_contains_fmt_on -.. autofunction:: black.nodes.first_leaf_column +.. autofunction:: black.nodes.first_leaf_of .. autofunction:: black.linegen.generate_trailers_to_omit From d4d5fa0345a09acedaea2627bc9609390c02f9ed Mon Sep 17 00:00:00 2001 From: "Yilei \"Dolee\" Yang" Date: Fri, 23 Sep 2022 08:10:33 -0700 Subject: [PATCH 4/5] 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 5f3a79c62b7..a6de979a48e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,7 +10,7 @@ -- Fix a crash when `# fmt: on` is used on a different block level as `# fmt: off` +- Fix a crash when `# fmt: on` is used on a different block level than `# fmt: off` (#3281) ### Preview style From 46777267f0f11d8cb3c767afc948301d83cd10c2 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Fri, 23 Sep 2022 08:12:20 -0700 Subject: [PATCH 5/5] Update test to ensure this is not formatted. --- tests/data/simple_cases/fmtonoff5.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/data/simple_cases/fmtonoff5.py b/tests/data/simple_cases/fmtonoff5.py index d16d24dd936..71b1381ed0d 100644 --- a/tests/data/simple_cases/fmtonoff5.py +++ b/tests/data/simple_cases/fmtonoff5.py @@ -39,7 +39,7 @@ def test_func(): # Regression test for https://github.com/psf/black/issues/2567. if True: # fmt: off - for _ in range(1): + for _ in range( 1 ): # fmt: on print ( "This won't be formatted" ) print ( "This won't be formatted either" ) @@ -119,7 +119,7 @@ def test_func(): # Regression test for https://github.com/psf/black/issues/2567. if True: # fmt: off - for _ in range(1): + for _ in range( 1 ): # fmt: on print ( "This won't be formatted" ) print ( "This won't be formatted either" )