From 662c8f91e05c17b3b2b343dfc9f9f52e6b11e18c Mon Sep 17 00:00:00 2001 From: "erpeldinger.g" Date: Wed, 4 Nov 2026 19:10:48 +0100 Subject: [PATCH 1/4] Blink: failing tests for clone parentheses and ConstantCase names The expected output of the tables comes from the PHP engine. The fix follows. --- .../fixer/rules/casing_constant_names_test.go | 96 ++++++++++++ ...unneeded_control_parentheses_clone_test.go | 140 ++++++++++++++++++ ...nneeded_control_parentheses_config_test.go | 24 +++ blink/internal/fixer/rules/rules_test.go | 3 + 4 files changed, 263 insertions(+) create mode 100644 blink/internal/fixer/rules/casing_constant_names_test.go create mode 100644 blink/internal/fixer/rules/no_unneeded_control_parentheses_clone_test.go diff --git a/blink/internal/fixer/rules/casing_constant_names_test.go b/blink/internal/fixer/rules/casing_constant_names_test.go new file mode 100644 index 0000000000..5bb2dc5e18 --- /dev/null +++ b/blink/internal/fixer/rules/casing_constant_names_test.go @@ -0,0 +1,96 @@ +package rules + +import "testing" + +// The expected output of every case comes from the PHP engine. +func TestConstantCaseNames(t *testing.T) { + t.Parallel() + cases := []struct{ src, a, b string }{ + {"$a = TRUE; $b = NULL; $c = False;", "$a = true; $b = null; $c = false;", "$a = TRUE; $b = NULL; $c = FALSE;"}, + {"const NULL = 1;", "const NULL = 1;", "const NULL = 1;"}, + {"const TRUE = 1, FALSE = 2;", "const TRUE = 1, FALSE = 2;", "const TRUE = 1, FALSE = 2;"}, + {"class A { const NULL = 1; }", "class A { const NULL = 1; }", "class A { const NULL = 1; }"}, + {"class A { const string NULL = 'n'; }", "class A { const string NULL = 'n'; }", "class A { const string NULL = 'n'; }"}, + {"class A { public const int|string TRUE = 1; }", "class A { public const int|string TRUE = 1; }", "class A { public const int|string TRUE = 1; }"}, + {"class A { const NULL /* c */ = 1; }", "class A { const NULL /* c */ = 1; }", "class A { const NULL /* c */ = 1; }"}, + {"echo A::NULL;", "echo A::NULL;", "echo A::NULL;"}, + {"echo A:: /* c */ NULL;", "echo A:: /* c */ NULL;", "echo A:: /* c */ NULL;"}, + {"echo $a->TRUE;", "echo $a->TRUE;", "echo $a->TRUE;"}, + {"echo $a?->TRUE;", "echo $a?->TRUE;", "echo $a?->TRUE;"}, + {"echo $a -> /* c */ TRUE;", "echo $a -> /* c */ TRUE;", "echo $a -> /* c */ TRUE;"}, + {"new NULL;", "new NULL;", "new NULL;"}, + {"new Null();", "new Null();", "new Null();"}, + {"new /* c */ Null();", "new /* c */ Null();", "new /* c */ Null();"}, + {"class A extends False {}", "class A extends False {}", "class A extends False {}"}, + {"class A implements True {}", "class A implements True {}", "class A implements True {}"}, + {"class A implements B, True {}", "class A implements B, true {}", "class A implements B, TRUE {}"}, + {"interface I extends True {}", "interface I extends True {}", "interface I extends True {}"}, + {"$a instanceof NULL;", "$a instanceof NULL;", "$a instanceof NULL;"}, + {"namespace Null;", "namespace Null;", "namespace Null;"}, + {"\\NULL;", "\\NULL;", "\\NULL;"}, + {"\\Null\\foo();", "\\Null\\foo();", "\\Null\\foo();"}, + {"Null\\foo();", "Null\\foo();", "Null\\foo();"}, + {"Foo\\TRUE;", "Foo\\TRUE;", "Foo\\TRUE;"}, + {"namespace\\TRUE;", "namespace\\TRUE;", "namespace\\TRUE;"}, + {"NULL::foo();", "NULL::foo();", "NULL::foo();"}, + {"True::BAR;", "True::BAR;", "True::BAR;"}, + {"Null /* c */ ::foo();", "Null /* c */ ::foo();", "Null /* c */ ::foo();"}, + {"enum E { case NULL; case True; }", "enum E { case NULL; case True; }", "enum E { case NULL; case True; }"}, + {"enum E: int { case NULL = 1; }", "enum E: int { case NULL = 1; }", "enum E: int { case NULL = 1; }"}, + {"switch ($a) { case NULL: break; }", "switch ($a) { case null: break; }", "switch ($a) { case NULL: break; }"}, + {"switch ($a) { case TRUE: case FALSE; }", "switch ($a) { case true: case FALSE; }", "switch ($a) { case TRUE: case FALSE; }"}, + {"switch ($a) { case NULL /* c */ ; }", "switch ($a) { case NULL /* c */ ; }", "switch ($a) { case NULL /* c */ ; }"}, + {"function f(): NULL {}", "function f(): null {}", "function f(): NULL {}"}, + {"function f(NULL $a) {}", "function f(null $a) {}", "function f(NULL $a) {}"}, + {"function f(?True $a, False|NULL $b = NULL) {}", "function f(?true $a, false|null $b = null) {}", "function f(?TRUE $a, FALSE|NULL $b = NULL) {}"}, + {"$f = fn (): NULL => NULL;", "$f = fn (): null => null;", "$f = fn (): NULL => NULL;"}, + {"foo(NULL, TRUE);", "foo(null, true);", "foo(NULL, TRUE);"}, + {"foo(NULL: 1);", "foo(NULL: 1);", "foo(NULL: 1);"}, + {"$a = [NULL => TRUE];", "$a = [null => true];", "$a = [NULL => TRUE];"}, + {"interface I { function TRUE(); }", "interface I { function true(); }", "interface I { function TRUE(); }"}, + {"function NULL() {}", "function null() {}", "function NULL() {}"}, + {"NULL();", "null();", "NULL();"}, + {"use Foo\\NULL;", "use Foo\\NULL;", "use Foo\\NULL;"}, + {"use NULL as B;", "use null as B;", "use NULL as B;"}, + {"use Foo as NULL;", "use Foo as null;", "use Foo as NULL;"}, + {"class NULL {}", "class null {}", "class NULL {}"}, + {"#[NULL] class A {}", "#[null] class A {}", "#[NULL] class A {}"}, + {"goto NULL;", "goto null;", "goto NULL;"}, + {"NULL: echo 1;", "null: echo 1;", "NULL: echo 1;"}, + {"\"{$a[NULL]}\";", "\"{$a[null]}\";", "\"{$a[NULL]}\";"}, + {"$a = NULL ?? TRUE;", "$a = null ?? true;", "$a = NULL ?? TRUE;"}, + {"$a == NULL;", "$a == null;", "$a == NULL;"}, + {"NULL === $a;", "null === $a;", "NULL === $a;"}, + {"class A { public NULL $x; public ?True $y = FALSE; }", "class A { public null $x; public ?true $y = false; }", "class A { public NULL $x; public ?TRUE $y = FALSE; }"}, + {"class A { public function __construct(private NULL $a = NULL) {} }", "class A { public function __construct(private null $a = null) {} }", "class A { public function __construct(private NULL $a = NULL) {} }"}, + {"const ABC = NULL;", "const ABC = null;", "const ABC = NULL;"}, + {"foo(1, TRUE: 2);", "foo(1, TRUE: 2);", "foo(1, TRUE: 2);"}, + {"foo(a: 1, NULL: 2);", "foo(a: 1, NULL: 2);", "foo(a: 1, NULL: 2);"}, + {"$a = $f ? NULL : TRUE;", "$a = $f ? null : true;", "$a = $f ? NULL : TRUE;"}, + {"$a = foo($f ? NULL : TRUE);", "$a = foo($f ? null : true);", "$a = foo($f ? NULL : TRUE);"}, + {"$a = [$f ? 1 : NULL];", "$a = [$f ? 1 : null];", "$a = [$f ? 1 : NULL];"}, + {"match ($a) { NULL => 1, default => TRUE };", "match ($a) { null => 1, default => true };", "match ($a) { NULL => 1, default => TRUE };"}, + {"$a = NULL;", "$a = null;", "$a = NULL;"}, + {"if ($a = NULL) {}", "if ($a = null) {}", "if ($a = NULL) {}"}, + {"NULL == TRUE;", "null == true;", "NULL == TRUE;"}, + {"new class extends Foo { const TRUE = 1; };", "new class extends Foo { const TRUE = 1; };", "new class extends Foo { const TRUE = 1; };"}, + {"<<b);", "$r = clone $a->b;"}, + {"$r = clone ($a::$b);", "$r = clone $a::$b;"}, + {"$r = clone ($a[0]);", "$r = clone $a[0];"}, + {"$r = clone (foo());", "$r = clone foo();"}, + {"$r = clone (new Foo());", "$r = clone new Foo();"}, + {"$r = clone (new Foo);", "$r = clone new Foo;"}, + {"$r = clone (new Foo(1 + 2));", "$r = clone new Foo(1 + 2);"}, + {"$r = clone (Foo::bar());", "$r = clone Foo::bar();"}, + {"$r = clone (\\Foo\\bar());", "$r = clone \\Foo\\bar();"}, + {"$r = clone ($a ?: $b);", "$r = clone ($a ?: $b);"}, + {"$r = clone ($a ?? $b);", "$r = clone ($a ?? $b);"}, + {"$r = clone ($f ? $a : $b);", "$r = clone ($f ? $a : $b);"}, + {"$r = clone ($a, ['x' => 1]);", "$r = clone ($a, ['x' => 1]);"}, + {"$r = clone ($a, ['x' => 1],);", "$r = clone ($a, ['x' => 1],);"}, + {"$r = clone (object: $a, withProperties: ['x' => 1]);", "$r = clone (object: $a, withProperties: ['x' => 1]);"}, + {"$r = clone(...);", "$r = clone(...);"}, + {"$r = clone ($a)->b;", "$r = clone ($a)->b;"}, + {"$r = $f ? clone ($a) : $b;", "$r = $f ? clone $a : $b;"}, + {"$r = $f ? $a : clone ($b);", "$r = $f ? $a : clone ($b);"}, + {"$r = $f ?clone ($a):$b;", "$r = $f ?clone $a :$b;"}, + {"$r = $a ?? clone ($b);", "$r = $a ?? clone ($b);"}, + {"$r = $f && clone ($a);", "$r = $f && clone ($a);"}, + {"$r = [clone ($a), clone ($b)];", "$r = [clone $a, clone $b];"}, + {"$r = [1, clone ($a), 2];", "$r = [1, clone $a, 2];"}, + {"$r = foo(clone ($a));", "$r = foo(clone $a);"}, + {"$r = foo(1, clone ($a));", "$r = foo(1, clone $a);"}, + {"$r = (clone ($a));", "$r = (clone $a);"}, + {"$r = fn () => clone ($a);", "$r = fn () => clone $a;"}, + {"$r = fn (): Foo => clone ($a);", "$r = fn (): Foo => clone $a;"}, + {"$r = fn (): array => clone ($a);", "$r = fn (): array => clone ($a);"}, + {"$r = fn (): ?Foo => clone ($a);", "$r = fn (): ?Foo => clone ($a);"}, + {"$r = fn (): A|B => clone ($a);", "$r = fn (): A|B => clone $a;"}, + {"$r = static fn &() => clone ($a);", "$r = static fn &() => clone $a;"}, + {"$o->p = clone ($a);", "$o->p = clone $a;"}, + {"$l[] = clone ($a);", "$l[] = clone $a;"}, + {"$r += clone ($a);", "$r += clone ($a);"}, + {"clone ($a);", "clone $a;"}, + {"clone ($a, ['x' => 1]);", "clone ($a, ['x' => 1]);"}, + {"echo clone ($a);", "echo clone $a;"}, + {"echo clone ($a ?: $b);", "echo clone ($a ?: $b);"}, + {"print clone ($a);", "print clone $a;"}, + {"return clone ($a);", "return clone $a;"}, + {"return clone ($a ?: $b);", "return clone ($a ?: $b);"}, + {"return clone ($a, ['x' => 1]);", "return clone ($a, ['x' => 1]);"}, + {"throw clone ($a);", "throw clone $a;"}, + {"yield clone ($a);", "yield clone $a;"}, + {"yield from clone ($a);", "yield from clone $a;"}, + {"for (clone ($a); clone ($b); clone ($c)) {}", "for (clone $a; clone $b; clone $c) {}"}, + {"if ($f) { clone ($a); }", "if ($f) { clone $a; }"}, + {"if ($f) { clone ($a); } clone ($b);", "if ($f) { clone $a; } clone $b;"}, + {"switch ($a) { case clone ($b): break; }", "switch ($a) { case clone $b : break; }"}, + {"$o->{'a'} = 1; clone ($a);", "$o->{'a'} = 1; clone $a;"}, + {"$r = $o->clone($a);", "$r = $o->clone($a);"}, + {"$r = $o?->clone($a);", "$r = $o?->clone($a);"}, + {"$r = Foo::clone($a);", "$r = Foo::clone($a);"}, + {"interface I { public function clone($a); }", "interface I { public function clone($a); }"}, + {"$r = clone /* c */ ($a);", "$r = clone /* c */ $a;"}, + {"$r = clone ( /* c */ $a);", "$r = clone /* c */ $a;"}, + {"$r = clone ($a /* c */);", "$r = clone $a /* c */;"}, + {"$r = clone (\"x\");", "$r = clone \"x\";"}, + {"$r = clone (__CLASS__);", "$r = clone __CLASS__;"}, + {"$r = clone (static::foo());", "$r = clone static::foo();"}, + {"$r = clone ($a->b());", "$r = clone $a->b();"}, + {"$r = clone ($$a);", "$r = clone $$a;"}, + {"$r = clone ($a::class);", "$r = clone ($a::class);"}, + {"$r = clone (-$a);", "$r = clone (-$a);"}, + {"$r = clone ((object) $a);", "$r = clone ((object) $a);"}, + {"$r = clone (\"a$b\");", "$r = clone (\"a$b\");"}, + {"$r = clone ($a->class);", "$r = clone $a->class;"}, + {"$r = clone (1);", "$r = clone 1;"}, + {"switch ($a) { case 1: { } clone ($b); }", "switch ($a) { case 1: { } clone $b; }"}, + {"yield from clone ($a);", "yield from clone $a;"}, + {"yield\nfrom clone ($a);", "yield\nfrom clone $a;"}, + {"yield /* c */ from clone ($a);", "yield /* c */ from clone $a;"}, + {"$r = new (clone ($a));", "$r = new (clone $a);"}, + {"$x ? 1 : 2; if ($a): { } clone ($b); endif;", "$x ? 1 : 2; if ($a): { } clone ($b); endif;"}, + {"$x ? 1 : 2; switch ($a) { case 1: { } clone ($b); }", "$x ? 1 : 2; switch ($a) { case 1: { } clone $b; }"}, + {"Foo::{'a'}(); clone ($b);", "Foo::{'a'}(); clone $b;"}, + {"$r = clone (<<<'EOT'\nx\nEOT);", "$r = clone (<<<'EOT'\nx\nEOT);"}, + {"$r = clone (<< 1);", "$r = clone (fn () => 1);"}, + {"$r = clone (match ($a) { 1 => 2 });", "$r = clone (match ($a) { 1 => 2 });"}, + {"$r = clone ($a->eval);", "$r = clone $a->eval;"}, + {"$r = clone ($a->namespace);", "$r = clone $a->namespace;"}, + {"$r = clone ($a?->exit());", "$r = clone $a?->exit();"}, + {"$r = clone (Foo::die());", "$r = clone Foo::die();"}, + {"$r = clone ($a->__PROPERTY__);", "$r = clone $a->__PROPERTY__;"}, + {"$r = clone ($a -> b);", "$r = clone $a -> b;"}, + {"$r = $f ? clone (object) $a : $b;", "$r = $f ? clone (object) $a : $b;"}, + {"$r = $f ? clone (object) ['a' => 1] : $b;", "$r = $f ? clone (object) ['a' => 1] : $b;"}, + {"$r = $f ? clone (int) $a : $b;", "$r = $f ? clone (int) $a : $b;"}, + {"clone (object) $a;", "clone (object) $a;"}, + {"$r = clone (App\\Namespace\\Foo::make());", "$r = clone App\\Namespace\\Foo::make();"}, + {"$r = clone (\\Foo\\die());", "$r = clone \\Foo\\die();"}, + {"$r = clone (Exit\\Foo::bar());", "$r = clone Exit\\Foo::bar();"}, + {"$r = clone (Foo\\Eval\\bar());", "$r = clone Foo\\Eval\\bar();"}, + {"$r = clone (Foo::/* c */die());", "$r = clone Foo::/* c */die();"}, + {"$r = clone (eval\\foo());", "$r = clone eval\\foo();"}, + {"$r = clone();", "$r = clone();"}, + {"$f ? clone() : 1;", "$f ? clone() : 1;"}, + {"$r = clone ($a) ?>", "$r = clone $a ?>"}, + {"clone ($a) ?>", "clone $a ?>"}, + {"?>", "?>"}, + {"foo(fn () => clone ($a), 1);", "foo(fn () => clone $a, 1);"}, + {"switch ($a) { case clone ($b); }", "switch ($a) { case clone $b; }"}, + {"$r = clone (\n int\n);", "$r = clone \n int\n;"}, + } + for _, c := range cases { + got, changed := apply(t, NoUnneededControlParentheses{}, "b", "$a::$b", "FOO", "K::X", "__CLASS__", "1", "enum", "new Foo", "new Foo()", "new $a", "(new Foo)"} + for _, operand := range operands { + for _, next := range []string{"->b", "?->b", "::b", "[0]", "()", " and $g", " + 1", " instanceof Foo", " ?: 1"} { + src := "TRUE;"); changed { t.Fatal("member named TRUE should not be touched") } + if _, changed := apply(t, ConstantCase{}, " Date: Wed, 4 Nov 2026 19:11:53 +0100 Subject: [PATCH 2/4] Blink: keep needed clone parentheses, and only change constants in ConstantCase clone was handled like return, so "clone ($a, [...])" became a parse error and "clone ($a ?: $b)" lost its meaning. Follow PHP-CS-Fixer's isWrappedCloneArgument. Different on purpose: after "?", the pair comes off only directly before ":", because PHP-CS-Fixer's output for "$f ? clone (new Foo)->bar : 1" is a parse error. ConstantCase changed every name that looks like true, false or null, so "const NULL" became "const null" and "self::NULL" failed at runtime. Follow PHP-CS-Fixer's checks. --- blink/internal/fixer/rules/casing.go | 28 +- .../rules/no_unneeded_control_parentheses.go | 252 +++++++++++++++++- 2 files changed, 274 insertions(+), 6 deletions(-) diff --git a/blink/internal/fixer/rules/casing.go b/blink/internal/fixer/rules/casing.go index a5b8bbe3bb..e92a4a23d7 100644 --- a/blink/internal/fixer/rules/casing.go +++ b/blink/internal/fixer/rules/casing.go @@ -82,8 +82,8 @@ func (f ConstantCase) Fix(s *tokens.Stream) bool { if lower != "true" && lower != "false" && lower != "null" { continue } - if memberPrev(s, i) { - continue // $obj->true, Foo::null - a member name, not the constant + if !isConstantCaseFixable(s, i) { + continue } want := lower if f.Upper { @@ -97,6 +97,30 @@ func (f ConstantCase) Fix(s *tokens.Stream) bool { return changed } +// isConstantCaseFixable follows PHP-CS-Fixer's list of the places where it keeps a +// true, false or null as written. It is no test for the constant itself: like the PHP +// fixer, it also changes "class NULL {}" and "function NULL() {}". +func isConstantCaseFixable(s *tokens.Stream, i int) bool { + prev, next := prevMeaningfulIndex(s, i), nextMeaningfulIndex(s, i) + for _, word := range []string{"extends", "implements", "instanceof", "namespace", "new"} { + if taKw(s, prev, word) { + return false + } + } + for _, operator := range []string{"::", "->", "?->", `\`} { + if isPunctVal(s, prev, operator) { + return false + } + } + if taKw(s, prev, "case") && isPunctVal(s, next, ";") { + return false + } + if taIsNamedArgument(s, i) { + return false + } + return !isPunctVal(s, next, "::") && !isPunctVal(s, next, `\`) && !isPunctVal(s, next, "=") +} + // PHP-CS-Fixer: https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/blob/master/src/Fixer/Casing/LowercaseStaticReferenceFixer.php // // LowercaseStaticReference lowercases self, static and parent. diff --git a/blink/internal/fixer/rules/no_unneeded_control_parentheses.go b/blink/internal/fixer/rules/no_unneeded_control_parentheses.go index 984de671d5..8f96b120d3 100644 --- a/blink/internal/fixer/rules/no_unneeded_control_parentheses.go +++ b/blink/internal/fixer/rules/no_unneeded_control_parentheses.go @@ -13,11 +13,11 @@ import ( // // NoUnneededControlParentheses removes a redundant parenthesis pair wrapping the // whole operand of a control statement: "return ($x);" -> "return $x;". It fires -// for return, echo, print, yield, break, continue, clone and case, only when the -// "(" directly follows the keyword and its matching ")" sits right before the +// for return, echo, print, yield, break, continue and case, only when the "(" +// directly follows the keyword and its matching ")" sits right before the // statement terminator (";" or, for case, ":"). Nested pairs are peeled in one // pass. The conservative boundary check leaves anything else (multiple arguments, -// a partial sub-expression, a mid-expression clone) untouched. +// a partial sub-expression) untouched. clone has its own rule, see unwrapClone. type NoUnneededControlParentheses struct { // Statements limits the fixed statements; nil means the default set. Statements []string @@ -73,7 +73,12 @@ func (f NoUnneededControlParentheses) Fix(s *tokens.Stream) bool { var term string kw := strings.ToLower(t.Value) switch kw { - case "return", "echo", "print", "yield", "break", "continue", "clone": + case "clone": + if f.enabled("clone") && unwrapClone(s, i) { + changed = true + } + continue + case "return", "echo", "print", "yield", "break", "continue": term = ";" case "case": term = ":" @@ -120,3 +125,242 @@ func (f NoUnneededControlParentheses) Fix(s *tokens.Stream) bool { } return changed } + +// unwrapClone removes the pairs wrapping the operand of the clone at index i, like +// PHP-CS-Fixer's isWrappedCloneArgument: only where the clone stands alone in its +// statement or sequence element, and only around a plain operand. The parentheses +// around anything else are needed: "clone ($a ?: $b)" clones the result of the +// ternary while "clone $a ?: $b" does not, and "clone ($a, [...])" and "clone(...)" +// are a call. +func unwrapClone(s *tokens.Stream, i int) bool { + changed := false + for { + open := nextMeaningfulIndex(s, i) + if open < 0 || !csbIsPunct(s.At(open), "(") { + return changed + } + closing := s.MatchForward(open) + if closing < 0 || nextMeaningfulIndex(s, open) == closing { + return changed + } + before := prevMeaningfulIndex(s, i) + after := nextMeaningfulIndex(s, closing) + if before < 0 || after < 0 || !cloneStandsAlone(s, before, after) || cloneOperandHasOperation(s, open, closing) { + return changed + } + removeParenthesis(s, closing, csbIsPunct(s.At(after), ":")) + removeParenthesis(s, open, true) + changed = true + } +} + +func cloneStandsAlone(s *tokens.Stream, before, after int) bool { + b, a := s.At(before), s.At(after) + switch { + case csbIsPunct(b, "?"): + // Only before ":". Anything else can then apply to another expression, where + // PHP-CS-Fixer unwraps anyway: "clone ($o->b)()" would call a method, and + // "clone (new Foo)->bar" and "clone (enum) and $b" become parse errors. + return csbIsPunct(a, ":") + case csbIsPunct(b, "=") && endsStatement(a): + return true + case isCloneStatementStart(s, before) && endsStatement(a): + return true + case csbIsKeyword(b, "case") && (csbIsPunct(a, ":") || csbIsPunct(a, ";")): + return true + case isFnBodyStart(s, before) && (endsStatement(a) || csbIsPunct(a, ",")): + return true + case isForElement(s, before, after): + return true + } + return isSequenceElement(b, a) +} + +func endsStatement(t token.Token) bool { + return csbIsPunct(t, ";") || t.Kind == token.CloseTag +} + +func isCloneStatementStart(s *tokens.Stream, before int) bool { + b := s.At(before) + if b.Kind == token.OpenTag || csbIsPunct(b, ";") || csbIsPunct(b, "{") { + return true + } + if csbIsPunct(b, "}") { + return !closesDynamicElement(s, before) + } + if b.Kind != token.Keyword { + return false + } + // yield from is one token that keeps its inner spacing and comments + word := strings.ToLower(b.Value) + return strings.HasPrefix(word, "yield") || slices.Contains([]string{ + "echo", "print", "return", "throw", "break", "continue", "require", "require_once", "include", "include_once", + }, word) +} + +// closesDynamicElement follows PHP-CS-Fixer's closeCurlyBelongsToDynamicElement: a +// "}" whose "{" follows a ":" with no case closer to it than a "?". +func closesDynamicElement(s *tokens.Stream, closing int) bool { + open := s.MatchBackward(closing) + if open < 0 { + return false + } + prev := prevMeaningfulIndex(s, open) + if prev < 0 { + return false + } + if !csbIsPunct(s.At(prev), ":") { + return false + } + for j := prev - 1; j >= 0; j-- { + if csbIsPunct(s.At(j), "?") { + return true + } + if csbIsKeyword(s.At(j), "case") { + return false + } + } + // PHP-CS-Fixer throws when it finds neither: keep the parentheses + return true +} + +// isFnBodyStart reports whether the token at index i is the "=>" of an arrow +// function, with at most a plain return type: "fn () =>", "fn (): A|B =>". +func isFnBodyStart(s *tokens.Stream, i int) bool { + if !csbIsPunct(s.At(i), "=>") { + return false + } + j := prevMeaningfulIndex(s, i) + if j >= 0 && s.At(j).Kind == token.Ident { + for j >= 0 && (s.At(j).Kind == token.Ident || csbIsPunct(s.At(j), "|") || csbIsPunct(s.At(j), "&")) { + j = prevMeaningfulIndex(s, j) + } + if j < 0 || !csbIsPunct(s.At(j), ":") { + return false + } + j = prevMeaningfulIndex(s, j) + } + if j < 0 || !csbIsPunct(s.At(j), ")") { + return false + } + open := s.MatchBackward(j) + if open < 0 { + return false + } + j = prevMeaningfulIndex(s, open) + if j >= 0 && csbIsPunct(s.At(j), "&") { + j = prevMeaningfulIndex(s, j) + } + return j >= 0 && csbIsKeyword(s.At(j), "fn") +} + +// isForElement matches the first and last expressions of a for head; the middle +// one is a statement start already: for (X; Y; Z). +func isForElement(s *tokens.Stream, before, after int) bool { + candidate := -1 + switch { + case csbIsPunct(s.At(before), "(") && csbIsPunct(s.At(after), ";"): + candidate = prevMeaningfulIndex(s, before) + case csbIsPunct(s.At(after), ")") && csbIsPunct(s.At(before), ";"): + if open := s.MatchBackward(after); open >= 0 { + candidate = prevMeaningfulIndex(s, open) + } + } + return candidate >= 0 && csbIsKeyword(s.At(candidate), "for") +} + +func isSequenceElement(before, after token.Token) bool { + beforeComma, afterComma := csbIsPunct(before, ","), csbIsPunct(after, ",") + beforeOpen := csbIsPunct(before, "(") || csbIsPunct(before, "[") || csbIsPunct(before, "{") + afterClose := csbIsPunct(after, ")") || csbIsPunct(after, "]") || csbIsPunct(after, "}") + return (beforeComma || beforeOpen) && (afterComma || afterClose) +} + +// cloneOperandHasOperation scans the content of the pair at open..closing, a nested +// block counting as one operand. A leading new belongs to the operand. +func cloneOperandHasOperation(s *tokens.Stream, open, closing int) bool { + start := open + if first := nextMeaningfulIndex(s, open); first >= 0 && csbIsKeyword(s.At(first), "new") { + start = first + } + for j := nextMeaningfulIndex(s, start); j >= 0 && j != closing; j = nextMeaningfulIndex(s, j) { + t := s.At(j) + if _, _, isCast := castAt(s, j); isCast { + return true + } + if prev := prevMeaningfulIndex(s, j); strings.EqualFold(t.Value, "class") && prev >= 0 && csbIsPunct(s.At(prev), "::") { + return true + } + if t.Kind == token.Ident { + if !isCloneOperandName(s, j) { + return true + } + continue + } + if csbIsPunct(t, "(") || csbIsPunct(t, "[") || csbIsPunct(t, "{") { + end := s.MatchForward(j) + if end < 0 { + return true + } + j = end + continue + } + if !isCloneOperandToken(t) { + return true + } + } + return false +} + +// isCloneOperandName rejects the names that are language constructs to PHP but plain +// names to the lexer, unless they are a member or a segment of a qualified name. +func isCloneOperandName(s *tokens.Stream, i int) bool { + word := strings.ToLower(s.At(i).Value) + if prev := prevMeaningfulIndex(s, i); prev >= 0 && s.At(prev).Kind == token.Punct { + switch s.At(prev).Value { + case "->", "?->", "::", `\`: + return true + } + } + if next := nextMeaningfulIndex(s, i); next >= 0 && csbIsPunct(s.At(next), `\`) { + return word != "namespace" + } + switch word { + case "eval", "exit", "die", "__property__": + return false + } + return true +} + +func isCloneOperandToken(t token.Token) bool { + switch t.Kind { + case token.Variable, token.Number: + return true + case token.String: + return t.IsLiteral() && !t.StartsHeredoc() && !t.IsShellExec() + case token.Punct: + return t.Value == "$" || t.Value == "::" || t.Value == `\` || t.Value == "->" || t.Value == "?->" + case token.Keyword: + return strings.EqualFold(t.Value, "static") + } + return false +} + +func removeParenthesis(s *tokens.Stream, i int, needsSpace bool) { + spaced := func(j int) bool { + if j < 0 || j >= s.Len() { + return false + } + k := s.At(j).Kind + return k == token.Whitespace || k == token.Comment || k == token.DocComment + } + if needsSpace && !spaced(i-1) && !spaced(i+1) { + s.Set(i, token.Token{Kind: token.Whitespace, Value: " "}) + return + } + s.RemoveAt(i) + if i > 0 && i < s.Len() && s.Kind(i-1) == token.Whitespace && s.Kind(i) == token.Whitespace { + s.SetValue(i-1, s.At(i-1).Value+s.At(i).Value) + s.RemoveAt(i) + } +} From 1c421e6f839ff4b103059813222abcfd55dde8a9 Mon Sep 17 00:00:00 2001 From: "erpeldinger.g" Date: Wed, 4 Nov 2026 20:18:45 +0100 Subject: [PATCH 3/4] Blink: failing tests for clone of a plain variable after "?" PHP-CS-Fixer removes these parentheses and the result is right, so blink should too. The expected output comes from the PHP engine. The fix follows. --- .../no_unneeded_control_parentheses_clone_test.go | 15 +++++++++++++++ ...no_unneeded_control_parentheses_config_test.go | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/blink/internal/fixer/rules/no_unneeded_control_parentheses_clone_test.go b/blink/internal/fixer/rules/no_unneeded_control_parentheses_clone_test.go index 4bee4b7122..7a7da5eb8a 100644 --- a/blink/internal/fixer/rules/no_unneeded_control_parentheses_clone_test.go +++ b/blink/internal/fixer/rules/no_unneeded_control_parentheses_clone_test.go @@ -130,6 +130,21 @@ func TestNoUnneededControlParenthesesClone(t *testing.T) { {"foo(fn () => clone ($a), 1);", "foo(fn () => clone $a, 1);"}, {"switch ($a) { case clone ($b); }", "switch ($a) { case clone $b; }"}, {"$r = clone (\n int\n);", "$r = clone \n int\n;"}, + {"$r = $f ? clone ($a)->b : $c;", "$r = $f ? clone $a->b : $c;"}, + {"$r = $f ? clone ($a)[0] : $c;", "$r = $f ? clone $a[0] : $c;"}, + {"$r = $f ? clone ($a)?->b : $c;", "$r = $f ? clone $a?->b : $c;"}, + {"$r = $f ? clone ($a)::$b : $c;", "$r = $f ? clone $a ::$b : $c;"}, + {"$r = $f ? clone ($a)() : $c;", "$r = $f ? clone $a () : $c;"}, + {"$r = $f ? clone ($a) + 1 : $c;", "$r = $f ? clone $a + 1 : $c;"}, + {"$r = $f ? clone ($a) and $g : $c;", "$r = $f ? clone $a and $g : $c;"}, + {"$r = $f ? clone ($a) instanceof Foo : $c;", "$r = $f ? clone $a instanceof Foo : $c;"}, + {"$r = $f ? clone ($a) ?: 1 : $c;", "$r = $f ? clone $a ?: 1 : $c;"}, + {"$r = $f ? clone ( $a )->b : $c;", "$r = $f ? clone $a ->b : $c;"}, + {"$r = $f ? clone (/* c */ $a)->b : $c;", "$r = $f ? clone /* c */ $a->b : $c;"}, + {"$r = $f ? clone (($a))->b : $c;", "$r = $f ? clone $a->b : $c;"}, + {"$r = $f ? clone ($this)->b : $c;", "$r = $f ? clone $this->b : $c;"}, + {"$r = $f ? clone ($a // c\n)->b : $c;", "$r = $f ? clone $a // c\n->b : $c;"}, + {"$r = $f ? clone ($a)?:1 : $c;", "$r = $f ? clone $a ?:1 : $c;"}, } for _, c := range cases { got, changed := apply(t, NoUnneededControlParentheses{}, "b", "$a::$b", "FOO", "K::X", "__CLASS__", "1", "enum", "new Foo", "new Foo()", "new $a", "(new Foo)"} + operands := []string{"$o->b", "$a::$b", "FOO", "K::X", "__CLASS__", "1", "enum", "new Foo", "new Foo()", "new $a", "(new Foo)"} for _, operand := range operands { for _, next := range []string{"->b", "?->b", "::b", "[0]", "()", " and $g", " + 1", " instanceof Foo", " ?: 1"} { src := " Date: Wed, 4 Nov 2026 20:19:34 +0100 Subject: [PATCH 4/4] Blink: remove the parentheses of a plain variable after "?" "$f ? clone ($a)->b : 1" keeps its meaning without them, so follow PHP-CS-Fixer there. Other operands still keep them before anything but ":", where its output breaks. --- .../rules/no_unneeded_control_parentheses.go | 42 +++++++++++++++---- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/blink/internal/fixer/rules/no_unneeded_control_parentheses.go b/blink/internal/fixer/rules/no_unneeded_control_parentheses.go index 8f96b120d3..a3e52c4f16 100644 --- a/blink/internal/fixer/rules/no_unneeded_control_parentheses.go +++ b/blink/internal/fixer/rules/no_unneeded_control_parentheses.go @@ -128,7 +128,8 @@ func (f NoUnneededControlParentheses) Fix(s *tokens.Stream) bool { // unwrapClone removes the pairs wrapping the operand of the clone at index i, like // PHP-CS-Fixer's isWrappedCloneArgument: only where the clone stands alone in its -// statement or sequence element, and only around a plain operand. The parentheses +// statement or sequence element, or around a plain variable after "?", and only around a +// plain operand. The parentheses // around anything else are needed: "clone ($a ?: $b)" clones the result of the // ternary while "clone $a ?: $b" does not, and "clone ($a, [...])" and "clone(...)" // are a call. @@ -145,23 +146,24 @@ func unwrapClone(s *tokens.Stream, i int) bool { } before := prevMeaningfulIndex(s, i) after := nextMeaningfulIndex(s, closing) - if before < 0 || after < 0 || !cloneStandsAlone(s, before, after) || cloneOperandHasOperation(s, open, closing) { + if before < 0 || after < 0 || !cloneStandsAlone(s, open, closing, before, after) || cloneOperandHasOperation(s, open, closing) { return changed } - removeParenthesis(s, closing, csbIsPunct(s.At(after), ":")) + removeParenthesis(s, closing, needsSpaceAfterClone(s.At(after))) removeParenthesis(s, open, true) changed = true } } -func cloneStandsAlone(s *tokens.Stream, before, after int) bool { +func cloneStandsAlone(s *tokens.Stream, open, closing, before, after int) bool { b, a := s.At(before), s.At(after) switch { case csbIsPunct(b, "?"): - // Only before ":". Anything else can then apply to another expression, where - // PHP-CS-Fixer unwraps anyway: "clone ($o->b)()" would call a method, and - // "clone (new Foo)->bar" and "clone (enum) and $b" become parse errors. - return csbIsPunct(a, ":") + // Only before ":" or around a plain variable. Anything else can then apply to + // another expression, where PHP-CS-Fixer unwraps anyway: "clone ($o->b)()" would + // call a method, and "clone (new Foo)->bar" and "clone (enum) and $b" become + // parse errors. + return csbIsPunct(a, ":") || cloneOperandIsVariable(s, open, closing) case csbIsPunct(b, "=") && endsStatement(a): return true case isCloneStatementStart(s, before) && endsStatement(a): @@ -176,6 +178,30 @@ func cloneStandsAlone(s *tokens.Stream, before, after int) bool { return isSequenceElement(b, a) } +// cloneOperandIsVariable reports "($a)" or "(($a))" for the pair at open..closing. +func cloneOperandIsVariable(s *tokens.Stream, open, closing int) bool { + i := nextMeaningfulIndex(s, open) + for csbIsPunct(s.At(i), "(") { + i = nextMeaningfulIndex(s, i) + } + if s.At(i).Kind != token.Variable { + return false + } + for i = nextMeaningfulIndex(s, i); i != closing; i = nextMeaningfulIndex(s, i) { + if !csbIsPunct(s.At(i), ")") { + return false + } + } + return true +} + +func needsSpaceAfterClone(after token.Token) bool { + if endsStatement(after) { + return false + } + return after.Kind != token.Punct || !slices.Contains([]string{",", ")", "]", "}", "->", "?->", "["}, after.Value) +} + func endsStatement(t token.Token) bool { return csbIsPunct(t, ";") || t.Kind == token.CloseTag }