Skip to content

Commit c552156

Browse files
committed
Fix array bracket issue with comments.
1 parent 1569cbf commit c552156

File tree

7 files changed

+169
-5
lines changed

7 files changed

+169
-5
lines changed

PhpCollective/Sniffs/Arrays/ArrayBracketSpacingSniff.php

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,15 @@ public function process(File $phpcsFile, $stackPtr): void
7070
// Any extra blank lines should be removed
7171
if ($closerLine - $lastContentLine > 1) {
7272
$error = 'Extra blank lines found before array closing bracket';
73+
74+
// Check if the last content is a comment - these are problematic for auto-fixing
75+
if ($tokens[$lastContentPtr]['code'] === T_COMMENT) {
76+
// Report as non-fixable error when last element is a comment
77+
$phpcsFile->addError($error, $closerPtr, 'ExtraBlankLineBeforeCloser');
78+
79+
return;
80+
}
81+
7382
$fix = $phpcsFile->addFixableError($error, $closerPtr, 'ExtraBlankLineBeforeCloser');
7483

7584
if ($fix === true) {
@@ -90,13 +99,33 @@ public function process(File $phpcsFile, $stackPtr): void
9099
}
91100
}
92101

93-
// Remove all tokens between last content and closer
102+
// Find whitespace tokens between last content and closer
103+
$whitespaceTokens = [];
94104
for ($i = $lastContentPtr + 1; $i < $closerPtr; $i++) {
95-
$phpcsFile->fixer->replaceToken($i, '');
105+
if ($tokens[$i]['code'] === T_WHITESPACE) {
106+
$whitespaceTokens[] = $i;
107+
}
96108
}
97109

98-
// Add a single newline with proper indentation after the last content
99-
$phpcsFile->fixer->addContent($lastContentPtr, "\n" . $indent);
110+
// Find the position to insert the newline
111+
// If there's already whitespace, replace it; otherwise add new
112+
$nextToken = $lastContentPtr + 1;
113+
114+
if ($nextToken < $closerPtr && $tokens[$nextToken]['code'] === T_WHITESPACE) {
115+
// There's whitespace right after the last content
116+
// Replace it with a single newline and indent
117+
$phpcsFile->fixer->replaceToken($nextToken, "\n" . $indent);
118+
119+
// Remove any additional whitespace tokens
120+
for ($i = $nextToken + 1; $i < $closerPtr; $i++) {
121+
if ($tokens[$i]['code'] === T_WHITESPACE) {
122+
$phpcsFile->fixer->replaceToken($i, '');
123+
}
124+
}
125+
} else {
126+
// No whitespace immediately after, need to add it
127+
$phpcsFile->fixer->addContent($lastContentPtr, "\n" . $indent);
128+
}
100129

101130
$phpcsFile->fixer->endChangeset();
102131
}

tests/PhpCollective/Sniffs/Arrays/ArrayBracketSpacingSniffTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ class ArrayBracketSpacingSniffTest extends TestCase
1717
*/
1818
public function testArrayBracketSpacingSniffer(): void
1919
{
20-
$this->assertSnifferFindsErrors(new ArrayBracketSpacingSniff(), 4);
20+
// 4 original errors + 1 new error from array with comments
21+
$this->assertSnifferFindsErrors(new ArrayBracketSpacingSniff(), 5);
2122
}
2223

2324
/**
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/**
4+
* MIT License
5+
* For full license information, please view the LICENSE file that was distributed with this source code.
6+
*/
7+
8+
namespace PhpCollective\Test\PhpCollective\Sniffs\Commenting;
9+
10+
use PhpCollective\Sniffs\Commenting\AttributesSniff;
11+
use PhpCollective\Test\TestCase;
12+
13+
class AttributesSniffTest extends TestCase
14+
{
15+
/**
16+
* @return void
17+
*/
18+
public function testAttributesSniffer(): void
19+
{
20+
$this->assertSnifferFindsErrors(new AttributesSniff(), 4);
21+
}
22+
}

