From cab3a507bcf3e40f8b493b48f875498348de681b Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 10 Jul 2023 15:55:19 +0200 Subject: [PATCH] Fix find_only_token_in_range with expression parentheses (#5645) ## Summary Fix an oversight in `find_only_token_in_range` where the following code would panic due do the closing and opening parentheses being in the range we scan: ```python d1 = [ ("a") if # 1 ("b") else # 2 ("c") ] ``` Closing and opening parentheses respectively are now correctly skipped. ## Test Plan I added a regression test --- .../test/fixtures/ruff/expression/if.py | 8 ++++++++ .../src/comments/placement.rs | 8 ++++++-- .../snapshots/format@expression__if.py.snap | 18 ++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/if.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/if.py index ed8c1ab97b..df4c488603 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/if.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/if.py @@ -31,3 +31,11 @@ c2 = ( # 8 "b" # 9 ) + +# regression test: parentheses outside the expression ranges interfering with finding +# the `if` and `else` token finding +d1 = [ + ("a") if # 1 + ("b") else # 2 + ("c") +] diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 89dc52896b..85b6534f49 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1215,11 +1215,15 @@ fn handle_expr_if_comment<'a>( CommentPlacement::Default(comment) } -/// Looks for a token in the range that contains no other tokens. +/// Looks for a token in the range that contains no other tokens except for parentheses outside +/// the expression ranges fn find_only_token_in_range(range: TextRange, locator: &Locator, token_kind: TokenKind) -> Token { - let mut tokens = SimpleTokenizer::new(locator.contents(), range).skip_trivia(); + let mut tokens = SimpleTokenizer::new(locator.contents(), range) + .skip_trivia() + .skip_while(|token| token.kind == TokenKind::RParen); let token = tokens.next().expect("Expected a token"); debug_assert_eq!(token.kind(), token_kind); + let mut tokens = tokens.skip_while(|token| token.kind == TokenKind::LParen); debug_assert_eq!(tokens.next(), None); token } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__if.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__if.py.snap index 3b563f0454..f5a195c622 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__if.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__if.py.snap @@ -37,6 +37,14 @@ c2 = ( # 8 "b" # 9 ) + +# regression test: parentheses outside the expression ranges interfering with finding +# the `if` and `else` token finding +d1 = [ + ("a") if # 1 + ("b") else # 2 + ("c") +] ``` ## Output @@ -78,6 +86,16 @@ c2 = ( # 8 else "b" # 9 ) + +# regression test: parentheses outside the expression ranges interfering with finding +# the `if` and `else` token finding +d1 = [ + ("a") + # 1 + if ("b") + # 2 + else ("c") +] ```