Skip to content

Fix variable named function calls #12

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

Merged
merged 1 commit into from
Feb 24, 2022
Merged

Conversation

kkmuffme
Copy link
Contributor

@kkmuffme kkmuffme commented Feb 23, 2022

Fix for:

$my_function = 'foo_bar';
$my_function();

@szepeviktor
Copy link
Member

szepeviktor commented Feb 23, 2022

Thank you!
Please use count() === 0 or === []

@kkmuffme
Copy link
Contributor Author

In addition to the !empty check? or instead of? (not possible, since this will cause a PHP notice)

@szepeviktor
Copy link
Member

Okay. Then please add a test that fails without this PR.

@kkmuffme
Copy link
Contributor Author

Sorry, no idea how to do that, I'm not a developer.
The example I provided above will fail without this PR.

@szepeviktor
Copy link
Member

TBH This codebase has many-many problems.
I carry this baby only for WP stubs. Please understand that.

@szepeviktor
Copy link
Member

Indeed parts property does not exist.

property_exists($node->expr->name, 'parts') &&

@szepeviktor
Copy link
Member

@szepeviktor
Copy link
Member

Please use $node->expr->name instanceof Name &&
That does the actual check.

@szepeviktor
Copy link
Member

Thank you for opening this PR!

@kkmuffme
Copy link
Contributor Author

Even when doing $node->expr->name instanceof Name && there is no guarantee $parts exists.
And that is the problem. In case of dynamic function this $parts part is null.

The way I did is the fast way to ensure we don't get any notice from there and it will not fail silently.
If you want, I can do the $node->expr->name instanceof Name && property_exists($node->expr->name, 'parts') check, but this is just unnecessarily slower than an !empty check.
Just let me know your preference.

@szepeviktor
Copy link
Member

this is just unnecessarily slower than an !empty check

Please take a look at empty's source code in the C code of PHP.

Let's go with Name and we'll see what the future holds.
https://github.com/nikic/PHP-Parser/blob/a6e34665fd6a2bcefc924cb5fd73788767ae510e/lib/PhpParser/Node/Name.php#L26

Fix for:
$my_function = 'foo_bar';
$my_function();
@kkmuffme
Copy link
Contributor Author

Changed it now, so it's ready to merge.

@szepeviktor szepeviktor changed the title fix notices and generator failing on dynamic functions Fix variable named function calls Feb 24, 2022
@szepeviktor szepeviktor merged commit b4bad6b into php-stubs:master Feb 24, 2022
@kkmuffme kkmuffme deleted the patch-1 branch February 9, 2023 14:59
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