Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Oct 25, 2022

Type::class,
ReflectionProvider::class,
], true)) {
if (in_array($extendedInterfaceReflection->getName(), BcUncoveredInterface::CLASSES, true)) {
Copy link
Contributor Author

@staabm staabm Oct 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am re-using the same constant as in ApiClassImplementsRule.. thats how I understand the todo.

since we are now ignoring 5 classes here instead of only 2, I could add more tests.
but I wasn't sure whether I should DRY only Type::class, ReflectionProvider::class or all 5 class-strings..?

@@ -14,6 +14,8 @@
* and its methods in their code.
*
* Methods on ExtendedMethodReflection are subject to change.
*
* @api
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

above this interface is this big comment, mentioning that this interface is not meant for re-use. I guess it should be dropped now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll rewrite it after merge.

@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Type\Type;

final class BcUncoveredInterface
Copy link
Member

@ondrejmirtes ondrejmirtes Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A food for thought - in the future we could add a new annotation like @api-interface-forbid-implements. That would allow us not to maintain these lists and also BackwardCompatibilityCheck with each new interface.

@ondrejmirtes ondrejmirtes merged commit 4945098 into phpstan:1.9.x Oct 26, 2022
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the bc-interfaces branch October 26, 2022 08:10
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

Successfully merging this pull request may close these issues.

3 participants