fix(formatter): ensure match expression with trailing comments on conditions is idempotent

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

Author: azjezz

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

@@ -901,12 +901,23 @@ impl<'arena> Format<'arena> for MatchExpressionArm<'arena> {
     fn format(&'arena self, f: &mut FormatterState<'_, 'arena>) -> Document<'arena> {
         wrap!(f, self, MatchExpressionArm, {
             let len = self.conditions.len();
+
+            let must_break = self
+                .conditions
+                .iter()
+                .take(len.saturating_sub(1))
+                .any(|condition| f.has_comment(condition.span(), CommentFlags::Trailing | CommentFlags::Line));
+
             let mut contents = vec![in f.arena];
             for (i, condition) in self.conditions.iter().enumerate() {
                 contents.push(condition.format(f));
                 if i != (len - 1) {
                     contents.push(Document::String(","));
-                    contents.push(Document::Line(Line::default()));
+                    contents.push(if must_break {
+                        Document::Line(Line::hard())
+                    } else {
+                        Document::Line(Line::default())
+                    });
                 } else if f.settings.trailing_comma && i > 0 {
                     contents.push(Document::IfBreak(IfBreak::then(f.arena, Document::String(","))));
                 }
@@ -917,18 +928,19 @@ impl<'arena> Format<'arena> for MatchExpressionArm<'arena> {
                 group_id,
                 vec![
                     in f.arena;
-                    Document::Line(Line::default()),
+                    if must_break { Document::Line(Line::hard()) } else { Document::Line(Line::default()) },
                     format_token(f, self.arrow, "=> "),
                 ],
             )));
 
             Document::Group(
                 Group::new(vec![
                     in f.arena;
-                    Document::Group(Group::new(contents)),
+                    Document::Group(Group::new(contents).with_break(must_break)),
                     self.expression.format(f),
                 ])
-                .with_id(group_id),
+                .with_id(group_id)
+                .with_break(must_break),
             )
         })
     }

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

@@ -0,0 +1,35 @@
+<?php
+
+class DoctrineExtractor
+{
+    /**
+     * Gets the corresponding built-in PHP type.
+     */
+    private function getTypeIdentifier(string $doctrineType): null|TypeIdentifier
+    {
+        return match ($doctrineType) {
+            Types::SMALLINT, Types::INTEGER => TypeIdentifier::INT,
+            Types::FLOAT => TypeIdentifier::FLOAT,
+            Types::BIGINT, Types::STRING, Types::TEXT, Types::GUID, Types::DECIMAL => TypeIdentifier::STRING,
+            Types::BOOLEAN => TypeIdentifier::BOOL,
+            Types::BLOB, Types::BINARY => TypeIdentifier::RESOURCE,
+            'object', // DBAL < 4
+            Types::DATE_MUTABLE,
+            Types::DATETIME_MUTABLE,
+            Types::DATETIMETZ_MUTABLE,
+            'vardatetime',
+            Types::TIME_MUTABLE,
+            Types::DATE_IMMUTABLE,
+            Types::DATETIME_IMMUTABLE,
+            Types::DATETIMETZ_IMMUTABLE,
+            Types::TIME_IMMUTABLE,
+            Types::DATEINTERVAL,
+                => TypeIdentifier::OBJECT,
+            'array', // DBAL < 4
+            'json_array', // DBAL < 3
+            Types::SIMPLE_ARRAY,
+                => TypeIdentifier::ARRAY,
+            default => null,
+        };
+    }
+}

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

@@ -0,0 +1,39 @@
+<?php
+
+class DoctrineExtractor
+{
+    /**
+     * Gets the corresponding built-in PHP type.
+     */
+    private function getTypeIdentifier(string $doctrineType): ?TypeIdentifier
+    {
+        return match ($doctrineType) {
+            Types::SMALLINT,
+            Types::INTEGER => TypeIdentifier::INT,
+            Types::FLOAT => TypeIdentifier::FLOAT,
+            Types::BIGINT,
+            Types::STRING,
+            Types::TEXT,
+            Types::GUID,
+            Types::DECIMAL => TypeIdentifier::STRING,
+            Types::BOOLEAN => TypeIdentifier::BOOL,
+            Types::BLOB,
+            Types::BINARY => TypeIdentifier::RESOURCE,
+            'object', // DBAL < 4
+            Types::DATE_MUTABLE,
+            Types::DATETIME_MUTABLE,
+            Types::DATETIMETZ_MUTABLE,
+            'vardatetime',
+            Types::TIME_MUTABLE,
+            Types::DATE_IMMUTABLE,
+            Types::DATETIME_IMMUTABLE,
+            Types::DATETIMETZ_IMMUTABLE,
+            Types::TIME_IMMUTABLE,
+            Types::DATEINTERVAL => TypeIdentifier::OBJECT,
+            'array', // DBAL < 4
+            'json_array', // DBAL < 3
+            Types::SIMPLE_ARRAY => TypeIdentifier::ARRAY,
+            default => null,
+        };
+    }
+}

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

@@ -0,0 +1,5 @@
+FormatSettings {
+    null_type_hint: NullTypeHint::NullPipe,
+    space_around_concatenation_binary_operator: false,
+    ..FormatSettings::default()
+}

crates/formatter/tests/mod.rs

@@ -223,6 +223,7 @@ test_case!(align_run_breaking);
 test_case!(align_modifier_breaking);
 test_case!(inline_block_comments_in_arguments);
 test_case!(idempotent_chain_with_array);
+test_case!(match_idempotency);
 
 // A special test case for regressions in the Psl codebase
 test_case!(psl_regressions);