fix(formatter): properly indent binary concatenation continuation lines in function arguments

closes #594

Signed-off-by: azjezz <[email protected]>

Author: azjezz

crates/formatter/src/internal/format/binaryish.rs

@@ -132,6 +132,35 @@ pub(super) fn print_binaryish_expression<'arena>(
             )
         );
 
+    let is_breaking_concat_in_arg = operator.is_concatenation()
+        && matches!(grandparent, Some(Node::PositionalArgument(_) | Node::NamedArgument(_)))
+        && (matches!(left, Expression::Call(_)) || matches!(right, Expression::Call(_)));
+
+    if is_breaking_concat_in_arg {
+        let has_space_around = match operator {
+            BinaryishOperator::Binary(BinaryOperator::StringConcat(_)) => {
+                f.settings.space_around_concatenation_binary_operator
+            }
+            _ => true,
+        };
+        let group_id = f.next_id();
+
+        return Document::Group(
+            Group::new(vec![
+                in f.arena;
+                left.format(f),
+                Document::IndentIfBreak(IndentIfBreak::new(group_id, vec![
+                    in f.arena;
+                    Document::Line(if has_space_around { Line::default() } else { Line::soft() }),
+                    format_token(f, operator.span(), operator.as_str()),
+                    Document::String(if has_space_around { " " } else { "" }),
+                    right.format(f),
+                ])),
+            ])
+            .with_id(group_id),
+        );
+    }
+
     let parts = print_binaryish_expression_parts(f, left, operator, right, is_inside_parenthesis, false);
 
     if is_inside_parenthesis {

crates/formatter/src/internal/format/misc.rs

@@ -121,6 +121,10 @@ pub(super) fn should_hug_expression<'arena>(
         }
 
         if binary.operator.is_concatenation() {
+            if matches!(binary.lhs, Expression::Call(_)) || matches!(binary.rhs, Expression::Call(_)) {
+                return false;
+            }
+
             return (is_left_hand_side_simple && should_hug_expression(f, binary.rhs, arrow_function_recursion))
                 || (is_right_hand_side_simple && should_hug_expression(f, binary.lhs, arrow_function_recursion));
         }
@@ -209,6 +213,16 @@ pub fn is_breaking_expression<'arena>(
     )
 }
 
