Blink: fix StandaloneLineRequiredParam, BracesPosition, StatementIndentation, MethodArgumentSpace and TrailingCommaInMultiline

blink/internal/fixer/rules/braces_position.go

@@ -1,7 +1,8 @@
 package rules
 
 import (
-	"strings"
+	"regexp"
+	"slices"
 
 	"github.com/ecsphp/ecs-src/blink/internal/fixer"
 	"github.com/ecsphp/ecs-src/blink/internal/token"
@@ -84,14 +85,8 @@ func (f BracesPosition) Fix(s *tokens.Stream) bool {
 		switch kind {
 		case braceClassLike:
 			category = "classes"
-			// an anonymous class ("new class ... {", also "new #[Attr] class ... {")
-			// is configured separately; step back over any leading attribute
-			if kw >= 0 {
-				if p := prevMeaningfulSkippingAttributes(s, kw); p >= 0 {
-					if prev := s.At(p); prev.Kind == token.Keyword && strings.EqualFold(prev.Value, "new") {
-						category = "anon_classes"
-					}
-				}
+			if taIsAnonymousClass(s, kw) {
+				category = "anon_classes"
 			}
 		case braceFunctionDecl:
 			category = "functions"
@@ -117,17 +112,23 @@ func (f BracesPosition) Fix(s *tokens.Stream) bool {
 		}
 
 		if nextLine, handle := f.wantNextLine(category, s, i); handle {
+			if target := bpBraceTarget(s, i, nextLine); target >= 0 {
+				i = bpMoveBrace(s, i, target)
+				changed = true
+			}
 			want := " "
 			if nextLine {
 				// php-cs-fixer aligns the brace with the line holding the keyword
 				want = "\n" + lineIndent(s, kw)
 			}
-			if i > 0 && s.At(i-1).Kind == token.Whitespace {
+			switch {
+			case !nextLine && bpFollowsLineComment(s, i):
+			case i > 0 && s.At(i-1).Kind == token.Whitespace:
 				if s.At(i-1).Value != want {
 					s.SetValue(i-1, want)
 					changed = true
 				}
-			} else {
+			default:
 				s.InsertAt(i, token.Token{Kind: token.Whitespace, Value: want})
 				i++
 				changed = true
@@ -141,6 +142,117 @@ func (f BracesPosition) Fix(s *tokens.Stream) bool {
 	return changed
 }
 
+// bpBraceTarget is PHP-CS-Fixer's $moveBraceToIndex: where the brace goes so that a
+// comment next to it stays out of the way, or -1. On the same line the brace goes
+// before the comments, as written after "// c" it would be part of the comment. On the
+// next line it goes after a "//" or "#" comment that follows it.
+func bpBraceTarget(s *tokens.Stream, open int, nextLine bool) int {
+	if nextLine {
+		return bpTargetAfterComment(s, open)
+	}
+	target := -1
+	previous := prevMeaningfulIndex(s, open)
+	for i := open - 1; i > previous; i-- {
+		if !isComment(s.At(i)) {
+			continue
+		}
+		i--
+		if s.At(i).Kind == token.Whitespace {
+			target = i
+		} else if i == previous {
+			target = previous + 1
+		}
+	}
+	// PHP-CS-Fixer moves it here too and writes a parse error
+	if target < 0 || isLineComment(s.At(target-1)) || s.MatchForward(open) < 0 {
+		return -1
+	}
+	return target
+}
+
+func bpTargetAfterComment(s *tokens.Stream, open int) int {
+	if before := s.At(open - 1); before.Kind == token.Whitespace && hasNewline(before.Value) {
+		return -1
+	}
+	closing := s.MatchForward(open)
+	if closing < 0 {
+		return -1
+	}
+	target := -1
+	for i := open + 1; i < closing; i++ {
+		t := s.At(i)
+		// PHP-CS-Fixer has already put a one-line body on its own line at this point;
+		// blink has not, so stop at code or the brace would jump over it
+		if t.Kind == token.Whitespace && hasNewline(t.Value) || t.Kind != token.Whitespace && !isComment(t) {
+			break
+		}
+		// blink's "// c ?>" comment keeps the space, so the close tag can follow directly
+		if isLineComment(t) && s.At(i+1).Kind == token.Whitespace {
+			target = i + 1
+		}
+	}
+	return target
+}
+
+// bpFollowsLineComment reports a brace whose line starts after a "//" or "#" comment:
+// on the same line as that comment it would be part of it.
+func bpFollowsLineComment(s *tokens.Stream, open int) bool {
+	before := open - 1
+	if s.At(before).Kind == token.Whitespace {
+		before--
+	}
+	return isLineComment(s.At(before))
+}
+
+var bpLeadingBreakRe = regexp.MustCompile(`^(?:[ \t]*?(?:\r\n|\n|\r))?[ \t]*`)
+
+// bpMoveBrace moves the brace at open to target like PHP-CS-Fixer: the tokens between
+// shift by one, and the whitespace left on either side is cleared or merged. It returns
+// the new index of the brace.
+func bpMoveBrace(s *tokens.Stream, open, target int) int {
+	first, last := min(open-1, target), max(open+1, target)
+	work := slices.Clone(s.Tokens()[first : last+1])
+	at := func(i int) *token.Token { return &work[i-first] }
+	clearAt := func(i int) { *at(i) = token.Token{Kind: token.Whitespace} }
+	isSpace := func(i int) bool { return at(i).Kind == token.Whitespace }
+
+	brace := *at(open)
+	delta := 1
+	if open > target {
+		delta = -1
+	}
+	if isSpace(open + delta) {
+		if delta == -1 && hasNewline(at(open-1).Value) {
+			if isSpace(open + 1) {
+				at(open + 1).Value = bpLeadingBreakRe.ReplaceAllString(at(open+1).Value, "")
+			}
+		} else if isSpace(open - 1) {
+			clearAt(open - 1)
+		}
+	}
+	for i := open; i != target; i += delta {
+		*at(i) = *at(i + delta)
+	}
+	*at(target) = brace
+	if isSpace(open) && isSpace(open+1) {
+		at(open).Value += at(open + 1).Value
+		clearAt(open + 1)
+	}
+
+	moved := make([]token.Token, 0, len(work))
+	index := -1
+	for i, t := range work {
+		if i+first == target {
+			index = first + len(moved)
+		}
+		if t.Value != "" {
+			moved = append(moved, t)
+		}
+	}
+	s.ReplaceRange(first, last, moved)
+	return index
+}
+
 // closureBodyNeedsNewlines reports whether the body of the anonymous function
 // whose "{" is at open goes on lines of its own: always when
 // allow_single_line_anonymous_functions is false, and otherwise only when

blink/internal/fixer/rules/braces_position_comment_guard_test.go

@@ -0,0 +1,63 @@
+package rules
+
+import (
+	"strings"
+	"testing"
+)
+
+// PHP-CS-Fixer first puts the body on its own line, which blink does not do, so its output
+// is no reference. The brace must not jump over the code to reach the comment.
+func TestBracesPositionBraceStaysBeforeCode(t *testing.T) {
+	t.Parallel()
+	got, _ := apply(t, BracesPosition{}, "<?php\nclass A { public function f() { // c\n        echo 1;\n    }\n}\n")
+	if !strings.HasPrefix(got, "<?php\nclass A\n{ public function f()") {
+		t.Fatalf("got=%q", got)
+	}
+}
+
+// The PHP engine throws on an unclosed brace, so there is no output to match. The brace
+// must stay where it is, and it must not stop the run.
+func TestBracesPositionUnclosedBraceAtEndOfFile(t *testing.T) {
+	t.Parallel()
+	cases := []struct{ src, want string }{
+		{"<?php\nif ($b) /* c */{", "<?php\nif ($b) /* c */ {"},
+		{"<?php\nif ($b) // c\n{", "<?php\nif ($b) // c\n{"},
+		{"<?php\nif ($b) /* c */ {", "<?php\nif ($b) /* c */ {"},
+	}
+	for _, c := range cases {
+		if got, _ := apply(t, BracesPosition{}, c.src); got != c.want {
+			t.Errorf("%s\n got=%q\nwant=%q", c.src, got, c.want)
+		}
+	}
+}
+
+// PHP-CS-Fixer writes the brace inside the comment on these, so its output is no reference.
+func TestBracesPositionBraceStaysOutOfLineComment(t *testing.T) {
+	t.Parallel()
+	for _, src := range []string{
+		"<?php\nif ($b) /*a*/// c\n{\n    echo 1;\n}\n",
+		"<?php\nif ($b)/*a*/// c\n{\n    echo 1;\n}\n",
+		"<?php\nif ($b) /*a*/# c\n{\n    echo 1;\n}\n",
+		"<?php\nif ($b) /*a*/// c\n/*d*/{\n    echo 1;\n}\n",
+	} {
+		got, _ := apply(t, BracesPosition{}, src)
+		for line := range strings.Lines(got) {
+			if comment := strings.Index(line, "//"); comment >= 0 && strings.Contains(line[comment:], "{") {
+				t.Errorf("%s got=%q", src, got)
+			}
+			if comment := strings.Index(line, "#"); comment >= 0 && strings.Contains(line[comment:], "{") {
+				t.Errorf("%s got=%q", src, got)
+			}
+		}
+	}
+}
+
+// PHP-CS-Fixer ends the comment before the space, so its output is no reference: the brace
+// must not go past the close tag, where it would be inline HTML.
+func TestBracesPositionBraceStaysBeforeCloseTag(t *testing.T) {
+	t.Parallel()
+	got, _ := apply(t, BracesPosition{}, "<?php\nfunction f() { // e ?>\n<b>x</b>\n<?php\n}\n")
+	if want := "<?php\nfunction f()\n{ // e ?>\n<b>x</b>\n<?php\n}\n"; got != want {
+		t.Fatalf("\n got=%q\nwant=%q", got, want)
+	}
+}

blink/internal/fixer/rules/braces_position_comment_next_line_test.go

@@ -0,0 +1,29 @@
+package rules
+
+import "testing"
+
+// The expected output of every case comes from the PHP engine.
+func TestBracesPositionCommentNextLine(t *testing.T) {
+	t.Parallel()
+	cases := []struct{ src, want string }{
+		{"if ($b) { // c\n    echo 1;\n}", "if ($b) // c\n{echo 1;\n}"},
+		{"if ($b) { # c\n    echo 1;\n}", "if ($b) # c\n{echo 1;\n}"},
+		{"if ($b) { /* c */\n    echo 1;\n}", "if ($b)\n{ /* c */\n    echo 1;\n}"},
+		{"if ($b)\n{ // c\n    echo 1;\n}", "if ($b)\n{ // c\n    echo 1;\n}"},
+		{"foreach ($a as $b) { // c\n    echo 1;\n}", "foreach ($a as $b) // c\n{echo 1;\n}"},
+		{"try { // c\n    echo 1;\n} catch (E $e) { // e\n    echo 2;\n}", "try // c\n{echo 1;\n} catch (E $e) // e\n{echo 2;\n}"},
+		{"$a = new class { // c\n    public $x;\n};", "$a = new class // c\n{public $x;\n};"},
+		{"$a = new class($b) implements C { // c\n    public $x;\n};", "$a = new class($b) implements C // c\n{public $x;\n};"},
+		{"$a = new class /* c */ {\n    public $x;\n};", "$a = new class /* c */\n{\n    public $x;\n};"},
+		{"$f = function () { // c\n    echo 1;\n};", "$f = function () // c\n{echo 1;\n};"},
+		{"$f = function () use ($a) { // c\n    echo 1;\n};", "$f = function () use ($a) // c\n{echo 1;\n};"},
+		{"$f = function () /* c */ {\n    echo 1;\n};", "$f = function () /* c */\n{\n    echo 1;\n};"},
+		{"if ($b) // c\n{\n    echo 1;\n}", "if ($b) // c\n{\n    echo 1;\n}"},
+	}
+	for _, c := range cases {
+		got, changed := apply(t, BracesPosition{}.WithConfig(map[string]any{"control_structures_opening_brace": bracesNextLine, "anonymous_classes_opening_brace": bracesNextLine, "anonymous_functions_opening_brace": bracesNextLine}).(fixerRule), "<?php\n"+c.src+"\n")
+		if want := "<?php\n" + c.want + "\n"; got != want || changed != (c.src != c.want) {
+			t.Errorf("%s\n got=%q changed=%v\nwant=%q", c.src, got, changed, want)
+		}
+	}
+}

blink/internal/fixer/rules/braces_position_comment_test.go

@@ -0,0 +1,57 @@
+package rules
+
+import "testing"
+
+// The expected output of every case comes from the PHP engine.
+func TestBracesPositionComment(t *testing.T) {
+	t.Parallel()
+	cases := []struct{ src, want string }{
+		{"if ($b) // c\n{\n    echo 1;\n}", "if ($b) { // c\necho 1;\n}"},
+		{"if ($b) # c\n{\n    echo 1;\n}", "if ($b) { # c\necho 1;\n}"},
+		{"if ($b) /* c */\n{\n    echo 1;\n}", "if ($b) { /* c */\necho 1;\n}"},
+		{"if ($b) /* c */ {\n    echo 1;\n}", "if ($b) { /* c */\n    echo 1;\n}"},
+		{"if ($b)/* c */{\n    echo 1;\n}", "if ($b) {/* c */\n    echo 1;\n}"},
+		{"if ($b) /** d */\n{\n    echo 1;\n}", "if ($b) { /** d */\necho 1;\n}"},
+		{"if ($b)\n// c\n{\n    echo 1;\n}", "if ($b) {\n// c\necho 1;\n}"},
+		{"if ($b) // c\n// e\n{\n    echo 1;\n}", "if ($b) { // c\n// e\necho 1;\n}"},
+		{"if ($b) // c\n/* e */\n{\n    echo 1;\n}", "if ($b) { // c\n/* e */\necho 1;\n}"},
+		{"if ($b) // c\n\n{\n    echo 1;\n}", "if ($b) { // c\n\necho 1;\n}"},
+		{"if ($b)// c\n{\n    echo 1;\n}", "if ($b) {// c\necho 1;\n}"},
+		{"if ($b) { // c\n    echo 1;\n}", "if ($b) { // c\n    echo 1;\n}"},
+		{"if ($b) // c\n{ // e\n    echo 1;\n}", "if ($b) { // c\n// e\n    echo 1;\n}"},
+		{"foreach ($a as $b) // c\n{\n    echo 1;\n}", "foreach ($a as $b) { // c\necho 1;\n}"},
+		{"while ($b) // c\n{\n    echo 1;\n}", "while ($b) { // c\necho 1;\n}"},
+		{"for (;;) // c\n{\n    echo 1;\n}", "for (;;) { // c\necho 1;\n}"},
+		{"do // c\n{\n    echo 1;\n} while ($b);", "do { // c\necho 1;\n} while ($b);"},
+		{"try // c\n{\n    echo 1;\n} catch (E $e) // e\n{\n} finally // f\n{\n}", "try { // c\necho 1;\n} catch (E $e) { // e\n} finally { // f\n}"},
+		{"switch ($b) // c\n{\n    case 1:\n}", "switch ($b) { // c\ncase 1:\n}"},
+		{"if ($b) {\n} else // c\n{\n    echo 1;\n}", "if ($b) {\n} else { // c\necho 1;\n}"},
+		{"if ($b) {\n} elseif ($c) // c\n{\n    echo 1;\n}", "if ($b) {\n} elseif ($c) { // c\necho 1;\n}"},
+		{"$a = new class // c\n{\n    public $x;\n};", "$a = new class { // c\npublic $x;\n};"},
+		{"$a = new class # c\n{\n    public $x;\n};", "$a = new class { # c\npublic $x;\n};"},
+		{"$a = new class /* c */\n{\n    public $x;\n};", "$a = new class { /* c */\npublic $x;\n};"},
+		{"$a = new class /* c */ {\n    public $x;\n};", "$a = new class { /* c */\n    public $x;\n};"},
+		{"$a = new class($b) // c\n{\n    public $x;\n};", "$a = new class($b) { // c\npublic $x;\n};"},
+		{"$a = new class($b) implements C // c\n{\n    public $x;\n};", "$a = new class($b) implements C { // c\npublic $x;\n};"},
+		{"$a = new class\n// c\n{\n    public $x;\n};", "$a = new class {\n// c\npublic $x;\n};"},
+		{"class A // c\n{\n}", "class A // c\n{\n}"},
+		{"class A /* c */ {\n}", "class A /* c */\n{\n}"},
+		{"class A // c\n{\n    public $x;\n}", "class A // c\n{\n    public $x;\n}"},
+		{"class A { // c\n    public $x;\n}", "class A // c\n{public $x;\n}"},
+		{"class A { /* c */\n    public $x;\n}", "class A\n{ /* c */\n    public $x;\n}"},
+		{"class A { # c\n    public $x;\n}", "class A # c\n{public $x;\n}"},
+		{"class A { /** d */\n    public $x;\n}", "class A\n{ /** d */\n    public $x;\n}"},
+		{"class A\n{ // c\n    public $x;\n}", "class A\n{ // c\n    public $x;\n}"},
+		{"class A {// c\n    public $x;\n}", "class A // c\n{public $x;\n}"},
+		{"function f() // c\n{\n}", "function f() // c\n{\n}"},
+		{"function f() { // c\n    echo 1;\n}", "function f() // c\n{echo 1;\n}"},
+		{"function f(): void // c\n{\n    echo 1;\n}", "function f(): void // c\n{\n    echo 1;\n}"},
+		{"function f() /* c */ {\n    echo 1;\n}", "function f() /* c */\n{\n    echo 1;\n}"},
+	}
+	for _, c := range cases {
+		got, changed := apply(t, BracesPosition{}, "<?php\n"+c.src+"\n")
+		if want := "<?php\n" + c.want + "\n"; got != want || changed != (c.src != c.want) {
+			t.Errorf("%s\n got=%q changed=%v\nwant=%q", c.src, got, changed, want)
+		}
+	}
+}

blink/internal/fixer/rules/braces_position_readonly_anonymous_test.go

@@ -0,0 +1,49 @@
+package rules
+
+import "testing"
+
+// The expected output of every case comes from the PHP engine.
+func TestBracesPositionReadonlyAnonymousClass(t *testing.T) {
+	t.Parallel()
+	cases := []struct{ src, want string }{
+		{"$a = new class {};", "$a = new class {};"},
+		{"$a = new class {\n};", "$a = new class {\n};"},
+		{"$a = new class\n{\n};", "$a = new class {\n};"},
+		{"$a = new readonly class {\n};", "$a = new readonly class {\n};"},
+		{"$a = new readonly class\n{\n};", "$a = new readonly class {\n};"},
+		{"$a = new readonly class() {\n};", "$a = new readonly class() {\n};"},
+		{"$a = new readonly class()\n{\n};", "$a = new readonly class() {\n};"},
+		{"$a = new readonly class($b) {\n};", "$a = new readonly class($b) {\n};"},
+		{"$a = new readonly class($b) implements C {\n};", "$a = new readonly class($b) implements C {\n};"},
+		{"$a = new readonly class($b) implements C\n{\n};", "$a = new readonly class($b) implements C {\n};"},
+		{"$a = new readonly class($b) extends D implements C {\n};", "$a = new readonly class($b) extends D implements C {\n};"},
+		{"$a = new READONLY class {\n};", "$a = new READONLY class {\n};"},
+		{"$a = new #[Foo] class {\n};", "$a = new #[Foo] class {\n};"},
+		{"$a = new #[Foo] readonly class {\n};", "$a = new #[Foo] readonly class {\n};"},
+		{"$a = new #[Foo] readonly class\n{\n};", "$a = new #[Foo] readonly class {\n};"},
+		{"$a = new /* c */ readonly class {\n};", "$a = new /* c */ readonly class {\n};"},
+		{"$a = new readonly /* c */ class {\n};", "$a = new readonly /* c */ class {\n};"},
+		{"$a = new\n    readonly class {\n};", "$a = new\n    readonly class {\n};"},
+		{"$a = new readonly\n    class {\n};", "$a = new readonly\n    class {\n};"},
+		{"foo(new readonly class {\n});", "foo(new readonly class {\n});"},
+		{"foo(new readonly class($b) implements C {\n}, 1);", "foo(new readonly class($b) implements C {\n}, 1);"},
+		{"return new readonly class {\n};", "return new readonly class {\n};"},
+		{"readonly class A {\n}", "readonly class A\n{\n}"},
+		{"readonly class A\n{\n}", "readonly class A\n{\n}"},
+		{"final readonly class A {\n}", "final readonly class A\n{\n}"},
+		{"readonly final class A {\n}", "readonly final class A\n{\n}"},
+		{"abstract readonly class A {\n}", "abstract readonly class A\n{\n}"},
+		{"#[Foo] readonly class A {\n}", "#[Foo] readonly class A\n{\n}"},
+		{"$a = new readonly class // c\n{\n    public $x;\n};", "$a = new readonly class { // c\npublic $x;\n};"},
+		{"$a = new readonly class # c\n{\n    public $x;\n};", "$a = new readonly class { # c\npublic $x;\n};"},
+		{"$a = new readonly class($b) implements C // c\n{\n    public $x;\n};", "$a = new readonly class($b) implements C { // c\npublic $x;\n};"},
+		{"$a = new readonly class\n// c\n{\n    public $x;\n};", "$a = new readonly class {\n// c\npublic $x;\n};"},
+		{"$a = new readonly class /* c */ {\n    public $x;\n};", "$a = new readonly class { /* c */\n    public $x;\n};"},
+	}
+	for _, c := range cases {
+		got, changed := apply(t, BracesPosition{}, "<?php\n"+c.src+"\n")
+		if want := "<?php\n" + c.want + "\n"; got != want || changed != (c.src != c.want) {
+			t.Errorf("%s\n got=%q changed=%v\nwant=%q", c.src, got, changed, want)
+		}
+	}
+}

blink/internal/fixer/rules/method_argument_space.go

@@ -665,7 +665,7 @@ func reflowAfterComma(s *tokens.Stream, comma int, base string, collapseBlanks b
 		// only method_argument_space normalizes the comma-to-comment gap to one
 		// space; the Symplify array/standalone-line reflows (collapseBlanks=false)
 		// keep the author's alignment, matching their PHP fixers
-		if collapseBlanks {
+		if collapseBlanks && !masCommentLastLine(s, n) {
 			if ws >= 0 {
 				if s.At(ws).Value != " " {
 					s.SetValue(ws, " ")