Python: Fix formatter bugs and add comprehensive format tests (#6727)

* Split auto_format_battle_test.py into 3 files by test category

Organize 70 formatter battle tests into separate files matching the
project's one-file-per-concern pattern:
- auto_format_idempotency_test.py (30 tests)
- auto_format_corrections_test.py (20 tests)
- auto_format_edge_cases_test.py (20 tests)

* Python: Fix empty diff detection and eliminate spurious AST identity changes

Add empty diff detection to the test framework so that recipes which
modify the AST without changing printed output are caught as failures.

Fix six root causes of empty diffs in the formatting pipeline:

- Parser: place whitespace on inner expression instead of
  ControlParentheses prefix for if/while/match (Python has no actual
  parens, so SpacesVisitor was moving whitespace between equivalent
  positions)
- Parser: consume whitespace into BoolOp prefix and MultiImport
  container.before
- NormalizeFormat: early-return from _concatenate_prefix when prefix
  is empty to avoid creating new list objects
- SpacesVisitor: skip before_colon on return type TypeHints (already
  handled by visit_method_declaration) and skip space removal on
  ExpressionTypeTree inside ClassDeclaration (already handled by
  visit_class_declaration)
- PythonVisitor: guard visit_space(binary.negation) with None check
  since non-negated operators (in, is) have no negation space
- StatementExpression.replace: use replace_if_changed instead of
  always calling dataclass_replace
- utils: _is_changed uses equality for primitives, identity for
  complex objects

Remove 12 xfail markers from tests that now pass.

* Python: Fix 6 formatter bugs in indentation, whitespace, and spacing visitors

- Fix try/except/else indentation by adding visit_trailing_else_wrapper to TabsAndIndentsVisitor
- Fix backslash line continuation stripping in RemoveTrailingWhitespaceVisitor
- Fix delete statement trailing whitespace by using pad_last=False in parser
- Fix multiline class bases losing indentation via ClassDeclaration CONTINUATION_INDENT
- Fix decorator closing paren over-indentation by treating Annotation like MethodInvocation
- Fix Binary continuation indent inside Parentheses with visit_binary/visit_parentheses overrides
- Fix empty diff in assignment with parenthesized RHS by removing redundant space update

* Python: Fix continuation indentation and string concatenation spacing

Fix 3 remaining formatter bugs for parenthesized expressions:

- Add _compute_column_of() to align Binary continuation lines to the
  opening content column inside parentheses
- Detect method chains (via select type and cursor path) to use
  continuation indent instead of column alignment for chained calls
- Remove incorrect space injection for implicit string concatenation
  (StringConcatenation has no visible operator)

Author: knutwannheden

rewrite-python/rewrite/src/rewrite/python/_parser_visitor.py

@@ -511,7 +511,7 @@ def visit_Delete(self, node):
             random_id(),
             self.__source_before('del'),
             Markers.EMPTY,
-            [self.__pad_list_element(self.__convert(e), last=i == len(node.targets) - 1) for i, e in
+            [self.__pad_list_element(self.__convert(e), last=i == len(node.targets) - 1, pad_last=False) for i, e in
              enumerate(node.targets)]
         )
 
@@ -614,7 +614,7 @@ def visit_While(self, node):
             Markers.EMPTY,
             j.ControlParentheses(
                 random_id(),
-                self.__whitespace(),
+                Space.EMPTY,
                 Markers.EMPTY,
                 self.__pad_right(self.__convert(node.test), Space.EMPTY)
             ),
@@ -638,7 +638,7 @@ def visit_If(self, node):
             prefix = self.__source_before('elif')
         else:
             prefix = self.__source_before('if')
-        condition = j.ControlParentheses(random_id(), self.__whitespace(), Markers.EMPTY,
+        condition = j.ControlParentheses(random_id(), Space.EMPTY, Markers.EMPTY,
                                          self.__pad_right(self.__convert(node.test), Space.EMPTY))
         then = self.__pad_right(self.__convert_block(node.body), Space.EMPTY)
         elze = None
@@ -790,14 +790,16 @@ def visit_Try(self, node):
 
     def visit_Import(self, node):
         # TODO only use `MultiImport` when necessary (requires corresponding changes to printer)
+        prefix = self.__source_before('import')
+        names_prefix = self.__whitespace()
         return py.MultiImport(
             random_id(),
-            self.__source_before('import'),
+            prefix,
             Markers.EMPTY,
             None,
             False,
             JContainer(
-                Space.EMPTY,
+                names_prefix,
                 [self.__pad_list_element(self.__convert(n), i == len(node.names) - 1, pad_last=False) for i, n in
                  enumerate(node.names)],
                 Markers.EMPTY
@@ -1125,7 +1127,7 @@ def visit_Match(self, node):
             Markers.EMPTY,
             j.ControlParentheses(
                 random_id(),
-                self.__whitespace(),
+                Space.EMPTY,
                 Markers.EMPTY,
                 self.__pad_right(self.__convert(node.subject), Space.EMPTY)
             ),
@@ -1697,6 +1699,7 @@ def visit_BinOp(self, node):
             )
 
     def visit_BoolOp(self, node):
+        prefix = self.__whitespace()
         left = self.__convert(node.values[0])
         for right_expr in node.values[1:]:
             left = j.Binary(
@@ -1708,7 +1711,7 @@ def visit_BoolOp(self, node):
                 self.__convert(right_expr),
                 self._type_mapping.type(node)
             )
-        return left
+        return left.replace(prefix=prefix)
 
     def visit_Call(self, node):
         prefix = self.__whitespace()

rewrite-python/rewrite/src/rewrite/python/format/normalize_format.py

@@ -63,6 +63,9 @@ def _common_margin(s1, s2):
 
 
 def _concatenate_prefix(j: J, prefix: Space) -> J2:
+    if prefix.is_empty():
+        return j
+
     shift = _common_margin(None, j.prefix.whitespace)
 
     def modify_comment(c: PyComment) -> PyComment:

rewrite-python/rewrite/src/rewrite/python/format/remove_trailing_whitespace_visitor.py

@@ -37,6 +37,8 @@ def visit_marker(self, marker: Marker, p: P) -> Marker:
 
     @staticmethod
     def _normalize_whitespace(s):
+        if '\\' in s.whitespace:
+            return s
         last_newline = s.whitespace.rfind('\n')
         if last_newline > 0:
             ws = [c for i, c in enumerate(s.whitespace) if i >= last_newline or c in {'\r', '\n'}]

rewrite-python/rewrite/src/rewrite/python/format/spaces_visitor.py

@@ -279,8 +279,6 @@ def visit_python_binary(self, binary: Binary, p: P) -> J:
             b = self._apply_binary_space_around(b, True)
         elif op in [Binary.Type.FloorDivision, Binary.Type.MatrixMultiplication]:
             b = self._apply_binary_space_around(b, self._style.around_operators.multiplicative)
-        elif op == Binary.Type.StringConcatenation:
-            b = self._apply_binary_space_around(b, self._style.around_operators.additive)
         elif op == Binary.Type.Power:
             b = self._apply_binary_space_around(b, self._style.around_operators.power)
 
@@ -366,7 +364,6 @@ def _process_element(index, arg, last, use_space):
 
     def visit_parentheses(self, parentheses: Parentheses, p: P) -> J:
         p2 = cast(Parentheses, super().visit_parentheses(parentheses, p))
-        p2 = p2.replace(prefix=update_space(p2.prefix, False))
         p2 = p2.padding.replace(tree=p2.padding.tree.replace(after=update_space(p2.padding.tree.after, False)))
         return p2
 
@@ -467,13 +464,19 @@ def visit_import(self, import_: Import, p: P) -> J:
 
     def visit_type_hint(self, type_hint: TypeHint, p: P) -> J:
         th: TypeHint = cast(TypeHint, super().visit_type_hint(type_hint, p))
-        th = space_before(th, self._style.other.before_colon)
+        # Don't apply before_colon to return type annotations (handled by visit_method_declaration)
+        parent = self.cursor.parent_tree_cursor()
+        if not (parent and isinstance(parent.value, MethodDeclaration)):
+            th = space_before(th, self._style.other.before_colon)
         th = th.replace(type_tree=space_before(th.type_tree, self._style.other.after_colon))
         return th
 
     def visit_expression_type_tree(self, expression_type_tree: ExpressionTypeTree, p: P) -> J:
         ett = cast(ExpressionTypeTree, super().visit_expression_type_tree(expression_type_tree, p))
-        ett = space_before(ett, False)
+        # Don't remove space when inside ClassDeclaration (handled by visit_class_declaration)
+        parent = self.cursor.parent_tree_cursor()
+        if not (parent and isinstance(parent.value, ClassDeclaration)):
+            ett = space_before(ett, False)
         return ett
 
     def visit_comprehension_expression(self, comprehension_expression: ComprehensionExpression, p: P) -> J:

rewrite-python/rewrite/src/rewrite/python/format/tabs_and_indents_visitor.py

@@ -6,7 +6,7 @@
 from typing import TypeVar, Optional, cast, List
 
 from rewrite.java import tree as j
-from rewrite.java.support_types import J, Space, Comment
+from rewrite.java.support_types import J, Space, Comment, Statement
 from rewrite.java.tree import (
     JRightPadded, JLeftPadded, JContainer, Block, ArrayDimension,
     ClassDeclaration, Empty, MethodDeclaration, MethodInvocation,
@@ -17,6 +17,7 @@
 from rewrite.python import (
     PythonVisitor, TabsAndIndentsStyle, DictLiteral, CollectionLiteral,
     ExpressionStatement, OtherStyle, IntelliJ, ComprehensionExpression, PyComment,
+    TrailingElseWrapper, Binary as PyBinary,
 )
 from rewrite.tree import Tree
 from rewrite.visitor import P, T, Cursor
@@ -79,6 +80,11 @@ def pre_visit(self, tree: T, p: P) -> Optional[T]:
                 if self._other.use_continuation_indent.collections_and_comprehensions else self.IndentType.INDENT)
         elif isinstance(tree, ExpressionStatement):
             pass
+        elif isinstance(tree, (Binary, PyBinary)):
+            if self.cursor.first_enclosing(j.Parentheses) is not None:
+                self.cursor.put_message("indent_type", self.IndentType.ALIGN)
+            else:
+                self.cursor.put_message("indent_type", self.IndentType.INDENT)
         elif isinstance(tree, j.Expression):
             self.cursor.put_message("indent_type", self.IndentType.INDENT)
 
@@ -274,7 +280,7 @@ def visit_container(self, container: Optional[JContainer], p: P) -> Optional[JCo
         if isinstance(parent_tree, MethodDeclaration):
             rp_context = "method_declaration_parameter"
             needs_continuation_on_before = True
-        elif isinstance(parent_tree, MethodInvocation):
+        elif isinstance(parent_tree, (MethodInvocation, Annotation)):
             rp_context = "method_invocation_argument"
         elif isinstance(parent_tree, DictLiteral):
             rp_context = "dict_literal_element"
@@ -300,10 +306,12 @@ def visit_container(self, container: Optional[JContainer], p: P) -> Optional[JCo
                 self.cursor.put_message("indent_type",
                     self.IndentType.CONTINUATION_INDENT if self._other.use_continuation_indent.method_declaration_parameters
                     else self.IndentType.INDENT)
-            elif isinstance(parent_tree, MethodInvocation):
+            elif isinstance(parent_tree, (MethodInvocation, Annotation)):
                 self.cursor.put_message("indent_type",
                     self.IndentType.CONTINUATION_INDENT if self._other.use_continuation_indent.method_call_arguments
                     else self.IndentType.INDENT)
+            elif isinstance(parent_tree, ClassDeclaration):
+                self.cursor.put_message("indent_type", self.IndentType.CONTINUATION_INDENT)
             before = self.visit_space(container.before, p)
             js = list_map(lambda t: self.visit_right_padded(t, p), container.padding.elements)
 
@@ -370,6 +378,26 @@ def visit_try(self, try_: j.Try, p: P) -> J:
 
         return try_
 
+    def visit_trailing_else_wrapper(self, wrapper: TrailingElseWrapper, p: P) -> J:
+        wrapper = wrapper.replace(prefix=self.visit_space(wrapper.prefix, p))
+
+        temp_stmt = cast(Statement, self.visit_statement(wrapper, p))
+        if not isinstance(temp_stmt, type(wrapper)):
+            return temp_stmt
+        wrapper = temp_stmt
+
+        wrapper = wrapper.replace(markers=self.visit_markers(wrapper.markers, p))
+        wrapper = wrapper.replace(
+            statement=self.visit_and_cast(wrapper.statement, Statement, p)
+        )
+
+        self.cursor.put_message("space_context", "else_prefix")
+        wrapper = wrapper.padding.replace(
+            else_block=self.visit_left_padded(wrapper.padding.else_block, p)
+        )
+        self.cursor.put_message("space_context", None)
+        return wrapper
+
     def visit_else(self, else_: j.If.Else, p: P) -> J:
         self.cursor.put_message("space_context", "else_prefix")
         else_ = else_.replace(prefix=self.visit_space(else_.prefix, p))
@@ -429,14 +457,149 @@ def visit_class_declaration(self, class_decl: ClassDeclaration, p: P) -> J:
             class_decl.body, Block, p))
         return class_decl
 
+    def visit_parentheses(self, parens: j.Parentheses, p: P) -> J:
+        parens = parens.replace(prefix=self.visit_space(parens.prefix, p))
+        temp_expr = self.visit_expression(parens, p)
+        if not isinstance(temp_expr, j.Parentheses):
+            return temp_expr
+        parens = temp_expr
+        parens = parens.replace(markers=self.visit_markers(parens.markers, p))
+
+        tree_rp = parens.padding.tree
+        elem = tree_rp.element
+
+        # When a Binary expression starts on the same line as '(', compute
+        # its column and set last_indent so continuation lines align there
+        if isinstance(elem, (Binary, PyBinary)) and '\n' not in elem.prefix.last_whitespace:
+            col = self._compute_column_of(elem)
+            if col >= 0:
+                self.cursor.put_message("last_indent", col)
+
+        if isinstance(elem, J):
+            elem = self.visit_and_cast(elem, J, p)
+
+        after = tree_rp.after
+        if '\n' in after.last_whitespace:
+            # Close paren should align to the enclosing indent level
+            indent = cast(int, self.cursor.parent_or_throw.get_nearest_message("last_indent")) or 0
+            after = self._indent_to(after, indent)
+        else:
+            after = self.visit_space(after, p)
+
+        if elem is not tree_rp.element or after is not tree_rp.after:
+            parens = parens.padding.replace(tree=tree_rp.replace(element=elem, after=after))
+        return parens
+
+    def visit_binary(self, binary: Binary, p: P) -> J:
+        if self.cursor.first_enclosing(j.Parentheses) is not None:
+            # Inside parentheses: operator before spaces align to same indent as first line
+            binary = binary.replace(prefix=self.visit_space(binary.prefix, p))
+            temp_expr = self.visit_expression(binary, p)
+            if not isinstance(temp_expr, Binary):
+                return temp_expr
+            binary = temp_expr
+            binary = binary.replace(markers=self.visit_markers(binary.markers, p))
+            binary = binary.replace(left=self.visit_and_cast(binary.left, j.Expression, p))
+
+            op = binary.padding.operator
+            if '\n' in op.before.last_whitespace:
+                indent = cast(int, self.cursor.get_nearest_message("last_indent")) or 0
+                before = self._indent_to(op.before, indent)
+                if before is not op.before:
+                    binary = binary.padding.replace(operator=op.replace(before=before))
+            else:
+                binary = binary.padding.replace(
+                    operator=self.visit_left_padded(binary.padding.operator, p))
+
+            binary = binary.replace(right=self.visit_and_cast(binary.right, j.Expression, p))
+            return binary
+        return super().visit_binary(binary, p)
+
     def visit_method_invocation(self, method: MethodInvocation, p: P) -> J:
         select = method.padding.select
         if select is not None and '\n' in select.after.last_whitespace:
-            col = self._compute_select_column(method)
-            if col >= 0:
-                self.cursor.put_message("method_select_indent", col)
+            # Skip column alignment for method chains inside parentheses.
+            # A chain exists when the method's select is itself a
+            # MethodInvocation, or when there's an enclosing
+            # MethodInvocation between this node and the Parentheses.
+            skip = isinstance(select.element, MethodInvocation)
+            if not skip:
+                for c in self.cursor.get_path_as_cursors():
+                    v = c.value
+                    if v is method:
+                        continue
+                    if isinstance(v, j.Parentheses):
+                        break
+                    if isinstance(v, MethodInvocation):
+                        skip = True
+                        break
+            if not skip:
+                col = self._compute_select_column(method)
+                if col >= 0:
+                    self.cursor.put_message("method_select_indent", col)
         return super().visit_method_invocation(method, p)
 
+    def _compute_column_of(self, target: Tree) -> int:
+        """Compute the column position of a target tree element by printing from line start."""
+        from rewrite.python.printer import PythonPrinter, PrintOutputCapture
+
+        line_start = None
+        line_start_cursor = None
+        for c in self.cursor.get_path_as_cursors():
+            v = c.value
+            if isinstance(v, J):
+                line_start = v
+                line_start_cursor = c
+                if '\n' in v.prefix.whitespace:
+                    break
+        if line_start is None:
+            return -1
+
+        class _ColumnCounter(PrintOutputCapture):
+            def __init__(self):
+                super().__init__()
+                self.col = 0
+                self.found = False
+
+            def append(self, text):
+                if text and not self.found:
+                    for ch in text:
+                        self.col = 0 if ch == '\n' else self.col + 1
+                return self
+
+        class _Printer(PythonPrinter):
+            def __init__(self):
+                super().__init__()
+                orig_visit = self._delegate.visit
+                def _check(tree, p):
+                    if tree is target:
+                        p.found = True
+                        return tree
+                    return orig_visit(tree, p) if not p.found else tree
+                self._delegate.visit = _check
+
+            def visit(self, tree, p, parent=None):
+                if p.found or tree is target:
+                    p.found = True
+                    return tree
+                return super().visit(tree, p)
+
+        counter = _ColumnCounter()
+        printer = _Printer()
+        # Build initial cursor with proper J parent so context-dependent
+        # printing (e.g., = vs :=) works correctly
+        initial = Cursor(None, Cursor.ROOT_VALUE)
+        for c in line_start_cursor.get_path_as_cursors():
+            v = c.value
+            if v is line_start:
+                continue
+            if isinstance(v, J):
+                initial = Cursor(Cursor(None, Cursor.ROOT_VALUE), v)
+                break
+        printer.set_cursor(initial)
+        printer.visit(line_start, counter)
+        return counter.col if counter.found else -1
+
     def _compute_select_column(self, method: MethodInvocation) -> int:
         from rewrite.python.printer import PythonPrinter, PrintOutputCapture
 

rewrite-python/rewrite/src/rewrite/python/tree.py

@@ -612,16 +612,7 @@ def replace(self, **kwargs) -> 'StatementExpression':
         if 'markers' in kwargs:
             new_statement = kwargs.get('statement', self._statement).replace(markers=kwargs.pop('markers'))
             kwargs['statement'] = new_statement
-        # Map remaining kwargs
-        mapped = {}
-        for key, value in kwargs.items():
-            if not key.startswith('_') and hasattr(self, f'_{key}'):
-                mapped[f'_{key}'] = value
-            else:
-                mapped[key] = value
-        if not mapped:
-            return self
-        return dataclass_replace(self, **mapped)
+        return replace_if_changed(self, **kwargs)
 
     def accept_python(self, v: PythonVisitor[P], p: P) -> J:
         return v.visit_statement_expression(self, p)

rewrite-python/rewrite/src/rewrite/python/visitor.py

@@ -131,9 +131,10 @@ def visit_python_binary(self, binary: Binary, p: P) -> J:
         binary = binary.padding.replace(
             operator=self.visit_left_padded(binary.padding.operator, p)
         )
-        binary = binary.replace(
-            negation=self.visit_space(binary.negation, p)
-        )
+        if binary.negation is not None:
+            binary = binary.replace(
+                negation=self.visit_space(binary.negation, p)
+            )
         binary = binary.replace(
             right=self.visit_and_cast(binary.right, Expression, p)
         )