amphp / amp

A non-blocking concurrency framework for PHP applications. 🐘
https://amphp.org/amp
MIT License
4.24k stars 257 forks source link

Using AMPHP with Psalm #439

Closed mcharytoniuk closed 4 months ago

mcharytoniuk commented 4 months ago

Hello!

I have a project that requires "amphp/amp": "^3.0" and I intended to lint it with Psalm.

I keep Psalm in a separate tools/psalm directory inside the project. To avoid versioning conflict.

So, the situation is, Psalm uses amphp/php and autoloads it and also my project uses amphp/php which results in PHP Fatal error: Cannot redeclare Amp\delay() (previously declared in ... error. That is because Psalm autoloads project's PHP files, so in general amp/src/functions.php file is loaded twice (from different AMP versions on top of it).

There are no checks in amp/src/functions.php to determine if they are already loaded or not, so it looks like it is not possible to use AMP project with Psalm (or any kind of tooling that uses AMPHP).

kelunik commented 4 months ago

You can definitely use Psalm with Amp v3. We do that in all or almost all of our packages. We usually recommend using the PHAR version of psalm for that.

mcharytoniuk commented 4 months ago

With psalm.phar it works without issues. Thanks for suggesting the solution!

I still think that be improved somehow. Maybe checking if a function exists and comparing already loaded AMP versions?

It is easy for me to just include Psalm in the repo with the project as just composer.json, with psalm.phar, I would have to attach ~40MB file instead:

{
    "require": {
        "vimeo/psalm": "^5.15"
    }
}

Do you download psalm.phar before running builds (for example in the CI environments)?

kelunik commented 4 months ago

I still think that be improved somehow. Maybe checking if a function exists and comparing already loaded AMP versions?

No, v2 and v3 of Amp are incompatible. You can't mix them.

We keep psalm/phar in require-dev. The size of the PHAR has never been an issue.

mcharytoniuk commented 4 months ago

My another issue with Phar is - I am also using other PHP extensions like ds for example, so I need something like that in my psalm.xml file, so

<stubs>
    <file name="tools/psalm/vendor/vimeo/psalm/stubs/extensions/ds.phpstub" />
</stubs>

That means what I will have to end up doing is keep a composer.json like this in the project, so it can have access to both stubs and phar:

{
    "require": {
        "psalm/phar": "^5.24",
        "vimeo/psalm": "^5.24"
    }
}

Just leaving that here in case someone has a similar issue. :D

Thanks for the help!

danog commented 4 months ago

You can also use dev-master of Psalm, which is compatible with amp v3