Skip to content
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

[CodeQuality] Add rule to move attributes under comment #7225

Closed
TomasVotruba opened this issue Jun 10, 2022 · 4 comments
Closed

[CodeQuality] Add rule to move attributes under comment #7225

TomasVotruba opened this issue Jun 10, 2022 · 4 comments
Assignees
Labels

Comments

@TomasVotruba
Copy link
Member

The attributes are part of code, while docblock is comment above them. The other order will crash.

This was addressed in PHP FIG Per coding style:
php-fig/per-coding-style#8

And php-parser: nikic/PHP-Parser#762


Looking at code sample, maybe we could deal with this mess with Rector rule :)

-#[Attribute]
 /**
  * @property-read $name 
  */
+#[Attribute]
 class SomeClass
 {
 }
@TomasVotruba TomasVotruba changed the title [CodeQuality] Move attributes under comment [CodeQuality] Add rule to move attributes under comment Jun 10, 2022
@samsonasik
Copy link
Member

Stmts/Exprs that have attrGroups:

\PhpParser\Internal\PrintableNewAnonClassNode;
\PhpParser\Node\Param;
\PhpParser\Node\Expr\ArrowFunction;
\PhpParser\Node\Expr\Closure;
\PhpParser\Node\Expr\ClassConst;
\PhpParser\Node\Expr\ClassLike;
\PhpParser\Node\Expr\ClassMethod;
\PhpParser\Node\Expr\EnumCase;
\PhpParser\Node\Expr\Function_;
\PhpParser\Node\Expr\Property;

it seems PrintableNewAnonClassNode is for internal PhpParser.

@Crell
Copy link

Crell commented Jun 13, 2022

Point of order: FIG has not addressed it yet. The question has been brought up to the working group. I agree putting the attribute after the comment is probably a good idea, but as of yet FIG has not made a statement one way or another.

@TomasVotruba
Copy link
Member Author

Sure, it is motivation to move forward. We've seen this bug since first native attributes release.

@TomasVotruba
Copy link
Member Author

TomasVotruba commented Jul 20, 2022

After some reasearch and experiment, it's clear the php-parser resolves attributes correctly, even if separated by comment:
rectorphp/rector-src#2474 (comment)

So there is no need for such rule.

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

Successfully merging a pull request may close this issue.

3 participants