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

fix(helper): ensure helpers can't be redeclared #146

Merged
merged 2 commits into from
May 10, 2024

Conversation

NickSdot
Copy link
Contributor

@NickSdot NickSdot commented May 10, 2024

To make packages like symfony/var-dumper available globally you prepend the global Composer autoload.php like so:

// php.ini

auto_prepend_file = ${HOME}/.composer/vendor/autoload.php
image

From: https://symfony.com/doc/current/components/var_dumper.html

To reproduce, at least these must be installed.

// composer

laravel/installer (global)
symfony/var-dumper (global)
laravel/framework (project)
phpstan/phpstan (project)

Then run PhpStan in a project.

// cli

$ vendor/bin/phpstan analyse Common

Result.

Fatal error: Cannot redeclare Laravel\Prompts\text() (previously declared in ~/.composer/vendor/laravel/prompts/src/helpers.php:11) in XXX/vendor/laravel/prompts/src/helpers.php on line 13

Call Stack:
0.0325    1260304   1. {main}() XXX/vendor/bin/phpstan:0
0.0325    1261384   2. include('XXX/vendor/phpstan/phpstan/phpstan') XXX/vendor/bin/phpstan:119
0.0754    4122912   3. require('phar://XXX/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan') XXX/vendor/phpstan/phpstan/phpstan:8
0.0754    4123296   4. _PHPStan_7961f7ae1\{closure:phar://XXX/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan:13-125}() phar://XXX/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan:125
0.1766   36372104   5. require_once('XXX/vendor/autoload.php') phar://XXX/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan:66
0.1766   36378600   6. ComposerAutoloaderInit9c491b8531eec05ba41a11d9276a5749::getLoader() XXX/vendor/autoload.php:25
0.1838   39023408   7. {closure:XXX/vendor/composer/autoload_real.php:37-43}($fileIdentifier = '47e1160838b5e5a10346ac4084b58c23', $file = 'XXX/vendor/composer/../laravel/prompts/src/helpers.php') XXX/vendor/composer/autoload_real.php:45

Notes

  • I did a Composer global update, and installed phpstan in the same time. Didn't dig deeper what causes the bug. But same as with other Laravel helper.php functions, I think they should be protected against redeclaration. This PR adds the required checks.
  • Tests pass. I don't think it will break something. However, give it a deep second thought please. :)
  • Diff here on GH is messed. In PhpStorm it's better.

Amazing work here btw, Jess. Thank you! 🫡❤️

@taylorotwell taylorotwell merged commit 37f94de into laravel:main May 10, 2024
6 checks passed
NickSdot added a commit to NickSdot/valet that referenced this pull request May 12, 2024
Similar issue as in laravel/prompts#146. To make packages like `symfony/var-dumper` available globally you prepend the global Composer autoload.php like so:

```
// php.ini

auto_prepend_file = ${HOME}/.composer/vendor/autoload.php
```

In Valet this can result in

```
( ! ) Warning: Constant VALET_HOME_PATH already defined in ~/.composer/vendor/laravel/valet/server.php on line 12
Call Stack
#	Time	Memory	Function	Location
1	0.0354	571296	{main}( )	.../server.php:0
2	0.0380	571488	define( $constant_name = 'VALET_HOME_PATH', $value = '~/.config/valet' )	.../server.php:12

( ! ) Warning: Constant VALET_STATIC_PREFIX already defined in ~/.composer/vendor/laravel/valet/server.php on line 13
Call Stack
#	Time	Memory	Function	Location
1	0.0354	571296	{main}( )	.../server.php:0
```
@jessarcher
Copy link
Member

I think it's caused by the Laravel installer being installed globally. It depends on Prompts, which effectively makes it installed globally as well. With the auto_prepend_file setting in place, there are effectively two copies of Prompts installed.

Prompts didn't previously have the function_exists guard like other Laravel helpers because they are namespaced functions rather than global, but I don't see any issues adding it.

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.

3 participants