From 2602c9530be116861965f5fb3a4844a0087fa212 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 8 Jan 2025 11:53:52 +0100 Subject: [PATCH 1/4] Squiz/ScopeKeywordSpacing: move parse error test to separate file Done in a way to maintain the error line numbers for the rest of the file. --- .../WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc | 4 ++-- .../ScopeKeywordSpacingUnitTest.1.inc.fixed | 4 ++-- .../WhiteSpace/ScopeKeywordSpacingUnitTest.4.inc | 11 +++++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.4.inc diff --git a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc index 9f871c0e77..f071ee094d 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc +++ b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc @@ -77,10 +77,10 @@ class MyOtherClass $varQ = 'string', $varR = 123; - // Intentionally missing a semicolon for testing. public $varS, - $varT + $varT, + $varU; } // Issue #3188 - static as return type. diff --git a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed index bf284b6188..d16a30f13d 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed @@ -72,10 +72,10 @@ class MyOtherClass $varQ = 'string', $varR = 123; - // Intentionally missing a semicolon for testing. public $varS, - $varT + $varT, + $varU; } // Issue #3188 - static as return type. diff --git a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.4.inc b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.4.inc new file mode 100644 index 0000000000..0459433549 --- /dev/null +++ b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.4.inc @@ -0,0 +1,11 @@ + Date: Wed, 8 Jan 2025 11:57:31 +0100 Subject: [PATCH 2/4] Squiz/ScopeKeywordSpacing: move some method tests into a class ... as otherwise the test methods would be _unintentional_ parse errors. Includes making the method names unique. --- .../ScopeKeywordSpacingUnitTest.1.inc | 28 +++++++++---------- .../ScopeKeywordSpacingUnitTest.1.inc.fixed | 28 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc index f071ee094d..b95878ff07 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc +++ b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc @@ -81,23 +81,23 @@ class MyOtherClass $varS, $varT, $varU; -} -// Issue #3188 - static as return type. -public static function fCreate($attributes = []): static -{ - return static::factory()->create($attributes); -} + // Issue #3188 - static as return type. + public static function staticAsReturnType($attributes = []): static + { + return static::factory()->create($attributes); + } -public static function fCreate($attributes = []): ?static -{ - return static::factory()->create($attributes); -} + public static function nullableStaticReturnType($attributes = []): ?static + { + return static::factory()->create($attributes); + } -// Also account for static used within union types. -public function staticLast($attributes = []): object|static {} -public function staticMiddle(): string|static|object {} -public function staticFirst(): static|object {} + // Also account for static used within union types. + public function staticLast($attributes = []): object|static {} + public function staticMiddle(): string|static|object {} + public function staticFirst(): static|object {} +} // Ensure that static as a scope keyword when preceeded by a colon which is not for a type declaration is still handled. $callback = $cond ? get_fn_name() : static function ($a) { return $a * 10; }; diff --git a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed index d16a30f13d..3b0fdd0015 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed @@ -76,23 +76,23 @@ class MyOtherClass $varS, $varT, $varU; -} -// Issue #3188 - static as return type. -public static function fCreate($attributes = []): static -{ - return static::factory()->create($attributes); -} + // Issue #3188 - static as return type. + public static function staticAsReturnType($attributes = []): static + { + return static::factory()->create($attributes); + } -public static function fCreate($attributes = []): ?static -{ - return static::factory()->create($attributes); -} + public static function nullableStaticReturnType($attributes = []): ?static + { + return static::factory()->create($attributes); + } -// Also account for static used within union types. -public function staticLast($attributes = []): object|static {} -public function staticMiddle(): string|static|object {} -public function staticFirst(): static|object {} + // Also account for static used within union types. + public function staticLast($attributes = []): object|static {} + public function staticMiddle(): string|static|object {} + public function staticFirst(): static|object {} +} // Ensure that static as a scope keyword when preceeded by a colon which is not for a type declaration is still handled. $callback = $cond ? get_fn_name() : static function ($a) { return $a * 10; }; From f08bd690e4cf0734011319ea8ec6fc0b9ff8299f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 8 Jan 2025 12:10:58 +0100 Subject: [PATCH 3/4] Squiz/ScopeKeywordSpacing: add additional tests This commit adds a few of tests for syntaxes in which the target keywords could be used, which were so far not covered by tests: * Tests with the `static` modifier keyword being used for closures. * Tests with visibility keyword applied to class constants. --- .../WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc | 13 +++++++++++++ .../ScopeKeywordSpacingUnitTest.1.inc.fixed | 13 +++++++++++++ .../WhiteSpace/ScopeKeywordSpacingUnitTest.php | 4 ++++ 3 files changed, 30 insertions(+) diff --git a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc index b95878ff07..9a74addc41 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc +++ b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc @@ -174,3 +174,16 @@ final class FinalSpacingCorrect { abstract class AbstractSpacingCorrect { public abstract function spacingCorrect() {} } + +$closure = static function() { return 'spacing correct'; }; +$closure = static function() { return 'spacing incorrect'; }; + +class ConstantVisibility { + public const PUBLIC_SPACING_CORRECT = true; + protected const PROTECTED_SPACING_CORRECT = true; + private const PRIVATE_SPACING_CORRECT = true; + + public const PUBLIC_SPACING_INCORRECT = true; + protected const PROTECTED_SPACING_INCORRECT = true; + private const PRIVATE_SPACING_INCORRECT = true; +} diff --git a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed index 3b0fdd0015..c18bda5c76 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed +++ b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.1.inc.fixed @@ -167,3 +167,16 @@ final class FinalSpacingCorrect { abstract class AbstractSpacingCorrect { public abstract function spacingCorrect() {} } + +$closure = static function() { return 'spacing correct'; }; +$closure = static function() { return 'spacing incorrect'; }; + +class ConstantVisibility { + public const PUBLIC_SPACING_CORRECT = true; + protected const PROTECTED_SPACING_CORRECT = true; + private const PRIVATE_SPACING_CORRECT = true; + + public const PUBLIC_SPACING_INCORRECT = true; + protected const PROTECTED_SPACING_INCORRECT = true; + private const PRIVATE_SPACING_INCORRECT = true; +} diff --git a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.php b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.php index 3773793e65..bd66e943aa 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.php +++ b/src/Standards/Squiz/Tests/WhiteSpace/ScopeKeywordSpacingUnitTest.php @@ -65,6 +65,10 @@ public function getErrorList($testFile='') 163 => 1, 166 => 1, 167 => 1, + 179 => 1, + 186 => 1, + 187 => 1, + 188 => 1, ]; case 'ScopeKeywordSpacingUnitTest.3.inc': From 0d0f8d4ce45b7cef90201abb22eff6dbfd961a9b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 8 Jan 2025 12:38:23 +0100 Subject: [PATCH 4/4] Squiz/ScopeKeywordSpacing: minor efficiency tweak Bow out earlier when the sniff won't be able to action anything anyway (live coding/parse errors). --- .../Squiz/Sniffs/WhiteSpace/ScopeKeywordSpacingSniff.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/WhiteSpace/ScopeKeywordSpacingSniff.php b/src/Standards/Squiz/Sniffs/WhiteSpace/ScopeKeywordSpacingSniff.php index 9ba69dcf14..abf05dc17e 100644 --- a/src/Standards/Squiz/Sniffs/WhiteSpace/ScopeKeywordSpacingSniff.php +++ b/src/Standards/Squiz/Sniffs/WhiteSpace/ScopeKeywordSpacingSniff.php @@ -44,7 +44,9 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - if (isset($tokens[($stackPtr + 1)]) === false) { + $nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true); + if ($nextNonWhitespace === false) { + // Parse error/live coding. Bow out. return; } @@ -126,9 +128,6 @@ public function process(File $phpcsFile, $stackPtr) if ($tokens[($stackPtr + 1)]['code'] !== T_WHITESPACE) { $spacing = 0; - } else if (isset($tokens[($stackPtr + 2)]) === false) { - // Parse error/live coding. Bow out. - return; } else { if ($tokens[($stackPtr + 2)]['line'] !== $tokens[$stackPtr]['line']) { $spacing = 'newline';