vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.55k stars 662 forks source link

AMPHP 3.x support #10003

Open emmanuelGuiton opened 1 year ago

emmanuelGuiton commented 1 year ago

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[bot] commented 1 year ago

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

ChristophWurst commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Started switching to amp v3 in https://github.com/vimeo/psalm/pull/10024, but forgot this would raise the minimum PHP requirement for psalm up to php 8.1...

orklah commented 1 year ago

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 commented 1 year ago

The dev-master branch now supports amp v3!

emmanuelGuiton commented 1 year ago

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

roukmoute commented 8 months ago

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 commented 8 months ago

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

roukmoute commented 8 months ago

@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 commented 6 months ago

@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 https://github.com/brick/varexporter/issues/31

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