Blink parity: collapse blank lines between call/parameter arguments; gate to 0.4%

.github/workflows/blink_parity_mautic.yaml

@@ -95,7 +95,7 @@ jobs:
             - name: Compare the two trees
               run: |
                   # ratchet gate: fail once the differing share crosses this; lower it as parity improves
-                  MAX_DIFF_PERCENT=0.5
+                  MAX_DIFF_PERCENT=0.4
                   total=0
                   differ=0
                   differing_files=""

blink/internal/fixer/rules/array_list_item_newline.go

@@ -68,7 +68,7 @@ func (ArrayListItemNewline) Fix(s *tokens.Stream) bool {
 			closeIdx++
 			changed = true
 		}
-		if reflowParen(s, open, closeIdx) {
+		if reflowParen(s, open, closeIdx, false) {
 			changed = true
 		}
 	}

blink/internal/fixer/rules/array_list_item_newline_test.go

@@ -56,4 +56,9 @@ func TestArrayListItemNewline(t *testing.T) {
 	if _, changed := apply(t, f, "<?php\nreturn [$a, $b, [\n    'k' => 1,\n    'm' => 2,\n]];"); changed {
 		t.Fatal("already-multiline inner must not break the outer opener")
 	}
+
+	// a blank line a user put between array items is preserved (unlike call args)
+	if _, changed := apply(t, f, "<?php\n$x = [\n    'a' => 1,\n\n    'b' => 2,\n];"); changed {
+		t.Fatal("blank line between array items must be preserved")
+	}
 }

blink/internal/fixer/rules/method_argument_space.go

@@ -279,7 +279,7 @@ func reflowSingleArgOrMultiline(s *tokens.Stream) bool {
 			if ensureSingleLineForParen(s, open, closeIdx) {
 				changed = true
 			}
-		} else if reflowParen(s, open, closeIdx) {
+		} else if reflowParen(s, open, closeIdx, true) {
 			changed = true
 		}
 	}
