-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
PSR2/ClassDeclaration: support namespace relative names / prevent fixer conflict #424
PSR2/ClassDeclaration: support namespace relative names / prevent fixer conflict #424
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this locally and can confirm that the code changes cover the added tests appropriately, and that the old code reported "FAILED TO FIX" for these test cases.
I have a query about one change, which we can address in the thread below.
ce1322f
to
a4f2ec8
Compare
a4f2ec8
to
18672c5
Compare
While the `PSR2.Classes.ClassDeclaration` sniff did take partially/fully qualified names into account for interfaces being extended, it did not take _namespace relative_ interface name into account. This led to a fixer conflict within the sniff, where the sniff would first add a space between the `namespace` keyword and the namespace separator (`SpaceBeforeName` fixer) and in a subsequent loop would remove that same space again as it would think it was a space before a comma (`SpaceBeforeComma` fixer). Fixed now by adding support for namespace relative interface names in the `extends` checks. Includes unit test. Related to 152 Note: at a glance, this sniff could probably do with a more thorough review and more defensive token walking/more precise checking, but that's outside the scope of this PR.
While the `PSR2.Classes.ClassDeclaration` sniff did take partially/fully qualified names into account for interfaces being implemented, it did not take _namespace relative_ interface names into account. This led to a fixer conflict within the sniff, where the sniff would first add a newline between the `namespace` keyword and the namespace separator (`InterfaceSameLine` fixer) and in a subsequent loop would remove that same new line again as it would think it was a space before a comma (`SpaceBeforeComma` fixer). Fixed now by adding support for namespace relative interface names in the `implements` check. Includes unit test. Related to 152 Note: at a glance, this sniff could probably do with a more thorough review and more defensive token walking/more precise checking, but that's outside the scope of this PR.
18672c5
to
1a3486e
Compare
Rebased without changes, merging once the build has passed. |
Description
PSR2/ClassDeclaration: prevent fixer conflict with itself [1]
While the
PSR2.Classes.ClassDeclaration
sniff did take partially/fully qualified names into account for interfaces being extended, it did not take namespace relative interface name into account.This led to a fixer conflict within the sniff, where the sniff would first add a space between the
namespace
keyword and the namespace separator (SpaceBeforeName
fixer) and in a subsequent loop would remove that same space again as it would think it was a space before a comma (SpaceBeforeComma
fixer).Fixed now by adding support for namespace relative interface names in the
extends
checks.Includes unit test.
PSR2/ClassDeclaration: prevent fixer conflict with itself [2]
While the
PSR2.Classes.ClassDeclaration
sniff did take partially/fully qualified names into account for interfaces being implemented, it did not take namespace relative interface names into account.This led to a fixer conflict within the sniff, where the sniff would first add a newline between the
namespace
keyword and the namespace separator (InterfaceSameLine
fixer) and in a subsequent loop would remove that same new line again as it would think it was a space before a comma (SpaceBeforeComma
fixer).Fixed now by adding support for namespace relative interface names in the
implements
check.Includes unit test.
Note: at a glance, this sniff could probably do with a more thorough review and more defensive token walking/more precise checking, but that's outside the scope of this PR.
Suggested changelog entry
PSR2.Classes.ClassDeclaration: using namespace relative interface names in the extends/implements part of a class declaration would lead to a fixer conflict.
Related issues/external references
Related to #152
Types of changes