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

AMPHP 3.x support #10003

Open
emmanuelGuiton opened this issue Jul 6, 2023 · 14 comments
Open

AMPHP 3.x support #10003

emmanuelGuiton opened this issue Jul 6, 2023 · 14 comments

Comments

@emmanuelGuiton
Copy link
Contributor

As stated by @weirdan in #7307

Once amp ships a stable release, we would welcome a PR to support 3.0.

Version 3.0 is out since December 2022 so I open a request to address the issue.

The migration guide is available at https://amphp.org/upgrade
The library is only used in Psalm's language server (Psalm\Internal\LanguageServer).

Without migration, developers that use AMPHP 3.x in their project suffer from a conflict with Psalm's dependency on AMPHP 2.x
Degraded functionality can still be available using psalm.phar - one loses other functionalities like Psalm plugin support (such as psalm/phpunit).

I do have the issue on a project but I am not familiar enough with Psalm's internal engine nor with AMPHP to fix it myself.

@psalm-github-bot
Copy link

Hey @emmanuelGuiton, can you reproduce the issue on https://psalm.dev ?

@ChristophWurst
Copy link

Without migration, developers that use AMPHP 3.x in their project suffer from a conflict with Psalm's dependency on AMPHP 2.x

You can try https://packagist.org/packages/bamarni/composer-bin-plugin and put Psalm into a scoped bin.

@danog
Copy link
Collaborator

danog commented Jul 6, 2023

I'll give a go at upgrading to v3 this weekend, since all amp libraries used by psalm are now stable.
In the meantime, yes, using composer-bin-plugin is always a great idea to prevent you dev deps from messing with the main deps :)

@emmanuelGuiton
Copy link
Contributor Author

emmanuelGuiton commented Jul 10, 2023

You can try https://packagist.org/packages/bamarni/composer-bin-plugin and put Psalm into a scoped bin.

@ChristophWurst I don't see how it could work since the project dependencies are autoloaded by Psalm... Any tip ?
Here is the result of my attempt :

myuser@myuser-laptop:~/tests$ ./target/vendor-bin/psalm/vendor/vimeo/psalm/psalm --no-cache
PHP Fatal error:  Cannot redeclare Amp\delay() (previously declared in /home/myuser/tests/target/vendor/amphp/amp/src/functions.php:64) in /home/myuser/tests/target/vendor-bin/psalm/vendor/amphp/amp/lib/functions.php on line 131
Fatal error: Cannot redeclare Amp\delay() (previously declared in /home/myuser/tests/target/vendor/amphp/amp/src/functions.php:64) in /home/myuser/tests/target/vendor-bin/psalm/vendor/amphp/amp/lib/functions.php on line 131

@xpader
Copy link

xpader commented Jul 11, 2023

Psalm should not analysis own vendor directory.
If I have a project my_amp_project, but use another directory project psalm_composer_bin_project to analysis my_amp_project.

./my_amp_project
    psalm.xml
    ./vendor/
    ./vendor/ (No psalm here)

./psalm_composer_bin_project/
    ./vendor/
    ./vendor/bin/psalm
    ./vendor/bin/psalm-language-server

Psalm will both load my_amp_project and psalm_composer_bin_project's vendor directory, and Cannot redeclare Amp\... happened, that's really weird, I think that because psalm load current project vendor, not just parse them.

@xpader
Copy link

xpader commented Jul 11, 2023

I've noticed when language server launched, some log output:

Registering autoloaded files
   /home/pader/php/psalm/vendor/autoload.php
   /home/pader/php/psalm/vendor/composer/autoload_real.php
   /home/pader/php/psalm/vendor/composer/ClassLoader.php
   /home/pader/php/psalm/vendor/composer/autoload_static.php
   /home/pader/php/psalm/vendor/symfony/polyfill-mbstring/bootstrap.php
   /home/pader/php/psalm/vendor/symfony/polyfill-mbstring/bootstrap80.php
   /home/pader/php/psalm/vendor/symfony/polyfill-ctype/bootstrap.php
   /home/pader/php/psalm/vendor/symfony/polyfill-ctype/bootstrap80.php
   /home/pader/php/psalm/vendor/amphp/amp/lib/functions.php
   /home/pader/php/psalm/vendor/amphp/amp/lib/Internal/functions.php
   /home/pader/php/psalm/vendor/symfony/polyfill-intl-grapheme/bootstrap.php
   /home/pader/php/psalm/vendor/symfony/polyfill-intl-grapheme/bootstrap80.php
   /home/pader/php/psalm/vendor/symfony/polyfill-intl-normalizer/bootstrap.php
   /home/pader/php/psalm/vendor/symfony/polyfill-intl-normalizer/bootstrap80.php
   /home/pader/php/psalm/vendor/symfony/deprecation-contracts/function.php
   /home/pader/php/psalm/vendor/symfony/string/Resources/functions.php
   /home/pader/php/psalm/vendor/amphp/byte-stream/lib/functions.php
   /home/pader/php/psalm/vendor/vimeo/psalm/src/Psalm/Internal/VersionUtils.php
   /home/pader/php/psalm/vendor/composer/InstalledVersions.php
   /home/pader/php/psalm/vendor/composer/installed.php

This is a single psalm composer vendor, not current project, may be we can have a config to disable psalm to auto registering composer autoload.

@danog
Copy link
Collaborator

danog commented Jul 19, 2023

Started switching to amp v3 in #10024, but forgot this would raise the minimum PHP requirement for psalm up to php 8.1...

@orklah
Copy link
Collaborator

orklah commented Jul 19, 2023

According to packagist, we have roughly 5% of our users on 7.4 and 5% on 8.0 for Psalm 5.

I think it's fine to prepare Psalm 6 to support 8.1+

@danog
Copy link
Collaborator

danog commented Jul 29, 2023

The dev-master branch now supports amp v3!

@emmanuelGuiton
Copy link
Contributor Author

@danog Should we close this issue ?
Ca we reference any commit / any release from this issue ?

@roukmoute
Copy link

Hello,

I wanted to get a little clarification regarding the current version of amphp/amp. I consulted the composer.json file and noticed that it still mentions version 2. Would it be possible to know if an update to version 3 is planned?

This information would be a great help to me in using it with grumphp. Thank you in advance for your attention and invaluable help.

@danog
Copy link
Collaborator

danog commented Jan 16, 2024

@roukmoute / @emmanuelGuiton / everyone: to get amphp v3 support, simply require psalm v6 via dev-master!

@roukmoute
Copy link

@danog Ohhh! I realized I was on the 5.x branch rather than the master branch. Your help is greatly appreciated, thank you very much!

@kynx
Copy link

kynx commented Apr 4, 2024

@danog Any plans for the Psalm 6 release?

I've been using dev-master, but am now hitting conflicts with nikic/php-parser:^5.0 (which dev-master requires) and brick/var-exporter which requires nikic/php-parser:^4.0 because of, err..., its psalm dependency. See brick/varexporter#31

FWIW I've seen plenty of packages bump PHP versions in minor releases. Though I understand if that's not policy here.

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

No branches or pull requests

7 participants