From d407165aa766252ee9b7663ef457611f5154accc Mon Sep 17 00:00:00 2001 From: konstin Date: Thu, 22 Jun 2023 16:52:48 +0200 Subject: [PATCH] Fix formatter panic with comment after parenthesized dict value (#5293) ## Summary This snippet used to panic because it expected to see a comma or something similar after the `2` but met the closing parentheses that is not part of the range and panicked ```python a = { 1: (2), # comment 3: True, } ``` Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109 This snippet is also the test plan. --- .../test/fixtures/ruff/expression/dict.py | 8 +++++++ .../src/comments/placement.rs | 24 ++++++++++++------- ...tests__ruff_test__expression__dict_py.snap | 16 +++++++++++++ 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py index f8cf5a93512ed..a0631959db36a 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/dict.py @@ -48,3 +48,11 @@ C: 0.1 * (10.0 / 12), D: 0.1 * (10.0 / 12), } + +# Regression test for formatter panic with comment after parenthesized dict value +# Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109 +a = { + 1: (2), + # comment + 3: True, +} diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index b118a80ef8549..e1cded005f0e4 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -991,14 +991,22 @@ fn handle_dict_unpacking_comment<'a>( .skip_trivia(); // we start from the preceding node but we skip its token - if let Some(first) = tokens.next() { - debug_assert!(matches!( - first, - Token { - kind: TokenKind::LBrace | TokenKind::Comma | TokenKind::Colon, - .. - } - )); + for token in tokens.by_ref() { + // Skip closing parentheses that are not part of the node range + if token.kind == TokenKind::RParen { + continue; + } + debug_assert!( + matches!( + token, + Token { + kind: TokenKind::LBrace | TokenKind::Comma | TokenKind::Colon, + .. + } + ), + "{token:?}", + ); + break; } // if the remaining tokens from the previous node is exactly `**`, diff --git a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__dict_py.snap b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__dict_py.snap index 2c0cba1259e23..d59022c544432 100644 --- a/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__dict_py.snap +++ b/crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__expression__dict_py.snap @@ -54,6 +54,14 @@ mapping = { C: 0.1 * (10.0 / 12), D: 0.1 * (10.0 / 12), } + +# Regression test for formatter panic with comment after parenthesized dict value +# Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109 +a = { + 1: (2), + # comment + 3: True, +} ``` @@ -112,6 +120,14 @@ mapping = { C: 0.1 * (10.0 / 12), D: 0.1 * (10.0 / 12), } + +# Regression test for formatter panic with comment after parenthesized dict value +# Originally found in https://github.com/bolucat/Firefox/blob/636a717ef025c16434997dc89e42351ef740ee6b/testing/marionette/client/marionette_driver/geckoinstance.py#L109 +a = { + 1: (2), + # comment + 3: True, +} ```