[Security] Adding __toString() method to Role class #10179
Conversation
| */ | ||
| public function __toString() | ||
| { | ||
| return $this->role; |
There was a problem hiding this comment.
This will produce a fatal error if $this->role is null. It is allowed by the RoleInterface.
There was a problem hiding this comment.
good spot, will fix and add a test
There was a problem hiding this comment.
Added tests to ensure that the __toString() behaves as expected, although it was never possible for it to be null because the role was immutable and cast to a string in the constructor
|
@fabpot is this okay to be merged? |
|
It cannot be merged into 2.3. Also the toString doesn't help much when it's not part of the interface. So it would need to be added there as well which makes it a BC break. Could be worth, not sure. |
|
@Tobion I'm not sure the interface would actually require the It certainly eases things when you're trying to compare an array of |
|
Closing in favor of what I explained in #8313 (comment) |
This PR proposes adding the
toString()method to the\Symfony\Component\Security\Core\Role\Roleclass. This should ease the comparison of role strings and instances ofRoleitself. For example, in theFOSUserBundletheUsermodel contains roles which are represented as strings. This makes comparisons between arrays ofRoleobjects and role strings difficult without creating your ownRoleclass with a__toString(), and this isn't always desirable.I found some conversation around adding the
\Seriazableinterface to theRoleclass, and I realise that this might also be considered a BC break if people are relying on the fact that it will not cast to a string.