Repository: prettier/plugin-php
Pull request #2320 (2024-01-29 12:46:13)

Make chain breaking as similar to original prettier (js) as much as possible
There is a hardcoded line which is the cause of chain breaks even though the expression would fit in one line. This is a quick and easy solution for https://github.com/prettier/plugin-php/issues/1545

I have adjusted test so they all pass but if this is merged than some of those could be removed as redundant.



eldair: I have updated the pull request to include a new option where user can define their own max calls in a chain before break occurs. The default option is left as is (I think the number 3 is based on similar option in main prettier repo) so it would not affect current codebases.
czosel: Hi @eldair, thanks for contributing! I think adding an option for this doesn't fit to the [Prettier option philosophy](https://prettier.io/docs/en/option-philosophy). Did you already look into how Prettier for JS handles this? It would be awesome if we could solve it the same way.
eldair: Me again, I have started digging into `isSimpleCallArgument` of prettier. I will get back if I manage to port it for php to be on par with js version.
eldair: @czosel Pull request can be checked now. The code is not perfect and I tried to test as many combination as I could think of but it is much closer to the JS counterpart and I think better than the current implementation.
Let me know if something should be changed/fixed/improved.
cseufert: Change looks good to me, however, can you reformat with prettier and commit? See CI / Lint action, at least `print.mjs` needs reformating.
eldair: @cseufert thanks for checking it out, i've fixed the file (on other note, it's weird that vscode prettier formats those lines differently from cli prettier :/ )
czosel: LGTM, thanks for the awesome contribution @eldair! :tada: 

