Skip to content

NoDynamicNameRule: Skip named lookup on known object#264

Merged
TomasVotruba merged 4 commits into
symplify:mainfrom
staabm:dync
May 21, 2026
Merged

NoDynamicNameRule: Skip named lookup on known object#264
TomasVotruba merged 4 commits into
symplify:mainfrom
staabm:dync

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented May 21, 2026

fix false positive

{
// trick to allow multiple node types
return Node::class;
return Expr::class;
Copy link
Copy Markdown
Contributor Author

@staabm staabm May 21, 2026

Choose a reason for hiding this comment

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

use a more specific parent type, so the rule is called less often

public function run()
{
$object = $this->getObject();
if ($object::MY_CONSTANT === 3) {
Copy link
Copy Markdown
Contributor Author

@staabm staabm May 21, 2026

Choose a reason for hiding this comment

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

the main motivation of this PR:

I think its fine to do a named lookup on a variable, as long as its a union of known types

{
/**
* @param array<int, array<string|int>> $expectedErrorMessagesWithLines
* @param list<array{string, int, 2?: string|null}> $expectedErrorMessagesWithLines
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

drive-by phpstan error cleanup

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented May 21, 2026

the failling rector build is unrelated (affects files not touch in this PR)

@TomasVotruba
Copy link
Copy Markdown
Member

Thanks for the feature 👍 Still, passing CI is still a requirement.

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented May 21, 2026

ok cool. I let rector process the unrelated files.

thank you!

@TomasVotruba
Copy link
Copy Markdown
Member

Very nice, the improvement makes sense 👍

@TomasVotruba TomasVotruba merged commit a518b21 into symplify:main May 21, 2026
7 checks passed
@staabm staabm deleted the dync branch May 21, 2026 10:47
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.

2 participants