Psalm fails to pick up #[Attribute] on #[AllowDynamicProperties] when running with forced lower PHP version

[Ocramius]

Context

I and @guidobonuzzi work on a project that is transitioning from PHP 8.1 to PHP 8.2 in the next few weeks.

We keep this project maintained through a large psalm-baseline.xml.

In order to upgrade to PHP 8.2, we had to sprinkle the codebase with #[AllowDynamicProperties] stuff (yes, questionable php-src choices).

One problem arises though: the project is analyzed with PHP 8.1 as PHP version, and Psalm gets confused about how #[AllowDynamicProperties] is declared or not, depending on whether we're running PHP 8.1 or PHP 8.2 for the analysis.

PHP 8.1

Target PHP version: 8.1 (set by config file).

<snip>

ERROR: UndefinedAttributeClass - demo.php:3:3 - Attribute class AllowDynamicProperties does not exist (see https://psalm.dev/241)

This is expected: PHP 8.1 does not have AllowDynamicProperties declared

PHP 8.2

Target PHP version: 8.1 (set by config file).

<snip>

ERROR: InvalidAttribute - demo.php:3:3 - The class AllowDynamicProperties doesn't have the Attribute attribute (see https://psalm.dev/242)

This seems wrong: the AllowDynamicProperties class is found (declared by PHP itself), but it doesn't have #[Attribute] declared on it.

Workarounds

This is fixable by declaring a stub:

<?php

#[Attribute]
final class AllowDynamicProperties {}

    <stubs>
        <file name="AllowDynamicProperties.php"/>
    </stubs>

Worth fixing?

This behavior seems broken-ish in how Psalm loads the AllowDynamicProperties from memory: perhaps it doesn't read its attributes?

Reproducer

Reproducing on the psalm playground is not really feasible: I created a separate repository to run things with docker at https://gist.github.com/Ocramius/88e61479821c1c16819b404d1b04ce0f

[psalm-github-bot]

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

[orklah]

It seems like it has no attributes in the wild: https://3v4l.org/po5FQ

So when Psalm fallback to Reflection, it just sees a random symbol, not an attribute so it complains

It's kinda expected and the way to fix that would be to create retroactive stubs for each attribute introduced from PHP8.0 until the end of times (Yay, symbol conflicts!)

I knew that it behaved like that though I expected PHP to crash when seeing an attribute pointing to an undefined class, but I guess it doesn't ?! So yeah, this is worth fixing for projects who need to support older versions...

I guess we can just create a new stub file for attributes, loaded as soon as we're >8.0 with all the core attributes

[Ocramius]

Is it worth fixing, or can this issue suffice in telling people "please use a stub until fully upgraded"?

[orklah]

I think it's worth it, it's confusing other people: #9728 (I had not seen their answer saying that doesn't crash, I still thought it would)

And you must be one of the 12 people that know about Psalm's stubs so yeah, not counting on that :D

[robchett]

Looks like PHP doesn't care in the slightest about missing backing classes for attributes: https://3v4l.org/Gb7I5 and they are designed to be backwards compatable

I've created #9920 but I'm usure how to write tests for it as the tests are run with PHP8.1 only, how can I reproduce in the test environment:

The class AllowDynamicProperties doesn't have the Attribute attribute

[weirdan]

@robchett we now have CI running on PHP 8.0-8.3.

[robchett]

I might be getting confused on how to reproduce this one, but would this test currently passing on 8.0-8.3 be sufficient?

https://github.com/vimeo/psalm/blob/5.x/tests/AttributeTest.php#L255-L269

[Ocramius]

@robchett seen the example repo?

[robchett]

I had not! Checked it against psalm 5.15 and 8.0, 8.1 and 8.2 all return no errors found.

I've removed the attribute in #10212 and it fails the tests on 8.0 so we should have sufficient regression testing here?

[Ocramius]

I did re-run the commands with vimeo/psalm:5.15.0, and all seems good now:

88e61479821c1c16819b404d1b04ce0f on  main [!] via 🐘 v8.2.10 via ❄️  impure (nix-shell) took 3s 
❯ docker run --rm -ti -v $(pwd):/app -w /app php:8.1 vendor/bin/psalm --no-cache

Install the opcache extension to make use of JIT on PHP 8.0+ for a 20%+ performance boost!

Target PHP version: 8.1 (set by config file).
Scanning files...
Analyzing files...

░
------------------------------
                              
       No errors found!       
                              
------------------------------

Checks took 0.49 seconds and used 45.299MB of memory
Psalm was unable to infer types in the codebase

88e61479821c1c16819b404d1b04ce0f on  main [!] via 🐘 v8.2.10 via ❄️  impure (nix-shell) 
❯ docker run --rm -ti -v $(pwd):/app -w /app php:8.2 vendor/bin/psalm --no-cache

Install the opcache extension to make use of JIT on PHP 8.0+ for a 20%+ performance boost!

Target PHP version: 8.1 (set by config file).
Scanning files...
Analyzing files...

░
------------------------------
                              
       No errors found!       
                              
------------------------------

Checks took 0.51 seconds and used 42.915MB of memory
Psalm was unable to infer types in the codebase

I am not sure what caused the discrepancy, or whether it's worth investigating at all, but IMO we're all good here.

[Ocramius]

Thanks for checking / updating, @robchett!