Format Attribute Expression (#5259)

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py

@@ -0,0 +1,29 @@
+(
+ a
+ # comment
+ .b # trailing comment
+)
+
+(
+ a
+ # comment
+ .b # trailing dot comment # trailing identifier comment
+)
+
+(
+ a
+ # comment
+ .b # trailing identifier comment
+)
+
+
+(
+ a
+ # comment
+ . # trailing dot comment
+ # in between
+ b # trailing identifier comment
+)
+
+
+aaaaaaaaaaaaaaaaaaaaa.lllllllllllllllllllllllllllloooooooooong.chaaaaaaaaaaaaaaaaaaaaaaiiiiiiiiiiiiiiiiiiiiiiinnnnnnnn.ooooooooooooooooooooooooofffffffff.aaaaaaaaaattr

crates/ruff_python_formatter/src/comments/placement.rs

@@ -31,6 +31,7 @@ pub(super) fn place_comment<'a>(
  handle_leading_function_with_decorators_comment,
  handle_dict_unpacking_comment,
  handle_slice_comments,
+ handle_attribute_comment,
  ];
  for handler in HANDLERS {
  comment = match handler(comment, locator) {
@@ -1005,6 +1006,43 @@ fn handle_dict_unpacking_comment<'a>(
  CommentPlacement::Default(comment)
 }
 
+// Own line comments coming after the node are always dangling comments
+// ```python
+// (
+// a
+// # trailing a comment
+// . # dangling comment
+// # or this
+// b
+// )
+// ```
+fn handle_attribute_comment<'a>(
+ comment: DecoratedComment<'a>,
+ locator: &Locator,
+) -> CommentPlacement<'a> {
+ let Some(attribute) = comment.enclosing_node().expr_attribute() else {
+ return CommentPlacement::Default(comment);
+ };
+
+ // It must be a comment AFTER the name
+ if comment.preceding_node().is_none() {
+ return CommentPlacement::Default(comment);
+ }
+
+ let between_value_and_attr = TextRange::new(attribute.value.end(), attribute.attr.start());
+
+ let dot = SimpleTokenizer::new(locator.contents(), between_value_and_attr)
+ .skip_trivia()
+ .next()
+ .expect("Expected the `.` character after the value");
+
+ if TextRange::new(dot.end(), attribute.attr.start()).contains(comment.slice().start()) {
+ CommentPlacement::dangling(attribute.into(), comment)
+ } else {
+ CommentPlacement::Default(comment)
+ }
+}
+
 /// Returns `true` if `right` is `Some` and `left` and `right` are referentially equal.
 fn are_same_optional<'a, T>(left: AnyNodeRef, right: Option<T>) -> bool
 where

crates/ruff_python_formatter/src/expression/expr_attribute.rs

@@ -1,9 +1,9 @@
-use crate::comments::Comments;
+use crate::comments::{leading_comments, trailing_comments, Comments};
 use crate::expression::parentheses::{
  default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
 };
 use crate::prelude::*;
-use crate::{not_yet_implemented_custom_text, FormatNodeRule};
+use crate::FormatNodeRule;
 use ruff_formatter::write;
 use rustpython_parser::ast::{Constant, Expr, ExprAttribute, ExprConstant};
 
@@ -15,28 +15,57 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
  let ExprAttribute {
  value,
  range: _,
- attr: _,
+ attr,
  ctx: _,
  } = item;
 
- let requires_space = matches!(
+ let needs_parentheses = matches!(
  value.as_ref(),
  Expr::Constant(ExprConstant {
  value: Constant::Int(_) | Constant::Float(_),
  ..
  })
  );
 
+ if needs_parentheses {
+ value.format().with_options(Parenthesize::Always).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,
  [
- item.value.format(),
- requires_space.then_some(space()),
  text("."),
- not_yet_implemented_custom_text("NOT_IMPLEMENTED_attr")
+ trailing_comments(trailing_dot_comments),
+ (!leading_attribute_comments.is_empty()).then_some(hard_line_break()),
+ leading_comments(leading_attribute_comments),
+ attr.format()
  ]
  )
  }
+
+ fn fmt_dangling_comments(
+ &self,
+ _node: &ExprAttribute,
+ _f: &mut PyFormatter,
+ ) -> FormatResult<()> {
+ // handle in `fmt_fields`
+ Ok(())
+ }
 }
 
 impl NeedsParentheses for ExprAttribute {
@@ -46,6 +75,9 @@ impl NeedsParentheses for ExprAttribute {
  source: &str,
  comments: &Comments,
  ) -> Parentheses {
- default_expression_needs_parentheses(self.into(), parenthesize, source, comments)
+ match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
+ Parentheses::Optional => Parentheses::Never,
+ parentheses => parentheses,
+ }
  }
 }