Patch:
diff --git a/src/printer.mjs b/src/printer.mjs
index c822dce..8c7e8fb 100644
--- a/src/printer.mjs
+++ b/src/printer.mjs
@@ -36,6 +36,7 @@ import {
   getAncestorNode,
   isReferenceLikeNode,
   normalizeMagicMethodName,
+  isSimpleCallArgument,
 } from "./util.mjs";
 
 const {
@@ -508,9 +509,9 @@ function printMemberChain(path, options, print) {
     printIndentedGroup(groups.slice(shouldMerge ? 2 : 1)),
   ];
 
-  const callExpressionCount = printedNodes.filter(
-    (tuple) => tuple.node.kind === "call"
-  ).length;
+  const callExpressions = printedNodes.filter((tuple) =>
+    ["call", "new"].includes(tuple.node.kind)
+  );
 
   // We don't want to print in one line if there's:
   //  * A comment.
@@ -519,7 +520,10 @@ function printMemberChain(path, options, print) {
   // If the last group is a function it's okay to inline if it fits.
   if (
     hasComment ||
-    callExpressionCount >= 3 ||
+    (callExpressions.length > 2 &&
+      callExpressions.some(
+        (exp) => !exp.node.arguments.every((arg) => isSimpleCallArgument(arg))
+      )) ||
     printedGroups.slice(0, -1).some(willBreak)
   ) {
     return group(expanded);
diff --git a/src/util.mjs b/src/util.mjs
index f54e9af..6890e7d 100644
--- a/src/util.mjs
+++ b/src/util.mjs
@@ -612,6 +612,79 @@ function normalizeMagicMethodName(name) {
   return name;
 }
 
+/**
+ * @param {string[]} kindsArray
+ * @returns {(node: Node | Comment) => Boolean}
+ */
+function createTypeCheckFunction(kindsArray) {
+  const kinds = new Set(kindsArray);
+  return (node) => kinds.has(node?.kind);
+}
+
+const isSingleWordType = createTypeCheckFunction([
+  "variable",
+  "parameter",
+  "variadic",
+  "clone",
+  "cast",
+  "boolean",
+  "number",
+  "string",
+  "literal",
+  "nullkeyword",
+  "namedargument",
+  "variadicplaceholder",
+]);
+
+const isArrayExpression = createTypeCheckFunction(["array"]);
+const isCallOrNewExpression = createTypeCheckFunction(["call", "new"]);
+const isArrowFuncExpression = createTypeCheckFunction(["arrowfunc"]);
+
+function getChainParts(node, prev = []) {
+  const parts = prev;
+  if (isCallOrNewExpression(node)) {
+    parts.push(node);
+  }
+
+  if (!node.what) {
+    return parts;
+  }
+
+  return getChainParts(node.what, parts);
+}
+
+function isSimpleCallArgument(node, depth = 2) {
+  if (depth <= 0) {
+    return false;
+  }
+
+  const isChildSimple = (child) => isSimpleCallArgument(child, depth - 1);
+
+  if (isSingleWordType(node)) {
+    return true;
+  }
+
+  if (isArrayExpression(node)) {
+    return node.items.every((x) => x === null || isChildSimple(x));
+  }
+
+  if (isCallOrNewExpression(node)) {
+    const parts = getChainParts(node);
+    return (
+      parts.filter((node) => node.kind === "call").length <= depth &&
+      parts.every((node) => node.arguments.every(isChildSimple))
+    );
+  }
+
+  if (isArrowFuncExpression(node)) {
+    return (
+      node.arguments.length <= depth && node.arguments.every(isChildSimple)
+    );
+  }
+
+  return false;
+}
+
 export {
   printNumber,
   getPrecedence,
@@ -641,4 +714,5 @@ export {
   isDocNode,
   getAncestorNode,
   normalizeMagicMethodName,
+  isSimpleCallArgument,
 };


Test patch:
diff --git a/tests/call/__snapshots__/jsfmt.spec.mjs.snap b/tests/call/__snapshots__/jsfmt.spec.mjs.snap
index a56a8dc..94fbc34 100644
--- a/tests/call/__snapshots__/jsfmt.spec.mjs.snap
+++ b/tests/call/__snapshots__/jsfmt.spec.mjs.snap
@@ -705,18 +705,8 @@ $var = $this->foo()();
 $var = ($this->foo->bar)();
 $var = $this->foo->bar()();
 $var = ($this->foo->bar->baz->foo->bar->baz)();
-$var = $this->foo()
-    ->bar()
-    ->baz()
-    ->foo()
-    ->bar()
-    ->baz()();
-$var = ($this->foo()
-    ->bar()
-    ->baz()
-    ->foo()
-    ->bar()
-    ->baz)();
+$var = $this->foo()->bar()->baz()->foo()->bar()->baz()();
+$var = ($this->foo()->bar()->baz()->foo()->bar()->baz)();
 $var = $this["foo"]();
 $var = $this["foo"]["bar"]();
 $var = ($this::$foo)();
@@ -733,10 +723,7 @@ $var = ((["Foo", "bar"])->bar)();
 $var = ($var->foo)()();
 $var = ($var->foo)()();
 $var = (($var->foo)()->bar)();
-$var = ((($var
-    ->foo)()
-    ->bar)()
-    ->baz)();
+$var = ((($var->foo)()->bar)()->baz)();
 
 $obj = call('return new class($value)
 {
diff --git a/tests/member_chain/__snapshots__/jsfmt.spec.mjs.snap b/tests/member_chain/__snapshots__/jsfmt.spec.mjs.snap
index eeaab35..5089bfd 100644
--- a/tests/member_chain/__snapshots__/jsfmt.spec.mjs.snap
+++ b/tests/member_chain/__snapshots__/jsfmt.spec.mjs.snap
@@ -110,6 +110,12 @@ printWidth: 80
 
 $object->foo()->bar()->baz();
 
+$object->foo()->bar()->baz()->foo()->bar()->baz();
+
+$object->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz();
+
+$object->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz();
+
 foo()->bar()->baz();
 
 foo()->bar->baz();
@@ -117,15 +123,34 @@ foo()->bar->baz();
 =====================================output=====================================
 <?php
 
+$object->foo()->bar()->baz();
+
+$object->foo()->bar()->baz()->foo()->bar()->baz();
+
+$object->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz();
+
 $object
     ->foo()
     ->bar()
-    ->baz();
-
-foo()
+    ->baz()
+    ->foo()
+    ->bar()
+    ->baz()
+    ->foo()
+    ->bar()
+    ->baz()
+    ->foo()
+    ->bar()
+    ->baz()
+    ->foo()
+    ->bar()
+    ->baz()
+    ->foo()
     ->bar()
     ->baz();
 
+foo()->bar()->baz();
+
 foo()->bar->baz();
 
 ================================================================================
@@ -191,10 +216,7 @@ $object[$valid
 
 ($a ? $b : $c)->d()->e();
 
-($a ? $b : $c)
-    ->d()
-    ->e()
-    ->f();
+($a ? $b : $c)->d()->e()->f();
 
 ($valid
     ? $helper->responseBody($this->currentUser)
@@ -436,12 +458,8 @@ $var = Foo::keys($items)->filter(function ($x) { return $x > 2; })->map(function
 <?php
 
 one()->two();
-one()
-    ->two()
-    ->three();
-one()
-    ->two->three()
-    ->four->five();
+one()->two()->three();
+one()->two->three()->four->five();
 
 Route::prefix("api")
     ->middleware("api")
@@ -476,9 +494,7 @@ $a->a()->b();
 $a->a()->b();
 $a->a()->b;
 $a->b->a();
-$a->b()
-    ->c()
-    ->d();
+$a->b()->c()->d();
 $a->b->c->d;
 
 // should inline
@@ -545,67 +561,19 @@ $foo->bar(
     "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLong"
 )()()()()();
 
-$brian->hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-$brian::$hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-$brian["hotel"]
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-Foo::$hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-(new Foo())->hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-(clone $a)->hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-
-$var = $brian->hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-$var = $brian::$hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-$var = $brian["hotel"]
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-$var = Foo::$hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-$var = (new Foo())->hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
-$var = (clone $a)->hotel
-    ->orders()
-    ->ordered()
-    ->with("smith")
-    ->get();
+$brian->hotel->orders()->ordered()->with("smith")->get();
+$brian::$hotel->orders()->ordered()->with("smith")->get();
+$brian["hotel"]->orders()->ordered()->with("smith")->get();
+Foo::$hotel->orders()->ordered()->with("smith")->get();
+(new Foo())->hotel->orders()->ordered()->with("smith")->get();
+(clone $a)->hotel->orders()->ordered()->with("smith")->get();
+
+$var = $brian->hotel->orders()->ordered()->with("smith")->get();
+$var = $brian::$hotel->orders()->ordered()->with("smith")->get();
+$var = $brian["hotel"]->orders()->ordered()->with("smith")->get();
+$var = Foo::$hotel->orders()->ordered()->with("smith")->get();
+$var = (new Foo())->hotel->orders()->ordered()->with("smith")->get();
+$var = (clone $a)->hotel->orders()->ordered()->with("smith")->get();
 
 $var = Foo::keys($items)
     ->filter(function ($x) {
@@ -615,9 +583,11 @@ $var = Foo::keys($items)
         return $x * 2;
     });
 
-(new static(func_get_args()))->push($this)->each(function ($item) {
-    VarDumper::dump($item);
-});
+(new static(func_get_args()))
+    ->push($this)
+    ->each(function ($item) {
+        VarDumper::dump($item);
+    });
 (new static(func_get_args()))
     ->offset(10)
     ->push($this)
diff --git a/tests/member_chain/break-multiple.php b/tests/member_chain/break-multiple.php
index 642568a..8bc7fbb 100644
--- a/tests/member_chain/break-multiple.php
+++ b/tests/member_chain/break-multiple.php
@@ -2,6 +2,12 @@
 
 $object->foo()->bar()->baz();
 
+$object->foo()->bar()->baz()->foo()->bar()->baz();
+
+$object->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz();
+
+$object->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz()->foo()->bar()->baz();
+
 foo()->bar()->baz();
 
 foo()->bar->baz();
diff --git a/tests/parens/__snapshots__/jsfmt.spec.mjs.snap b/tests/parens/__snapshots__/jsfmt.spec.mjs.snap
index 72c1922..373b185 100644
--- a/tests/parens/__snapshots__/jsfmt.spec.mjs.snap
+++ b/tests/parens/__snapshots__/jsfmt.spec.mjs.snap
@@ -1568,10 +1568,7 @@ $var = (clone foo())->bar()->foo();
 $var = (clone foo())->bar()->foo();
 $var = (clone foo())->bar()->foo()[0];
 $var = (clone foo())->bar()->foo()[0][1];
-$var = (clone foo())
-    ->bar()
-    ->foo()
-    ->baz();
+$var = (clone foo())->bar()->foo()->baz();
 $var = (clone $foo())->bar;
 $var = (clone $bar->y)->x;
 $var = (clone $foo)[0];
@@ -2562,10 +2559,7 @@ $var = (new foo())->bar()->foo();
 $var = (new foo())->bar()->foo();
 $var = (new foo())->bar()->foo()[0];
 $var = (new foo())->bar()->foo()[0][1];
-$var = (new foo())
-    ->bar()
-    ->foo()
-    ->baz();
+$var = (new foo())->bar()->foo()->baz();
 $var = (new $foo())->bar;
 $var = (new $bar->y())->x;
 $var = (new foo())[0];
@@ -2935,10 +2929,7 @@ $var = ((int) $var) + 1 === 2 ? "1" : "2";
 ($var ? $var : $var)[1];
 ($var ? $var : $var)->d();
 ($var ? $var : $var)->d()->e();
-($var ? $var : $var)
-    ->d()
-    ->e()
-    ->f();
+($var ? $var : $var)->d()->e()->f();
 ($var
     ? $var->responseBody($var->currentUser)
     : $var->responseBody($var->defaultUser)
diff --git a/tests/print/__snapshots__/jsfmt.spec.mjs.snap b/tests/print/__snapshots__/jsfmt.spec.mjs.snap
index 2aa77bc..d93a2e4 100644
--- a/tests/print/__snapshots__/jsfmt.spec.mjs.snap
+++ b/tests/print/__snapshots__/jsfmt.spec.mjs.snap
@@ -299,10 +299,7 @@ print $var->veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongProperty
     ->veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongProperty
     ->veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongProperty;
 
-print $var
-    ->call()
-    ->call()
-    ->call();
+print $var->call()->call()->call();
 print $var
     ->veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongCall()
     ->veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongCall()
diff --git a/tests/retif/__snapshots__/jsfmt.spec.mjs.snap b/tests/retif/__snapshots__/jsfmt.spec.mjs.snap
index b372be8..94fc73a 100644
--- a/tests/retif/__snapshots__/jsfmt.spec.mjs.snap
+++ b/tests/retif/__snapshots__/jsfmt.spec.mjs.snap
@@ -636,15 +636,11 @@ $callback =
 $var =
     $delay instanceof DateTimeInterface
         ? $delay->getTimestamp()
-        : Carbon::now()
-            ->addSeconds($delay)
-            ->getTimestamp();
+        : Carbon::now()->addSeconds($delay)->getTimestamp();
 
 return $delay instanceof DateTimeInterface
     ? $delay->getTimestamp()
-    : Carbon::now()
-        ->addSeconds($delay)
-        ->getTimestamp();
+    : Carbon::now()->addSeconds($delay)->getTimestamp();
 
 $var = true
     ? 'string
diff --git a/tests/return/__snapshots__/jsfmt.spec.mjs.snap b/tests/return/__snapshots__/jsfmt.spec.mjs.snap
index ac27d06..c1c416e 100644
--- a/tests/return/__snapshots__/jsfmt.spec.mjs.snap
+++ b/tests/return/__snapshots__/jsfmt.spec.mjs.snap
@@ -319,30 +319,10 @@ return (function () {})();
 
 return call();
 return $a->b->c->d->e->f;
-return $a
-    ->b()
-    ->c()
-    ->d()
-    ->e()
-    ->f();
-return call()
-    ->b()
-    ->c()
-    ->d()
-    ->e()
-    ->f();
-return $a
-    ::b()
-    ::c()
-    ::d()
-    ::e()
-    ::f();
-return Foo::a()
-    ::b()
-    ::c()
-    ::d()
-    ::e()
-    ::f();
+return $a->b()->c()->d()->e()->f();
+return call()->b()->c()->d()->e()->f();
+return $a::b()::c()::d()::e()::f();
+return Foo::a()::b()::c()::d()::e()::f();
 
 return "
 string