Skip to content
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

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Mar 29, 2024

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

  • Bug fix (non-breaking change which fixes an issue)

Copy link
Member

@fredden fredden left a 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.

@jrfnl jrfnl force-pushed the feature/psr2-classdeclaration-support-namespace-relative-names branch from ce1322f to a4f2ec8 Compare March 30, 2024 02:46
@jrfnl jrfnl force-pushed the feature/psr2-classdeclaration-support-namespace-relative-names branch from a4f2ec8 to 18672c5 Compare March 30, 2024 02:55
@jrfnl jrfnl modified the milestones: 3.9.1, 3.9.2 Mar 31, 2024
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.
@jrfnl jrfnl force-pushed the feature/psr2-classdeclaration-support-namespace-relative-names branch from 18672c5 to 1a3486e Compare April 3, 2024 01:11
@jrfnl
Copy link
Member Author

jrfnl commented Apr 3, 2024

Rebased without changes, merging once the build has passed.

@jrfnl jrfnl enabled auto-merge April 3, 2024 01:12
@jrfnl jrfnl merged commit a6b9bfa into master Apr 3, 2024
44 checks passed
@jrfnl jrfnl deleted the feature/psr2-classdeclaration-support-namespace-relative-names branch April 3, 2024 01:29
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.

2 participants