Skip to content

Update php-parser, fix unary snapshot tests, and make multiple fixes for attribute formatting. #2502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
acharron-hl wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update php-parser, fix unary snapshot tests, and make multiple fixes for attribute formatting. #2502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
071a365
Update php-parser, fix unary snapshot tests, and make multiple fixes …
acharron-hl Jun 2, 2026
058a2b6
Update to php-parser 3.7.0
acharron-hl Jun 12, 2026
b5b2af3
Remove package manager declaration
acharron-hl Jun 12, 2026
6784397
Address PR feedback
acharron-hl Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
],
"dependencies": {
"linguist-languages": "^8.0.0",
"php-parser": "^3.4.0"
"php-parser": "^3.7.0"
},
"devDependencies": {
"@babel/preset-env": "^7.27.2",
Expand Down
46 changes: 46 additions & 0 deletions src/comments.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ function handleOwnLineComment(comment, text, options) {
options
) ||
handleTryComments(enclosingNode, followingNode, comment) ||
handleClassMemberStatementComments(enclosingNode, followingNode, comment) ||
handleClassComments(enclosingNode, followingNode, comment) ||
handleFunctionParameter(
text,
Expand Down Expand Up @@ -201,6 +202,7 @@ function handleRemainingComment(comment, text, options) {
handleGoto(enclosingNode, comment) ||
handleHalt(precedingNode, enclosingNode, followingNode, comment) ||
handleBreakAndContinueStatementComments(enclosingNode, comment) ||
handleClassMemberStatementComments(enclosingNode, followingNode, comment) ||
handleInlineComments(
enclosingNode,
precedingNode,
Expand Down Expand Up @@ -433,6 +435,32 @@ function handleTraitUseComments(enclosingNode, followingNode, comment) {
return false;
}

function handleClassMemberStatementComments(
enclosingNode,
followingNode,
comment
) {
if (
enclosingNode &&
enclosingNode.kind === "propertystatement" &&
enclosingNode.properties?.includes(followingNode)
) {
addLeadingComment(enclosingNode, comment);
return true;
}

if (
enclosingNode &&
enclosingNode.kind === "classconstant" &&
enclosingNode.constants?.includes(followingNode)
) {
addLeadingComment(enclosingNode, comment);
return true;
}

return false;
}

function handleClassComments(enclosingNode, followingNode, comment) {
if (
enclosingNode &&
Expand Down Expand Up @@ -907,6 +935,24 @@ function getCommentChildNodes(node) {
node.what.__parent_new_arguments = [...node.arguments];
return [node.what];
}

if (node.attrGroups && node.attrGroups.length > 0) {
if (node.kind === "method" || node.kind === "function") {
return [
...node.attrGroups,
...node.arguments,
...(node.type ? [node.type] : []),
];
}

if (node.kind === "classconstant") {
return [...node.attrGroups, ...node.constants];
}

if (node.kind === "enumcase") {
return node.attrGroups;
}
}
}

function canAttachComment(node) {
Expand Down
51 changes: 30 additions & 21 deletions src/printer.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1199,31 +1199,16 @@ function printAttrs(path, options, print, { inline = false } = {}) {
if (!path.node.attrGroups) {
return [];
}
path.each(() => {
const attrGroup = ["#["];
path.each((attrGroupPath) => {
if (!inline && allAttrs.length > 0) {
allAttrs.push(hardline);
}
attrGroup.push(softline);
path.each(() => {
const attrNode = path.node;
if (attrGroup.length > 2) {
attrGroup.push(",", line);
}
const attrStmt = [attrNode.name];
if (attrNode.args.length > 0) {
attrStmt.push(printArgumentsList(path, options, print, "args"));
}
attrGroup.push(group(attrStmt));
}, "attrs");
allAttrs.push(
group([
indent(attrGroup),
ifBreak(shouldPrintComma(options, 8.0) ? "," : ""),
softline,
"]",
inline ? ifBreak(softline, " ") : "",
])
printAllComments(
attrGroupPath,
() => printAttrGroup(attrGroupPath, options, print, { inline }),
options
)
);
}, "attrGroups");
if (allAttrs.length === 0) {
Expand All @@ -1232,6 +1217,29 @@ function printAttrs(path, options, print, { inline = false } = {}) {
return [...allAttrs, inline ? "" : hardline];
}

function printAttrGroup(path, options, print, { inline = false } = {}) {
const attrGroup = ["#["];
attrGroup.push(softline);
path.each(() => {
const attrNode = path.node;
if (attrGroup.length > 2) {
attrGroup.push(",", line);
}
const attrStmt = [attrNode.name];
if (attrNode.args.length > 0) {
attrStmt.push(printArgumentsList(path, options, print, "args"));
}
attrGroup.push(group(attrStmt));
}, "attrs");
return group([
indent(attrGroup),
ifBreak(shouldPrintComma(options, 8.0) ? "," : ""),
softline,
"]",
inline ? ifBreak(softline, " ") : "",
]);
}

function printClass(path, options, print) {
const { node } = path;
const isAnonymousClass = node.kind === "class" && node.isAnonymous;
Expand Down Expand Up @@ -2909,6 +2917,7 @@ function printNode(path, options, print) {

case "enumcase":
return group([
...printAttrs(path, options, print),
"case ",
print("name"),
node.value
Expand Down
6 changes: 3 additions & 3 deletions tests/attributes/__snapshots__/jsfmt.spec.mjs.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,12 @@ class D
function k(#[L] int $m): callable
{
return #[N, O] #[P] fn(#[Q] int $r) => $r * 2;
} //Testing T

@jorgsowa jorgsowa Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see that something is messed up with comments. Why did this comment change its position?

Do you know whether it's a fault of the PHP parser or your PR?

@acharron-hl acharron-hl Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from the changes I made to comment/attribute formatting (Item 1 in my snapshot changes listed in the PR description).

So this is the raw unformatted code

    }

    // Testing S
    #[S]
//Testing S-T
#[T] //Testing T

This is the old snapshot

    } //Testing T          ← comment moved here (wrong)
    // Testing S
    //Testing S-T          ← moved above #[S] (wrong)
    #[S]
    #[T]

And my udpated snapshot

    }                       ← no stray comment
    // Testing S
    #[S]
    //Testing S-T          ← stays between #[S] and #[T]
    #[T] //Testing T        ← comment stays with #[T]
    private function u()

It's kind of hard to tell in the git diff that's for sure. One of the negatives of such a huge snapshot is that the input/output are hundreds of lines apart when looking at the snapshot.

}

// Testing S
//Testing S-T
#[S]
#[T]
//Testing S-T
#[T] //Testing T
private function u()
{
return #[V] function () {
Expand Down
4 changes: 2 additions & 2 deletions tests/comments/__snapshots__/jsfmt.spec.mjs.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4922,9 +4922,9 @@ class Foo

public function emptyMethod(/* comments */) {}

abstract public function sortByName(/* bool $useNaturalSort = false */); /* comment */
abstract public function sortByName(/* bool $useNaturalSort = false */);

/* comment */ /* comment */ protected static $foo; /* comment */
/* comment */ /* comment */ /* comment */ protected static $foo; /* comment */

@jorgsowa jorgsowa Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/* comment */ /* comment */ /* comment */ protected static $foo; /* comment */
/* comment */ protected /* comment */ static /* comment */ $foo /* comment */;

Everything looks correct, except of this part. Some comments moved into wrong places.

@acharron-hl acharron-hl Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel quite right either?

So the unformatted was this:

We have 4 comment blocks attached to the property in total. One before the declaration, 2 inside of it, and 2 after.

    abstract public function sortByName(/* bool $useNaturalSort = false */);

    /* comment */ protected /* comment */ static /* comment */ $foo /* comment */;

    public function foo( // Comment
    ) {}

Previously it formatted to this, which emits the leading comment in the spot and emits the inner comments before the declaration.

    abstract public function sortByName(/* bool $useNaturalSort = false */); /* comment */  ← stolen

    /* comment */ /* comment */ protected static $foo; /* comment */                      ← 1 comment missing

It just fixed the leading comment getting emitted in the wrong spot, but did not modify the behaviour of comments getting placed in between property keywords.

    abstract public function sortByName(/* bool $useNaturalSort = false */);

    /* comment */ /* comment */ /* comment */ protected static $foo; /* comment */        ← all 4 preserved

I guess what it really comes down to is:

Do we want to allow comments on property keywords or not. In my mind, that doesn't make much sense at all (If I saw someone putting PHPDoc between the modifier and the property I'd be asking wtf they are doing), so I preserve them but emit them as leading comments on the property declaration. In any case it's an improvement over it getting attached to the previous node.

@acharron-hl acharron-hl Jun 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll note as well supporting preservation of property keywords gets a bit gnarlier when you consider we'd want to do it for constants as well, and there are a lot of other modifiers as well (types, readonly, attributes, multi property declarations etc).


public function foo() {} // Comment

Expand Down
48 changes: 47 additions & 1 deletion tests/enum/__snapshots__/jsfmt.spec.mjs.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing

exports[`enum.php 1`] = `
====================================options=====================================
Expand Down Expand Up @@ -143,3 +143,49 @@ class Enum extends Foo {}

================================================================================
`;

exports[`enum-case-attributes.php 1`] = `
====================================options=====================================
parsers: ["php"]
printWidth: 80
| printWidth
=====================================input======================================
<?php

namespace App\\Enums;

use App\\Support\\Attributes\\Description;

enum SalutationEnum: string
{
#[Description('Mr.')]
case MR = 'mr';

#[Description('Mrs.')]
case MRS = 'mrs';

#[Description('Miss')]
case MISS = 'miss';
}

=====================================output=====================================
<?php

namespace App\\Enums;

use App\\Support\\Attributes\\Description;

enum SalutationEnum: string
{
#[Description("Mr.")]
case MR = "mr";

#[Description("Mrs.")]
case MRS = "mrs";

#[Description("Miss")]
case MISS = "miss";
}

================================================================================
`;
17 changes: 17 additions & 0 deletions tests/enum/enum-case-attributes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

namespace App\Enums;

use App\Support\Attributes\Description;

enum SalutationEnum: string
{
#[Description('Mr.')]
case MR = 'mr';

#[Description('Mrs.')]
case MRS = 'mrs';

#[Description('Miss')]
case MISS = 'miss';
}
8 changes: 4 additions & 4 deletions tests/parens/__snapshots__/jsfmt.spec.mjs.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1300,7 +1300,7 @@ $var = $var + $var ** 2;
$var = ($var + $var) ** 2;
$var = $var + $var ** 2;
$var = (+$var) ** 2;
$var = (+$var) ** 2;
$var = +($var ** 2);

$var = $foo instanceof Foo;
$var = $foo instanceof Foo || $foo instanceof Foo;
Expand Down Expand Up @@ -2067,7 +2067,7 @@ $var = $var + $var ** 2;
$var = ($var + $var) ** 2;
$var = $var + $var ** 2;
$var = (+$var) ** 2;
$var = (+$var) ** 2;
$var = +($var ** 2);

$var = $foo instanceof Foo;
$var = $foo instanceof Foo || $foo instanceof Foo;
Expand Down Expand Up @@ -6574,7 +6574,7 @@ $var = ~~$var;
$var = !$var;
$var = !!$var;

$a = (+$a) ** 1;
$a = +($a ** 1);
$a = (+$a) ** 1;
$a = 1 ** +$a;

Expand Down Expand Up @@ -6718,7 +6718,7 @@ $var = ~~$var;
$var = !$var;
$var = !!$var;

$a = (+$a) ** 1;
$a = +($a ** 1);
$a = (+$a) ** 1;
$a = 1 ** +$a;

Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5737,10 +5737,10 @@ path-type@^4.0.0:
resolved "https://registry.yarnpkg.com/path-type/-/path-type-4.0.0.tgz#84ed01c0a7ba380afe09d90a8c180dcd9d03043b"
integrity sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==

php-parser@^3.4.0:
version "3.4.0"
resolved "https://registry.yarnpkg.com/php-parser/-/php-parser-3.4.0.tgz#dcd7743bf01caaaa7281e8b253f4cdbbea6e1265"
integrity sha512-JoDPazv8OESrVtcoIuO8HC587zVNJYSYUIl9zYn+JEpwlHSOrxmyMHB+sDS0O1ID5z1aFxPnr+vAUGoSKphlHA==
php-parser@^3.7.0:
version "3.7.0"
resolved "https://registry.yarnpkg.com/php-parser/-/php-parser-3.7.0.tgz#b40b82dc9ba2ad3068b418130f50a84b663d29ac"
integrity sha512-JRc1t78GZAEa+MuzVC5A5RJS1NDFTS4UnprUEu/NnsN9cyHbGZLUqghO9IQZUSCay62HYQiWd3PxyWAEF45zmA==

picocolors@^1.1.1:
version "1.1.1"
Expand Down
Loading