@@ -409,7 +409,7 @@ func reflowMultilineArgs(s *tokens.Stream) bool {
 		if sigNext(s, open) == closeIdx {
 			continue // empty ()
 		}
-		if reflowParen(s, open, closeIdx) {
+		if reflowParen(s, open, closeIdx, true) {
 			changed = true
 		}
 	}
@@ -542,7 +542,7 @@ func placeAttributesInParen(s *tokens.Stream, open, closeIdx int, placement stri
 
 // reflowParen puts each top-level argument of the paren at open on its own line,
 // with "(" and ")" on their own lines, indented one level past the call.
-func reflowParen(s *tokens.Stream, open, closeIdx int) bool {
+func reflowParen(s *tokens.Stream, open, closeIdx int, collapseBlanks bool) bool {
 	changed := false
 	base := lineIndentBefore(s, open)
 	argNL := "\n" + base + "    "
@@ -573,7 +573,7 @@ func reflowParen(s *tokens.Stream, open, closeIdx int) bool {
 		changed = true
 	}
 	for _, c := range slices.Backward(commas) {
-		if reflowAfterComma(s, c, base) {
+		if reflowAfterComma(s, c, base, collapseBlanks) {
 			changed = true
 		}
 	}
@@ -638,7 +638,7 @@ func reflowBreakAfterComments(s *tokens.Stream, open, closeIdx int, base string)
 // comment that sits on the comma's line ("arg, // note" or "arg, /* note */")
 // stays there and the break goes after the comment, matching php-cs-fixer;
 // otherwise the break goes right after the comma.
-func reflowAfterComma(s *tokens.Stream, comma int, base string) bool {
+func reflowAfterComma(s *tokens.Stream, comma int, base string, collapseBlanks bool) bool {
 	n := comma + 1
 	ws := -1
 	if n < s.Len() && s.At(n).Kind == token.Whitespace && !hasNewline(s.At(n).Value) {
@@ -663,7 +663,7 @@ func reflowAfterComma(s *tokens.Stream, comma int, base string) bool {
 		}
 		return changed
 	}
-	return editSlotAfter(s, comma, argNLAfterComma(s, comma, base))
+	return editSlotAfter(s, comma, argNLAfterComma(s, comma, base, collapseBlanks))
 }
 
 // argListIsMultiline reports whether the argument list is split at the top level
@@ -756,12 +756,24 @@ func lineIndentBefore(s *tokens.Stream, idx int) string {
 // arguments is preserved (ECS keeps blank lines in a multiline argument list and
 // only normalizes the indentation), so a comma whose following whitespace holds a
 // blank line keeps that blank rather than collapsing to a single newline.
-func argNLAfterComma(s *tokens.Stream, comma int, base string) string {
+func argNLAfterComma(s *tokens.Stream, comma int, base string, collapseBlanks bool) string {
+	// an array literal keeps a blank line a user put between items; a call or
+	// declaration list collapses it, except before a comment that annotates the
+	// next argument, which php-cs-fixer keeps
 	if comma+1 < s.Len() {
 		ws := s.At(comma + 1)
 		if ws.Kind == token.Whitespace {
 			if newlines := strings.Count(ws.Value, "\n"); newlines >= 2 {
-				return strings.Repeat("\n", newlines) + base + "    "
+				keep := !collapseBlanks
+				if collapseBlanks {
+					if nx := nextSignificantIndex(s, comma); nx >= 0 &&
+						isComment(s.At(nx)) && !isAttributeComment(s.At(nx)) {
+						keep = true
+					}
+				}
+				if keep {
+					return strings.Repeat("\n", newlines) + base + "    "
+				}
 			}
 		}
 	}

blink/internal/fixer/rules/method_argument_space_test.go

@@ -22,7 +22,9 @@ func TestMethodArgumentSpace(t *testing.T) {
 		{"multiline arg list becomes fully multiline", "<?php foo($a,\n    $b);", "<?php foo(\n    $a,\n    $b\n);", true},
 		{"newline only inside a nested arg is left alone", "<?php foo($a, [\n    1,\n]);", "<?php foo($a, [\n    1,\n]);", false},
 		{"trailing comma before paren", "<?php foo($a,);", "<?php foo($a, );", true},
-		{"blank line between multiline args is preserved", "<?php foo(\n    $a,\n\n    $b\n);", "<?php foo(\n    $a,\n\n    $b\n);", false},
+		{"blank line between call args is collapsed", "<?php foo(\n    $a,\n\n    $b\n);", "<?php foo(\n    $a,\n    $b\n);", true},
+		{"blank line between attributed promoted params is collapsed", "<?php class C { public function __construct(\n    #[A]\n    int $a,\n\n    #[B]\n    int $b,\n) {} }", "<?php class C { public function __construct(\n    #[A]\n    int $a,\n    #[B]\n    int $b,\n) {} }", true},
+		{"blank line before a commented call arg is kept", "<?php foo(\n    $a,\n\n    // next\n    $b\n);", "<?php foo(\n    $a,\n\n    // next\n    $b\n);", false},
 		{"grouping paren after ! breaks after (", "<?php return !($a\n|| $b\n);", "<?php return !(\n    $a\n|| $b\n);", true},
 		{"grouping paren after && breaks after (", "<?php $x = $a && ($b\n|| $c\n);", "<?php $x = $a && (\n    $b\n|| $c\n);", true},
 		{"single-line grouping paren left alone", "<?php return !($a || $b);", "<?php return !($a || $b);", false},

blink/internal/fixer/rules/standalone_line_in_multiline_array.go

@@ -41,7 +41,7 @@ func (StandaloneLineInMultilineArray) Fix(s *tokens.Stream) bool {
 		if standaloneArrayShouldSkip(s, open, closeIdx) {
 			return changed // upstream fix() returns on the first skipped array
 		}
-		if reflowParen(s, open, closeIdx) {
+		if reflowParen(s, open, closeIdx, false) {
 			changed = true
 		}
 	}

blink/internal/fixer/rules/standalone_line_plain_constructor_param.go

@@ -48,7 +48,7 @@ func (StandaloneLinePlainConstructorParam) Fix(s *tokens.Stream) bool {
 		if hasPromotedParam(s, open, closeIdx) {
 			continue // promoted constructors are handled by StandaloneLinePromotedProperty
 		}
-		if reflowParen(s, open, closeIdx) {
+		if reflowParen(s, open, closeIdx, true) {
 			changed = true
 		}
 	}

blink/internal/fixer/rules/standalone_line_promoted_property.go

@@ -44,7 +44,7 @@ func (StandaloneLinePromotedProperty) Fix(s *tokens.Stream) bool {
 		if !hasPromotedParam(s, open, closeIdx) {
 			continue // only constructor property promotion is split out
 		}
-		if reflowParen(s, open, closeIdx) {
+		if reflowParen(s, open, closeIdx, true) {
 			changed = true
 		}
 	}

blink/internal/fixer/rules/standalone_line_required_param.go

@@ -50,7 +50,7 @@ func (StandaloneLineRequiredParam) Fix(s *tokens.Stream) bool {
 		if closeIdx < 0 || sigNext(s, open) == closeIdx {
 			continue // no parameters
 		}
-		if reflowParen(s, open, closeIdx) {
+		if reflowParen(s, open, closeIdx, true) {
 			changed = true
 		}
 	}