From 6f694a948743fc77b1dd50d0a56daa6ba4a3e2ca Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 23 Jun 2023 11:21:45 +0200 Subject: [PATCH 1/2] Fix attribute chain own line comments --- .../fixtures/ruff/expression/attribute.py | 60 ++++++++++++ .../src/expression/expr_attribute.rs | 47 +++++++-- .../format@expression__attribute.py.snap | 98 +++++++++++++++++++ 3 files changed, 196 insertions(+), 9 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py index bdc18f4696c67..8e339f08b11b7 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py @@ -27,3 +27,63 @@ aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr + + +# Test that we add parentheses around the outermost attribute access in an attribute +# chain if and only if we need them, that is if there are own line comments inside +# the chain. +x1 = ( + a1 + .a2 + # a + . # b + # c + a3 + .a4 +) +x20 = ( + a + .b +) +x21 = ( + a + # trailing name own line + .b +) +x22 = ( + "" + # leading + .b +) +x23 = ( + # leading + a + .b +) +x24 = ( + a # trailing name end-of-line + .b +) +x31 = ( + a + .# a + b +) +x32 = ( + a + .# a + b + .c +) +x4 = ( + a. + # a + b +) +x5 = ( + a.b.c +) +x61 = askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x62 = ( + askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +) diff --git a/crates/ruff_python_formatter/src/expression/expr_attribute.rs b/crates/ruff_python_formatter/src/expression/expr_attribute.rs index fd43e9f20a56e..16ccaa330c318 100644 --- a/crates/ruff_python_formatter/src/expression/expr_attribute.rs +++ b/crates/ruff_python_formatter/src/expression/expr_attribute.rs @@ -27,25 +27,28 @@ impl FormatNodeRule for FormatExprAttribute { }) ); + let comments = f.context().comments().clone(); + let dangling_comments = comments.dangling_comments(item); + let leading_attribute_comments_start = + dangling_comments.partition_point(|comment| comment.line_position().is_end_of_line()); + let (trailing_dot_comments, leading_attribute_comments) = + dangling_comments.split_at(leading_attribute_comments_start); + if needs_parentheses { value.format().with_options(Parenthesize::Always).fmt(f)?; + } else if let Expr::Attribute(expr_attribute) = value.as_ref() { + // We're in a attribute chain (`a.b.c`). The outermost node adds parentheses if + // required, the inner ones don't need them so we skip the `Expr` formatting that + // normally adds the parentheses. + expr_attribute.format().fmt(f)?; } else { value.format().fmt(f)?; } - let comments = f.context().comments().clone(); - if comments.has_trailing_own_line_comments(value.as_ref()) { hard_line_break().fmt(f)?; } - let dangling_comments = comments.dangling_comments(item); - - let leading_attribute_comments_start = - dangling_comments.partition_point(|comment| comment.line_position().is_end_of_line()); - let (trailing_dot_comments, leading_attribute_comments) = - dangling_comments.split_at(leading_attribute_comments_start); - write!( f, [ @@ -68,6 +71,28 @@ impl FormatNodeRule for FormatExprAttribute { } } +/// Checks if there are any own line comments in an attribute chain (a.b.c). This method is +/// recursive up to the innermost expression that the attribute chain starts behind. +fn has_breaking_comments_attribute_chain( + expr_attribute: &ExprAttribute, + comments: &Comments, +) -> bool { + if comments + .dangling_comments(expr_attribute) + .iter() + .any(|comment| comment.line_position().is_own_line()) + || comments.has_trailing_own_line_comments(expr_attribute) + { + return true; + } + + if let Expr::Attribute(inner) = expr_attribute.value.as_ref() { + return has_breaking_comments_attribute_chain(inner, comments); + } + + return comments.has_trailing_own_line_comments(expr_attribute.value.as_ref()); +} + impl NeedsParentheses for ExprAttribute { fn needs_parentheses( &self, @@ -75,6 +100,10 @@ impl NeedsParentheses for ExprAttribute { source: &str, comments: &Comments, ) -> Parentheses { + if has_breaking_comments_attribute_chain(self, comments) { + return Parentheses::Always; + } + match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) { Parentheses::Optional => Parentheses::Never, parentheses => parentheses, diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap index 82792c7d3c9c7..b20f7e94e46ea 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap @@ -33,6 +33,66 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr + + +# Test that we add parentheses around the outermost attribute access in an attribute +# chain if and only if we need them, that is if there are own line comments inside +# the chain. +x1 = ( + a1 + .a2 + # a + . # b + # c + a3 + .a4 +) +x20 = ( + a + .b +) +x21 = ( + a + # trailing name own line + .b +) +x22 = ( + "" + # leading + .b +) +x23 = ( + # leading + a + .b +) +x24 = ( + a # trailing name end-of-line + .b +) +x31 = ( + a + .# a + b +) +x32 = ( + a + .# a + b + .c +) +x4 = ( + a. + # a + b +) +x5 = ( + a.b.c +) +x61 = askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x62 = ( + askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +) ``` @@ -68,6 +128,44 @@ aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr + + +# Test that we add parentheses around the outermost attribute access in an attribute +# chain if and only if we need them, that is if there are own line comments inside +# the chain. +x1 = ( + a1.a2 + # a + . # b + # c + a3.a4 +) +x20 = a.b +x21 = ( + a + # trailing name own line + .b +) +x22 = ( + "" + # leading + .b +) +x23 = ( + # leading + a.b +) +x24 = a.b # trailing name end-of-line +x31 = a.b # a +x32 = a.b.c # a +x4 = ( + a. + # a + b +) +x5 = a.b.c +x61 = askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x62 = askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs ``` From 8bc2b89b2fe1712e4bd5fefc4a6ae47ce48ff2bb Mon Sep 17 00:00:00 2001 From: konstin Date: Mon, 26 Jun 2023 11:03:34 +0200 Subject: [PATCH 2/2] Better test cases and after dot assignment --- .../fixtures/ruff/expression/attribute.py | 72 ++++++---- .../src/comments/placement.rs | 14 +- .../format@expression__attribute.py.snap | 135 +++++++++++------- 3 files changed, 137 insertions(+), 84 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py index 8e339f08b11b7..24bea5ca42fb7 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py @@ -1,3 +1,7 @@ +from argparse import Namespace + +a = Namespace() + ( a # comment @@ -26,64 +30,74 @@ ) -aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr +a.aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr # Test that we add parentheses around the outermost attribute access in an attribute # chain if and only if we need them, that is if there are own line comments inside # the chain. x1 = ( - a1 - .a2 - # a - . # b - # c - a3 - .a4 + a + .b + # comment 1 + . # comment 2 + # comment 3 + c + .d ) + x20 = ( a .b ) x21 = ( - a - # trailing name own line + # leading name own line + a # trailing name end-of-line .b ) x22 = ( - "" - # leading - .b -) -x23 = ( - # leading a - .b + # outermost leading own line + .b # outermost trailing end-of-line ) -x24 = ( - a # trailing name end-of-line + +x31 = ( + a + # own line between nodes 1 .b ) -x31 = ( +x321 = ( a - .# a + . # end-of-line dot comment b ) -x32 = ( +x322 = ( a - .# a + . # end-of-line dot comment 2 b .c ) -x4 = ( +x331 = ( a. - # a + # own line between nodes 3 b ) -x5 = ( +x332 = ( + "" + # own line between nodes + .find +) + +x8 = ( + (a + a) + .b +) + +x51 = ( a.b.c ) -x61 = askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs -x62 = ( - askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x52 = a.askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x53 = ( + a.askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs ) + diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index ca0fec261e75f..0e801c7096099 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1077,7 +1077,19 @@ fn handle_attribute_comment<'a>( .expect("Expected the `.` character after the value"); if TextRange::new(dot.end(), attribute.attr.start()).contains(comment.slice().start()) { - CommentPlacement::dangling(attribute.into(), comment) + if comment.line_position().is_end_of_line() { + // Attach to node with b + // ```python + // x322 = ( + // a + // . # end-of-line dot comment 2 + // b + // ) + // ``` + CommentPlacement::trailing(comment.enclosing_node(), comment) + } else { + CommentPlacement::dangling(attribute.into(), comment) + } } else { CommentPlacement::Default(comment) } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap index b20f7e94e46ea..a1871573c8d6e 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__attribute.py.snap @@ -4,6 +4,10 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression --- ## Input ```py +from argparse import Namespace + +a = Namespace() + ( a # comment @@ -32,73 +36,87 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression ) -aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr +a.aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr # Test that we add parentheses around the outermost attribute access in an attribute # chain if and only if we need them, that is if there are own line comments inside # the chain. x1 = ( - a1 - .a2 - # a - . # b - # c - a3 - .a4 + a + .b + # comment 1 + . # comment 2 + # comment 3 + c + .d ) + x20 = ( a .b ) x21 = ( - a - # trailing name own line + # leading name own line + a # trailing name end-of-line .b ) x22 = ( - "" - # leading - .b -) -x23 = ( - # leading a - .b + # outermost leading own line + .b # outermost trailing end-of-line ) -x24 = ( - a # trailing name end-of-line + +x31 = ( + a + # own line between nodes 1 .b ) -x31 = ( +x321 = ( a - .# a + . # end-of-line dot comment b ) -x32 = ( +x322 = ( a - .# a + . # end-of-line dot comment 2 b .c ) -x4 = ( +x331 = ( a. - # a + # own line between nodes 3 b ) -x5 = ( +x332 = ( + "" + # own line between nodes + .find +) + +x8 = ( + (a + a) + .b +) + +x51 = ( a.b.c ) -x61 = askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs -x62 = ( - askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x52 = a.askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x53 = ( + a.askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs ) + ``` ## Output ```py +NOT_YET_IMPLEMENTED_StmtImportFrom + +a = NOT_IMPLEMENTED_call() + ( a # comment @@ -121,51 +139,60 @@ x62 = ( ( a # comment - . # trailing dot comment + . # in between - b # trailing identifier comment + b # trailing dot comment # trailing identifier comment ) -aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr +a.aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr # Test that we add parentheses around the outermost attribute access in an attribute # chain if and only if we need them, that is if there are own line comments inside # the chain. x1 = ( - a1.a2 - # a - . # b - # c - a3.a4 + a.b + # comment 1 + . + # comment 3 + c.d # comment 2 ) + x20 = a.b x21 = ( - a - # trailing name own line - .b + # leading name own line + a.b # trailing name end-of-line ) x22 = ( - "" - # leading - .b + a + # outermost leading own line + .b # outermost trailing end-of-line ) -x23 = ( - # leading - a.b + +x31 = ( + a + # own line between nodes 1 + .b ) -x24 = a.b # trailing name end-of-line -x31 = a.b # a -x32 = a.b.c # a -x4 = ( +x321 = a.b # end-of-line dot comment +x322 = a.b.c # end-of-line dot comment 2 +x331 = ( a. - # a + # own line between nodes 3 b ) -x5 = a.b.c -x61 = askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs -x62 = askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x332 = ( + "" + # own line between nodes + .find +) + +x8 = (a + a).b + +x51 = a.b.c +x52 = a.askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs +x53 = a.askjdfahdlskjflsajfadhsaf.akjdsf.aksjdlfadhaljsashdfljaf.askjdflhasfdlashdlfaskjfd.asdkjfksahdfkjafs ```