+fn contains_breaking_concatenation(node: &Expression<'_>) -> bool {
+    match node {
+        Expression::Binary(binary) if binary.operator.is_concatenation() => {
+            matches!(binary.lhs, Expression::Call(_)) || matches!(binary.rhs, Expression::Call(_))
+        }
+        Expression::Parenthesized(inner) => contains_breaking_concatenation(inner.expression),
+        _ => false,
+    }
+}
+
 pub fn is_expandable_expression<'arena>(node: &'arena Expression<'arena>, include_calls: bool) -> bool {
     if let Expression::Parenthesized(inner) = node {
         return is_expandable_expression(inner.expression, include_calls);
@@ -570,7 +584,19 @@ pub(super) fn print_condition<'arena>(
     let was_in_condition = f.in_condition;
     f.in_condition = true;
 
+    let has_breaking_concat = match condition {
+        Expression::Call(call) => {
+            call.get_argument_list().arguments.iter().any(|arg| contains_breaking_concatenation(arg.value()))
+        }
+        Expression::Instantiation(inst) => inst
+            .argument_list
+            .as_ref()
+            .is_some_and(|args| args.arguments.iter().any(|arg| contains_breaking_concatenation(arg.value()))),
+        _ => false,
+    };
+
     let condition = if is_expandable_expression(condition, true)
+        && !has_breaking_concat
         && !f.has_comment(condition.span(), CommentFlags::Leading | CommentFlags::Trailing)
     {
         Document::Group(Group::new(vec![

crates/formatter/tests/cases/issue_594/after.php

@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+if (
+    file_exists(
+        dirname(__DIR__)
+            . '/var/cache/prod/App_KernelProdContainer.preload.php',
+    )
+) {
+    include_file(
+        dirname(__DIR__)
+            . '/var/cache/prod/App_KernelProdContainer.preload.php',
+    );
+
+    require
+        dirname(__DIR__)
+            . '/var/cache/prod/App_KernelProdContainer.preload.php';
+}

crates/formatter/tests/cases/issue_594/before.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+if (file_exists(dirname(__DIR__)
+. '/var/cache/prod/App_KernelProdContainer.preload.php')) {
+    include_file(
+        dirname(__DIR__)
+            . '/var/cache/prod/App_KernelProdContainer.preload.php'
+            );
+
+            require
+                dirname(__DIR__)
+                    . '/var/cache/prod/App_KernelProdContainer.preload.php';
+}

crates/formatter/tests/cases/issue_594/settings.inc

@@ -0,0 +1,4 @@
+FormatSettings {
+    print_width: 80,
+    ..Default::default()
+}

crates/formatter/tests/mod.rs

@@ -166,6 +166,48 @@ test_case!(unary_prefix_prec);
 test_case!(rescue);
 test_case!(yield_kv_closure);
 test_case!(symfony_form_builder);
+test_case!(format_ignore_single_line_comment);
+test_case!(format_ignore_block_comment);
+test_case!(format_ignore_docblock);
+test_case!(format_ignore_hash_comment);
+test_case!(format_ignore_range_basic);
+test_case!(format_ignore_range_no_end);
+test_case!(format_ignore_range_multiple);
+test_case!(format_ignore_range_block_comment);
+test_case!(format_ignore_range_multi_statements);
+test_case!(format_ignore_next_basic);
+test_case!(format_ignore_next_multiple);
+test_case!(class_member_format_ignore_next);
+test_case!(class_member_format_ignore_region);
+test_case!(class_member_format_ignore_next_trait_use);
+test_case!(class_member_format_ignore_next_enum_case);
+test_case!(class_member_format_ignore_next_with_attributes);
+test_case!(class_member_format_ignore_region_mixed_types);
+test_case!(class_member_format_ignore_in_trait);
+test_case!(class_member_format_ignore_in_interface);
+test_case!(class_member_format_ignore_in_enum);
+test_case!(class_member_format_ignore_in_anonymous_class);
+test_case!(class_member_format_ignore_next_last_member);
+test_case!(class_member_format_ignore_next_multiple_consecutive);
+test_case!(class_member_format_ignore_region_entire_body);
+test_case!(class_member_format_ignore_region_single_member);
+test_case!(class_member_format_ignore_region_standalone_comment);
+test_case!(class_member_format_ignore_next_abstract_method);
+test_case!(class_member_format_ignore_next_static_members);
+test_case!(class_member_format_ignore_next_readonly_property);
+test_case!(class_member_format_ignore_next_promoted_property);
+test_case!(class_member_format_ignore_block_comment);
+test_case!(class_member_format_ignore_hash_comment);
+test_case!(class_member_format_ignore_docblock);
+test_case!(class_member_format_ignore_multiple_regions);
+test_case!(class_member_format_ignore_nested_class);
+test_case!(class_member_format_ignore_next_final_method);
+test_case!(class_member_format_ignore_next_visibility_variations);
+test_case!(class_member_format_ignore_region_with_comments);
+test_case!(class_member_format_ignore_next_hooked_property);
+test_case!(class_member_format_ignore_next_interface_constant);
+test_case!(class_member_format_ignore_next_backed_enum);
+test_case!(class_member_format_ignore_in_nested_anonymous_class);
 
 // A special test case for regressions in the Psl codebase
 test_case!(psl_regressions);
@@ -241,63 +283,14 @@ test_case!(issue_503);
 test_case!(issue_545);
 test_case!(issue_536);
 test_case!(issue_556);
+test_case!(issue_594);
 test_case!(issue_602);
 test_case!(issue_634);
 test_case!(issue_640);
 test_case!(issue_683);
 test_case!(issue_727);
 test_case!(issue_738);
 
-// Format ignore tests
-test_case!(format_ignore_single_line_comment);
-test_case!(format_ignore_block_comment);
-test_case!(format_ignore_docblock);
-test_case!(format_ignore_hash_comment);
-
-// Range-based format ignore tests
-test_case!(format_ignore_range_basic);
-test_case!(format_ignore_range_no_end);
-test_case!(format_ignore_range_multiple);
-test_case!(format_ignore_range_block_comment);
-test_case!(format_ignore_range_multi_statements);
-
-// Format ignore-next tests
-test_case!(format_ignore_next_basic);
-test_case!(format_ignore_next_multiple);
-
-// Class member format ignore tests
-test_case!(class_member_format_ignore_next);
-test_case!(class_member_format_ignore_region);
-test_case!(class_member_format_ignore_next_trait_use);
-test_case!(class_member_format_ignore_next_enum_case);
-test_case!(class_member_format_ignore_next_with_attributes);
-test_case!(class_member_format_ignore_region_mixed_types);
-test_case!(class_member_format_ignore_in_trait);
-test_case!(class_member_format_ignore_in_interface);
-test_case!(class_member_format_ignore_in_enum);
-test_case!(class_member_format_ignore_in_anonymous_class);
-test_case!(class_member_format_ignore_next_last_member);
-test_case!(class_member_format_ignore_next_multiple_consecutive);
-test_case!(class_member_format_ignore_region_entire_body);
-test_case!(class_member_format_ignore_region_single_member);
-test_case!(class_member_format_ignore_region_standalone_comment);
-test_case!(class_member_format_ignore_next_abstract_method);
-test_case!(class_member_format_ignore_next_static_members);
-test_case!(class_member_format_ignore_next_readonly_property);
-test_case!(class_member_format_ignore_next_promoted_property);
-test_case!(class_member_format_ignore_block_comment);
-test_case!(class_member_format_ignore_hash_comment);
-test_case!(class_member_format_ignore_docblock);
-test_case!(class_member_format_ignore_multiple_regions);
-test_case!(class_member_format_ignore_nested_class);
-test_case!(class_member_format_ignore_next_final_method);
-test_case!(class_member_format_ignore_next_visibility_variations);
-test_case!(class_member_format_ignore_region_with_comments);
-test_case!(class_member_format_ignore_next_hooked_property);
-test_case!(class_member_format_ignore_next_interface_constant);
-test_case!(class_member_format_ignore_next_backed_enum);
-test_case!(class_member_format_ignore_in_nested_anonymous_class);
-
 #[test]
 fn test_all_test_cases_are_ran() {
     let test_case_file = include_str!("mod.rs");