Skip to content

Commit 030a200

Browse files
jrfnljrfnl
committed
PSR2/ClassDeclaration: bug fix - space before class keyword is not checked correctly
* If there would be a newline + indentation between a modifier keyword and the "class" keyword, the space between them would not be flagged as incorrect (should be one space). * Along the same lines, if there would be a comment between the modifier keyword and the "class" keyword, the space between them would not be checked, let alone flagged. Fixed now. Includes additional tests.
1 parent ed4a8ee commit 030a200

File tree

4 files changed

+95
-33
lines changed

4 files changed

+95
-33
lines changed

‎src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php

+42-33
Original file line numberDiff line numberDiff line change
@@ -65,42 +65,51 @@ public function processOpen(File $phpcsFile, $stackPtr)
6565
$stackPtrType = strtolower($tokens[$stackPtr]['content']);
6666

6767
// Check alignment of the keyword and braces.
68-
if ($tokens[($stackPtr - 1)]['code'] === T_WHITESPACE) {
69-
$prevContent = $tokens[($stackPtr - 1)]['content'];
70-
if ($prevContent !== $phpcsFile->eolChar) {
71-
$blankSpace = substr($prevContent, strpos($prevContent, $phpcsFile->eolChar));
72-
$spaces = strlen($blankSpace);
73-
74-
if (in_array($tokens[($stackPtr - 2)]['code'], [T_ABSTRACT, T_FINAL, T_READONLY], true) === true
75-
&& $spaces !== 1
76-
) {
77-
$prevContent = strtolower($tokens[($stackPtr - 2)]['content']);
78-
$error = 'Expected 1 space between %s and %s keywords; %s found';
79-
$data = [
80-
$prevContent,
81-
$stackPtrType,
82-
$spaces,
83-
];
84-
85-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpaceBeforeKeyword', $data);
86-
if ($fix === true) {
87-
$phpcsFile->fixer->replaceToken(($stackPtr - 1), ' ');
88-
}
89-
}
90-
} else if ($tokens[($stackPtr - 2)]['code'] === T_ABSTRACT
91-
|| $tokens[($stackPtr - 2)]['code'] === T_FINAL
92-
|| $tokens[($stackPtr - 2)]['code'] === T_READONLY
93-
) {
94-
$prevContent = strtolower($tokens[($stackPtr - 2)]['content']);
95-
$error = 'Expected 1 space between %s and %s keywords; newline found';
96-
$data = [
97-
$prevContent,
68+
$classModifiers = [
69+
T_ABSTRACT => T_ABSTRACT,
70+
T_FINAL => T_FINAL,
71+
T_READONLY => T_READONLY,
72+
];
73+
74+
$prevNonSpace = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true);
75+
$prevNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true);
76+
77+
if (isset($classModifiers[$tokens[$prevNonEmpty]['code']]) === true) {
78+
$spaces = 0;
79+
$errorCode = 'SpaceBeforeKeyword';
80+
if ($tokens[$prevNonEmpty]['line'] !== $tokens[$stackPtr]['line']) {
81+
$spaces = 'newline';
82+
$errorCode = 'NewlineBeforeKeyword';
83+
} else if ($tokens[($stackPtr - 1)]['code'] === T_WHITESPACE) {
84+
$spaces = $tokens[($stackPtr - 1)]['length'];
85+
}
86+
87+
if ($spaces !== 1) {
88+
$error = 'Expected 1 space between %s and %s keywords; %s found';
89+
$data = [
90+
strtolower($tokens[$prevNonEmpty]['content']),
9891
$stackPtrType,
92+
$spaces,
9993
];
10094

101-
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'NewlineBeforeKeyword', $data);
102-
if ($fix === true) {
103-
$phpcsFile->fixer->replaceToken(($stackPtr - 1), ' ');
95+
if ($prevNonSpace !== $prevNonEmpty) {
96+
// Comment found between modifier and class keyword. Do not auto-fix.
97+
$phpcsFile->addError($error, $stackPtr, $errorCode, $data);
98+
} else {
99+
$fix = $phpcsFile->addFixableError($error, $stackPtr, $errorCode, $data);
100+
if ($fix === true) {
101+
if ($spaces === 0) {
102+
$phpcsFile->fixer->addContentBefore($stackPtr, ' ');
103+
} else {
104+
$phpcsFile->fixer->beginChangeset();
105+
$phpcsFile->fixer->replaceToken(($stackPtr - 1), ' ');
106+
for ($i = ($stackPtr - 2); $i > $prevNonSpace; $i--) {
107+
$phpcsFile->fixer->replaceToken($i, ' ');
108+
}
109+
110+
$phpcsFile->fixer->endChangeset();
111+
}
112+
}
104113
}
105114
}//end if
106115
}//end if

‎src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc

+25
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,28 @@ if (!class_exists('IndentedDeclaration')) {
257257

258258
}
259259
}
260+
261+
// Space between modifier and class keyword would not be flagged nor fixed if newline + indentation.
262+
final
263+
class FinalClassWithIndentation
264+
{
265+
}
266+
267+
readonly
268+
class ReadonlyClassWithIndentation
269+
{
270+
}
271+
272+
// And would also not be flagged if there was a comment between (not auto-fixable).
273+
final/*comment*/class FinalClassWithComment
274+
{
275+
}
276+
abstract /*comment*/ class AbstractClassWithComment
277+
{
278+
}
279+
280+
readonly
281+
// comment
282+
class ReadonlyClassWithComment
283+
{
284+
}

‎src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed

+23
Original file line numberDiff line numberDiff line change
@@ -247,3 +247,26 @@ if (!class_exists('IndentedDeclaration')) {
247247
function foo() {}
248248
}
249249
}
250+
251+
// Space between modifier and class keyword would not be flagged nor fixed if newline + indentation.
252+
final class FinalClassWithIndentation
253+
{
254+
}
255+
256+
readonly class ReadonlyClassWithIndentation
257+
{
258+
}
259+
260+
// And would also not be flagged if there was a comment between (not auto-fixable).
261+
final/*comment*/class FinalClassWithComment
262+
{
263+
}
264+
abstract /*comment*/ class AbstractClassWithComment
265+
{
266+
}
267+
268+
readonly
269+
// comment
270+
class ReadonlyClassWithComment
271+
{
272+
}

‎src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.php

+5
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ public function getErrorList()
7171
244 => 1,
7272
248 => 1,
7373
258 => 1,
74+
263 => 1,
75+
268 => 1,
76+
273 => 1,
77+
276 => 1,
78+
282 => 1,
7479
];
7580

7681
}//end getErrorList()

0 commit comments

Comments
 (0)