crates/ruff_python_formatter/src/expression/parentheses.rs

@@ -23,8 +23,12 @@ pub(super) fn default_expression_needs_parentheses(
  "Should only be called for expressions"
  );
 
+ #[allow(clippy::if_same_then_else)]
+ if parenthesize.is_always() {
+ Parentheses::Always
+ }
  // `Optional` or `Preserve` and expression has parentheses in source code.
- if !parenthesize.is_if_breaks() && is_expression_parenthesized(node, source) {
+ else if !parenthesize.is_if_breaks() && is_expression_parenthesized(node, source) {
  Parentheses::Always
  }
  // `Optional` or `IfBreaks`: Add parentheses if the expression doesn't fit on a line but enforce
@@ -53,9 +57,15 @@ pub enum Parenthesize {
 
  /// Parenthesizes the expression only if it doesn't fit on a line.
  IfBreaks,
+
+ Always,
 }
 
 impl Parenthesize {
+ pub(crate) const fn is_always(self) -> bool {
+ matches!(self, Parenthesize::Always)
+ }
+
  pub(crate) const fn is_if_breaks(self) -> bool {
  matches!(self, Parenthesize::IfBreaks)
  }
@@ -70,7 +80,8 @@ impl Parenthesize {
 /// whether there are parentheses in the source code or not.
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum Parentheses {
- /// Always create parentheses
+ /// Always set parentheses regardless if the expression breaks or if they were
+ /// present in the source.
  Always,
 
  /// Only add parentheses when necessary because the expression breaks over multiple lines.

crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__attribute_access_on_number_literals_py.snap

@@ -55,22 +55,21 @@ y = 100(no)
 +x = NOT_IMPLEMENTED_call()
 +x = NOT_IMPLEMENTED_call()
 +x = NOT_IMPLEMENTED_call()
-+x = 1. .NOT_IMPLEMENTED_attr
-+x = 1E+1 .NOT_IMPLEMENTED_attr
-+x = 1E-1 .NOT_IMPLEMENTED_attr
++x = (1.).imag
++x = (1E+1).imag
++x = (1E-1).real
 +x = NOT_IMPLEMENTED_call()
-+x = 123456789.123456789E123456789 .NOT_IMPLEMENTED_attr
++x = (123456789.123456789E123456789).real
 +x = NOT_IMPLEMENTED_call()
-+x = 123456789J.NOT_IMPLEMENTED_attr
++x = 123456789J.real
 +x = NOT_IMPLEMENTED_call()
 +x = NOT_IMPLEMENTED_call()
 +x = NOT_IMPLEMENTED_call()
-+x = 0O777 .NOT_IMPLEMENTED_attr
++x = (0O777).real
 +x = NOT_IMPLEMENTED_call()
 +x = -100.0000J
  
--if (10).real:
-+if 10 .NOT_IMPLEMENTED_attr:
+ if (10).real:
  ...
  
  y = 100[no]
@@ -84,21 +83,21 @@ y = 100(no)
 x = NOT_IMPLEMENTED_call()
 x = NOT_IMPLEMENTED_call()
 x = NOT_IMPLEMENTED_call()
-x = 1. .NOT_IMPLEMENTED_attr
-x = 1E+1 .NOT_IMPLEMENTED_attr
-x = 1E-1 .NOT_IMPLEMENTED_attr
+x = (1.).imag
+x = (1E+1).imag
+x = (1E-1).real
 x = NOT_IMPLEMENTED_call()
-x = 123456789.123456789E123456789 .NOT_IMPLEMENTED_attr
+x = (123456789.123456789E123456789).real
 x = NOT_IMPLEMENTED_call()
-x = 123456789J.NOT_IMPLEMENTED_attr
+x = 123456789J.real
 x = NOT_IMPLEMENTED_call()
 x = NOT_IMPLEMENTED_call()
 x = NOT_IMPLEMENTED_call()
-x = 0O777 .NOT_IMPLEMENTED_attr
+x = (0O777).real
 x = NOT_IMPLEMENTED_call()
 x = -100.0000J
 
-if 10 .NOT_IMPLEMENTED_attr:
+if (10).real:
  ...
 
 y = 100[no]

crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__collections_py.snap

@@ -165,7 +165,7 @@ if True:
  
  # looping over a 1-tuple should also not get wrapped
  for x in (1,):
-@@ -63,37 +42,17 @@
+@@ -63,14 +42,10 @@
  for (x,) in (1,), (2,), (3,):
  pass
  
@@ -181,13 +181,9 @@ if True:
 +NOT_IMPLEMENTED_call()
  
  if True:
-- IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = (
-- Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING
-- | {pylons.controllers.WSGIController}
-- )
-+ IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = Config.NOT_IMPLEMENTED_attr | {
-+ pylons.NOT_IMPLEMENTED_attr.NOT_IMPLEMENTED_attr,
-+ }
+ IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = (
+@@ -79,21 +54,6 @@
+ )
  
  if True:
 - ec2client.get_waiter("instance_stopped").wait(
@@ -266,9 +262,10 @@ division_result_tuple = (6 / 2,)
 NOT_IMPLEMENTED_call()
 
 if True:
- IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = Config.NOT_IMPLEMENTED_attr | {
- pylons.NOT_IMPLEMENTED_attr.NOT_IMPLEMENTED_attr,
- }
+ IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING = (
+ Config.IGNORED_TYPES_FOR_ATTRIBUTE_CHECKING
+ | {pylons.controllers.WSGIController}
+ )
 
 if True:
  NOT_IMPLEMENTED_call()

crates/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__black_test__comments2_py.snap

@@ -227,7 +227,7 @@ instruction()#comment with bad spacing
  ]
  
  not_shareables = [
-@@ -37,49 +33,57 @@
+@@ -37,31 +33,35 @@
  # builtin types and objects
  type,
  object,
@@ -263,31 +263,24 @@ instruction()#comment with bad spacing
  def inline_comments_in_brackets_ruin_everything():
  if typedargslist:
 - parameters.children = [children[0], body, children[-1]] # (1 # )1
-- parameters.children = [
-+ parameters.NOT_IMPLEMENTED_attr = [
+ parameters.children = [
 + children[0], # (1
 + body,
 + children[-1], # )1
 + ]
-+ parameters.NOT_IMPLEMENTED_attr = [
++ parameters.children = [
  children[0],
  body,
  children[-1], # type: ignore
- ]
- else:
-- parameters.children = [
-- parameters.children[0], # (2 what if this was actually long
-+ parameters.NOT_IMPLEMENTED_attr = [
-+ parameters.NOT_IMPLEMENTED_attr[0], # (2 what if this was actually long
+@@ -72,14 +72,18 @@
  body,
-- parameters.children[-1], # )2
-+ parameters.NOT_IMPLEMENTED_attr[-1], # )2
+ parameters.children[-1], # )2
  ]
 - parameters.children = [parameters.what_if_this_was_actually_long.children[0], body, parameters.children[-1]] # type: ignore
-+ parameters.NOT_IMPLEMENTED_attr = [
-+ parameters.NOT_IMPLEMENTED_attr.NOT_IMPLEMENTED_attr[0],
++ parameters.children = [
++ parameters.what_if_this_was_actually_long.children[0],
 + body,
-+ parameters.NOT_IMPLEMENTED_attr[-1],
++ parameters.children[-1],
 + ] # type: ignore
  if (
 - self._proc is not None
@@ -457,26 +450,26 @@ else:
 # Comment before function.
 def inline_comments_in_brackets_ruin_everything():
  if typedargslist:
- parameters.NOT_IMPLEMENTED_attr = [
+ parameters.children = [
  children[0], # (1
  body,
  children[-1], # )1
  ]
- parameters.NOT_IMPLEMENTED_attr = [
+ parameters.children = [
  children[0],
  body,
  children[-1], # type: ignore
  ]
  else:
- parameters.NOT_IMPLEMENTED_attr = [
- parameters.NOT_IMPLEMENTED_attr[0], # (2 what if this was actually long
+ parameters.children = [
+ parameters.children[0], # (2 what if this was actually long
  body,
- parameters.NOT_IMPLEMENTED_attr[-1], # )2
+ parameters.children[-1], # )2
  ]
- parameters.NOT_IMPLEMENTED_attr = [
- parameters.NOT_IMPLEMENTED_attr.NOT_IMPLEMENTED_attr[0],
+ parameters.children = [
+ parameters.what_if_this_was_actually_long.children[0],
  body,
- parameters.NOT_IMPLEMENTED_attr[-1],
+ parameters.children[-1],
  ] # type: ignore
  if (
  NOT_IMPLEMENTED_left < NOT_IMPLEMENTED_right