Skip to content

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
jrfnl added 2 commits April 3, 2024 03:10
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
@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