Skip to content

[2.8][Security][Guard] GuardTokenInterface vs. TokenInterface in phpDoc #15884

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
wants to merge 1 commit into from

Conversation

derrabus
Copy link
Member

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets none
License MIT
Doc PR none

The new GuardTokenInterface is used as a marker to annotate a security token that is compatible with the new Guard component. I noticed that whenever GuardTokenInterface is annotated, the code expects the object to also implement TokenInterface. This asummption is correct, but we should make it explicit. Especially when looking at GuardAuthenticationProvider::authenticate method, I find it confusing that the doc block is incompatible with the type hint.

With this tiny PR, I changed the annotations to TokenInterface|GuardTokenInterface where I found it helpful to do this. By doing this, I also found an unused import.

An alternative approach could be declaring GuardTokenInterface as an interface that extends TokenInterface.

/ping @weaverryan

@fabpot
Copy link
Member

fabpot commented Sep 25, 2015

I would instead make GuardTokenInterface extend TokenInterface
ping @weaverryan

@stof
Copy link
Member

stof commented Sep 26, 2015

I agree with @fabpot here

@weaverryan
Copy link
Member

I agree as well - see #15927 - if we make GuardTokenInterface extend TokenInterface, it looks like no other changes are needed.

@derrabus
Copy link
Member Author

Yes, #15927 is probably the better solution.

fabpot added a commit that referenced this pull request Sep 27, 2015
This PR was merged into the 2.8 branch.

Discussion
----------

Making GuardTokenInterface extend TokenInterface

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #15884
| License       | MIT
| Doc PR        | n/a

See #15884

Commits
-------

7f04fbb Making GuardTokenInterface extend TokenInterface
@fabpot fabpot closed this Sep 27, 2015
@derrabus derrabus deleted the 2.8-guard-token-phpdoc branch November 30, 2015 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants