Skip to content

fix: attribute PSR compatibility #2424

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.

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
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

apollisa
Copy link

@apollisa apollisa commented May 2, 2025

This partially fixes #2360, in the sense that formatting now matches PSR guidelines. It will still split across multiple lines if need be though.

@apollisa
Copy link
Author

apollisa commented May 2, 2025

I’m confused I ran the exact same command on my laptop and everything passes; did I miss something?

@fisker
Copy link
Member

fisker commented Jun 7, 2025

Hello, @apollisa The test fails because the AST changed, run AST_COMPARE=1 yarn test you'll see the failure on local.

@apollisa
Copy link
Author

apollisa commented Jun 8, 2025

Oh indeed with AST_COMPARE the tests do fail in local 👍

Should Prettier keep the AST unchanged? Because here if we want to stick with the PSR rules, we must split attributes that span multiple lines:

#[
    IA("interface"),
    \\Looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong\\Namespace\\WithStuff\\IB
]

becomes

#[IA("interface")]
#[\\Looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong\\Namespace\\WithStuff\\IB]

@fisker
Copy link
Member

fisker commented Jun 8, 2025

I'm not sure if this will be accepted, but if it's expected to produce different AST, you'll need change the old shape to new shape in https://github.com/prettier/plugin-php/blob/main/src/clean.mjs

@apollisa
Copy link
Author

apollisa commented Jun 8, 2025

Well I’m sorry I can’t seem to make it to work 🤔 It would seem that the clean function only gets a single attribute group at a time, so I don’t see how I could split it into multiple ones

@fisker
Copy link
Member

fisker commented Jun 8, 2025

Looking at how printAttrs is called, I think you can

if (node.kind === 'arrowfunc' || node.kind === 'parameter' /* || other kinds */) {
  // Update newObj.attrGroups
}

clean should also have a third argument to access parent, I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Symfony #[Route] Formatting Bug
2 participants