From c6359c4754d378735e2f4a8ce5679b480525fb10 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 29 Mar 2024 10:07:06 +0100 Subject: [PATCH 1/2] 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. 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. --- src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php | 5 +++-- .../PSR2/Tests/Classes/ClassDeclarationUnitTest.inc | 5 +++++ .../PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed | 5 +++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php index 22cf2abc0f..6eaa6ac049 100644 --- a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php +++ b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php @@ -397,9 +397,10 @@ public function processOpen(File $phpcsFile, $stackPtr) } }//end if } else if ($tokens[($className - 1)]['code'] !== T_NS_SEPARATOR - || $tokens[($className - 2)]['code'] !== T_STRING + || ($tokens[($className - 2)]['code'] !== T_STRING + && $tokens[($className - 2)]['code'] !== T_NAMESPACE) ) { - // Not part of a longer fully qualified class name. + // Not part of a longer fully qualified or namespace relative class name. if ($tokens[($className - 1)]['code'] === T_COMMA || ($tokens[($className - 1)]['code'] === T_NS_SEPARATOR && $tokens[($className - 2)]['code'] === T_COMMA) diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc index 303846b721..00892d00c7 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc @@ -282,3 +282,8 @@ readonly class ReadonlyClassWithComment { } + +// Safeguard against fixer conflict when there are namespace relative interface names in extends. +interface FooBar extends namespace\BarFoo +{ +} diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed index 78dbbbb4b2..797f50b5d7 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed @@ -270,3 +270,8 @@ readonly class ReadonlyClassWithComment { } + +// Safeguard against fixer conflict when there are namespace relative interface names in extends. +interface FooBar extends namespace\BarFoo +{ +} From 1a3486e0934c18e89b95c500ffbecbdf69206db0 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 29 Mar 2024 10:11:33 +0100 Subject: [PATCH 2/2] 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. 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. --- src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php | 3 ++- .../PSR2/Tests/Classes/ClassDeclarationUnitTest.inc | 6 ++++++ .../PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed | 6 ++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php index 6eaa6ac049..d356e76ef2 100644 --- a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php +++ b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php @@ -314,7 +314,8 @@ public function processOpen(File $phpcsFile, $stackPtr) if ($checkingImplements === true && $multiLineImplements === true && ($tokens[($className - 1)]['code'] !== T_NS_SEPARATOR - || $tokens[($className - 2)]['code'] !== T_STRING) + || ($tokens[($className - 2)]['code'] !== T_STRING + && $tokens[($className - 2)]['code'] !== T_NAMESPACE)) ) { $prev = $phpcsFile->findPrevious( [ diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc index 00892d00c7..962f182dfd 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc @@ -287,3 +287,9 @@ readonly interface FooBar extends namespace\BarFoo { } + +// Safeguard against fixer conflict when there are namespace relative interface names in a multi-line implements. +class BarFoo implements + namespace\BarFoo +{ +} diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed index 797f50b5d7..8725c4fac7 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed @@ -275,3 +275,9 @@ readonly interface FooBar extends namespace\BarFoo { } + +// Safeguard against fixer conflict when there are namespace relative interface names in a multi-line implements. +class BarFoo implements + namespace\BarFoo +{ +}