Skip to content

RoleHierarchyInterface doesn't contain any methods. #31814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
polyakov84 opened this issue Jun 3, 2019 · 11 comments
Closed

RoleHierarchyInterface doesn't contain any methods. #31814

polyakov84 opened this issue Jun 3, 2019 · 11 comments
Labels

Comments

@polyakov84
Copy link

Symfony version(s) affected: 4.3

Description
RoleHierarchyInterface doesn't contain any methods.

Comment said "The getReachableRoles(Role[] $roles) method that returns an array of all reachable Role objects is deprecated
since Symfony 4.3."
And we should use method "getReachableRoleNames" or something else.

BUT RoleHierarchyInterface is empty.
I couldn't make a mock from it.

@xabbuh xabbuh added the Security label Jun 3, 2019
@xabbuh
Copy link
Member

xabbuh commented Jun 3, 2019

Adding the method as PHP code will be done in Symfony 5. But you can already find its signature in the docblock:

* @method string[] getReachableRoleNames(string[] $roles) The associated roles - not implementing it is deprecated since Symfony 4.3

@polyakov84
Copy link
Author

So for now old method removed instead of to be marked as deprecated.
And no any new methods available until 5.0

I've got the next error in PHPUnit tests.
"Trying to configure method "getReachableRoles" which cannot be configured because it does not exist, has not been specified, is final, or is static"

And, as I understand, there is no other methods that could be used and exists in this interface.
And this interface is useless until version 5.0

@polyakov84
Copy link
Author

polyakov84 commented Jun 3, 2019

Why this method has been already removed?
Comment in implementation said that it should be removed in version 5.0, but not in 4.3.

@xabbuh
Copy link
Member

xabbuh commented Jun 3, 2019

#30388 (comment) contains some more information

@teohhanhui
Copy link
Contributor

Removing a method from an interface is a BC break. It causes exceptions such as:

Prophecy\Exception\Doubler\MethodNotFoundException: Method Double\RoleHierarchyInterface\P19::getReachableRoles() is not defined.

@nicolas-grekas
Copy link
Member

That's only an issue for mocks, not for real code.
You might need to create a TestRoleHierarchyInterface that extends the native one with the added method.
I did something similar in https://github.com/symfony/symfony/pull/31597/files#diff-13b6c6ea7cdaba73a859c38af4231fd6R70 (for another reason)
That could as well be considered an issue with mocking libraries. It would actually be great if they could handle @method annotations or similar.

No bug here to me.

@lyrixx
Copy link
Member

lyrixx commented Jun 4, 2019

For what it worth, I stop mocking RoleHierarchyInterface and start using RoleHierarchy without mock. Simpler!
https://github.com/symfony/symfony/pull/31824/files#diff-953a0eb8ac77c56ed64c155b3c5789feR45

@teohhanhui
Copy link
Contributor

Mocking is just one of the ways that the problem would manifest itself.

Consider this, it was a real method defined on the interface before, as far as PHP is concerned. In other words, causing the return value of method_exists to be changed is a BC break.

@teohhanhui
Copy link
Contributor

teohhanhui commented Jun 4, 2019

Adding a new method that only exists in PHPDoc @method is fine. Removing an existing actually defined method is not.

@Tobion
Copy link
Contributor

Tobion commented Jun 5, 2019

According to https://symfony.com/doc/current/contributing/code/bc.html#changing-interfaces @teohhanhui is right.

@Tobion
Copy link
Contributor

Tobion commented Apr 10, 2020

I think we can close this as it's solved in 5.0 and people shouldn't need to mock the interface as explained in #31814 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants