Remove internal from Metadata

[VincentLanglet]

Q	A
Branch?	main
Tickets	Closes #...
License	MIT
Doc PR	api-platform/docs#...

Hi @soyuka,

When using Operation::getExtraProperties, phpstan report

 Call to method getExtraProperties() of internal class          
         ApiPlatform\Metadata\Metadata from outside its root namespace  
         ApiPlatform

This is because those getter are defined in Metadata which is marked as internal.
You can see a reproducer on phpstan.org with https://phpstan.org/r/ec5a2ab4-eac7-4814-836d-0edf97ccee25

Since all Metadata setter/getter shouldn't be considered as internal ; I feel like the simplest solution would be to remove the @internal tag from the abstract class.

Would be nice if it could be added before the 4.2 release. 🙏

[dunglas]

It feels like a bug in PHPStan to me.
Only the abstract class in internal, not the public methods inherited by the concrete non-abstract class.

[dunglas]

Actually, this is the same issue as phpstan/phpstan#13021 (cc @ondrejmirtes).

On our case, we don't want library consumers to use extend the abstract class. They must use the concrete class we provide, and which inherit the abstract internal class. I'm open to change the PHPDoc if there is a way to make PHPStan happy, but not to make the abstract class part of the public API.

[VincentLanglet]

It feels like a bug in PHPStan to me.

Notice it's not just a PHPStan bug since Psalm has the same behavior
https://psalm.dev/r/e1f58f8a24

On our case, we don't want library consumers to use extend the abstract class. They must use the concrete class we provide, and which inherit the abstract internal class. I'm open to change the PHPDoc if there is a way to make PHPStan happy, but not to make the abstract class part of the public API.

I feel like it's a difference of definition between your @internal and phpstan @internal.
In your opinion, it only apply to extends.
In PHPStan opinion, @internal on a class apply to everything because it basically say this class could be freely modified or deleted.

I think issue could be solved if the @psalm-inheritor (and implement this on phpstan side)
https://psalm.dev/docs/annotating_code/supported_annotations/#psalm-inheritors

You would write @psalm-inheritor ApiResource|Operation. Should I update the PR @dunglas ?

Or you would prefer a @not-internal tag to add to every method of the class (?)

On our case, we don't want library consumers to use extend the abstract class.

I'm not fully sure of the benefit on your side, since you have to provide BC for all the methods.
@internal is more about BC policy.

[dunglas]

The rationale is (was? cc @soyuka) that this abstract class is purely for code reuse and shouldn't leak in our public API as we may remove it at some point.

I agree that we use this tag with a different meaning than PHPstan and Psalm. I'm open to any solution TBH but I would prefer to keep this abstract class entirely internal (we may use an internal trait instead).

Switching to the Psalm-specific annotations for now sounds good to me.

[VincentLanglet]

Switching to the Psalm-specific annotations for now sounds good to me.

Thanks ; I updated the PR and I'll try to implements
phpstan/phpstan#13128 on PHPStan side.
(already implemented the parser phpstan/phpdoc-parser#273)