tests/_data/ArrayBracketSpacing/after.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,14 @@ public function test(): void
4343
'item1',
4444
'item2',
4545
];
46+
47+
// Array with comment as last element followed by blank lines
48+
// This should report an error but NOT be auto-fixable
49+
$array7 = [
50+
'key' => 'value',
51+
//'conditions' => array('ConversationUsers.status <'=>ConversationUser::STATUS_REMOVED),
52+
//'group' => array('ConversationUser.conversation_id HAVING SUM(...)'), //HAVING COUNT(*) = '.count($users).'
53+
54+
];
4655
}
4756
}

tests/_data/ArrayBracketSpacing/before.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,14 @@ public function test(): void
4848
'item2',
4949

5050
];
51+
52+
// Array with comment as last element followed by blank lines
53+
// This should report an error but NOT be auto-fixable
54+
$array7 = [
55+
'key' => 'value',
56+
//'conditions' => array('ConversationUsers.status <'=>ConversationUser::STATUS_REMOVED),
57+
//'group' => array('ConversationUser.conversation_id HAVING SUM(...)'), //HAVING COUNT(*) = '.count($users).'
58+
59+
];
5160
}
5261
}

tests/_data/Attributes/after.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PhpCollective;
4+
5+
// Correct - FQCN with leading backslash
6+
#[\AllowDynamicProperties]
7+
class CorrectExample1
8+
{
9+
}
10+
11+
// Correct - FQCN with namespace separator tokens
12+
#[\Some\Namespace\MyAttribute]
13+
class CorrectExample2
14+
{
15+
}
16+
17+
// Incorrect - missing leading backslash
18+
#[AllowDynamicProperties]
19+
class IncorrectExample1
20+
{
21+
}
22+
23+
// Incorrect - not FQCN
24+
#[MyAttribute]
25+
class IncorrectExample2
26+
{
27+
}
28+
29+
// Multiple attributes - mixed correct and incorrect
30+
#[\AllowDynamicProperties]
31+
#[MyAttribute]
32+
#[\Another\Namespace\Attr]
33+
class MixedExample
34+
{
35+
}
36+
37+
// Attribute with parameters - correct
38+
#[\Attribute(Attribute::TARGET_CLASS)]
39+
class WithParameters
40+
{
41+
}
42+
43+
// Attribute with parameters - incorrect
44+
#[Attribute(Attribute::TARGET_METHOD)]
45+
class WithParametersIncorrect
46+
{
47+
}

tests/_data/Attributes/before.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PhpCollective;
4+
5+
// Correct - FQCN with leading backslash
6+
#[\AllowDynamicProperties]
7+
class CorrectExample1
8+
{
9+
}
10+
11+
// Correct - FQCN with namespace separator tokens
12+
#[\Some\Namespace\MyAttribute]
13+
class CorrectExample2
14+
{
15+
}
16+
17+
// Incorrect - missing leading backslash
18+
#[AllowDynamicProperties]
19+
class IncorrectExample1
20+
{
21+
}
22+
23+
// Incorrect - not FQCN
24+
#[MyAttribute]
25+
class IncorrectExample2
26+
{
27+
}
28+
29+
// Multiple attributes - mixed correct and incorrect
30+
#[\AllowDynamicProperties]
31+
#[MyAttribute]
32+
#[\Another\Namespace\Attr]
33+
class MixedExample
34+
{
35+
}
36+
37+
// Attribute with parameters - correct
38+
#[\Attribute(Attribute::TARGET_CLASS)]
39+
class WithParameters
40+
{
41+
}
42+
43+
// Attribute with parameters - incorrect
44+
#[Attribute(Attribute::TARGET_METHOD)]
45+
class WithParametersIncorrect
46+
{
47+
}

0 commit comments

Comments
 (0)