Skip to content

[code-quality] Add AttributeNamedArgsRector#8079

Merged
TomasVotruba merged 2 commits into
rectorphp:mainfrom
DaveLiddament:add-attribute-named-args-rector
Jun 27, 2026
Merged

[code-quality] Add AttributeNamedArgsRector#8079
TomasVotruba merged 2 commits into
rectorphp:mainfrom
DaveLiddament:add-attribute-named-args-rector

Conversation

@DaveLiddament

Copy link
Copy Markdown
Contributor

Convert positional arguments on configured attributes into named arguments, taking the names from the attribute constructor signature. Configurable per attribute class via the AttributeNamedArgs value object, with an optional firstNamedPosition threshold to leave leading arguments positional. Skips already-named arguments and bails out when an argument maps to a variadic or missing parameter to avoid producing invalid PHP.

Convert positional arguments on configured attributes into named
arguments, taking the names from the attribute constructor signature.
Configurable per attribute class via the AttributeNamedArgs value
object, with an optional firstNamedPosition threshold to leave leading
arguments positional. Skips already-named arguments and bails out when
an argument maps to a variadic or missing parameter to avoid producing
invalid PHP.
* @param int $firstNamedPosition first positional index to name (0 = name all arguments)
*/
public function __construct(
private string $attributeClass,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add simple class-name validation here?

Ref \Rector\Validation\RectorAssert::className()

@TomasVotruba

Copy link
Copy Markdown
Member

Very nice rule! 👏

We actually have PHPStan rule enforcing this, but still do it manually lol

@TomasVotruba

Copy link
Copy Markdown
Member

I wasn't sure what is does untill I checked examples.

Could you update name to ExplicitNamedArgsAttributeRector or sth?

}
CODE_SAMPLE
,
[new AttributeNamedArgs('Some\Attribute\SomeAttribute')]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, this rule should handle all or none. It doesn't make sense to explictly list the attributes one by one (there is just ~30 in Symfony, maybe same in Laravel).

…configurable (name all attributes), move to named-args set
@TomasVotruba

Copy link
Copy Markdown
Member

Pushed an update addressing the review:

  • Renamed AttributeNamedArgsRectorExplicitAttributeNamedArgsRector.
  • Made it non-configurable — names positional args on all attributes (no per-attribute list). The AttributeNamedArgs value object is removed, so the class-name validation point is moot.
  • Kept the safety guard: attributes with a variadic / overflow parameter are left untouched to avoid invalid PHP.
  • Added the rule to the named-args set (config/set/named-args.php).

Tests reworked accordingly (7 fixtures, all green).

@TomasVotruba

Copy link
Copy Markdown
Member

I've updated PR and go for merge 👍 Thanks!

@TomasVotruba TomasVotruba merged commit 26a142f into rectorphp:main Jun 27, 2026
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants