Skip to content

Commit 56abf7d

Browse files
jrfnljrfnl
committed
PEAR/ClassDeclaration: add missing fixed files, more tests + minor fixes
The sniff contains fixers, but didn't have `fixed` files. While I was at it, I've reviewed the sniff and applied any improvements I could think of: * Prevent moving a PHPCS ignore annotation trailing after a class open brace. If found, the error will still be thrown, but won't be auto-fixed. Includes new unit test. * Simplified the "found indent" length determination. This will prevent the sniff throwing false positives for correctly indented, tab indented braces and from replacing tabs with spaces (not the concern of this sniff). This should also lower the risk of fixer conflicts if/when this sniff is used in a tab-based standard. To test this, a secondary test case file has been added which is tested with `tabWidth=4`. * Added tests documenting the sniff and fixer behaviour when too little/too much indentation is found for indented class declarations. * Added unit test to document the behaviour of the sniff when there is a comment on a line of its own between the class declaration and the opening brace. Includes minor change to the relevant error message text for accuracy. * Added unit test to test the parse error warning, which was previously untested. This test has been added to the new second test case file to keep it easy to add new tests to the main test case file. Includes additional unit tests for the actual fixes.
1 parent 995300e commit 56abf7d

File tree

6 files changed

+216
-16
lines changed

6 files changed

+216
-16
lines changed

‎package.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
893893
<dir name="Tests">
894894
<dir name="Classes">
895895
<file baseinstalldir="PHP/CodeSniffer" name="ClassDeclarationUnitTest.1.inc" role="test" />
896+
<file baseinstalldir="PHP/CodeSniffer" name="ClassDeclarationUnitTest.1.inc.fixed" role="test" />
897+
<file baseinstalldir="PHP/CodeSniffer" name="ClassDeclarationUnitTest.2.inc" role="test" />
896898
<file baseinstalldir="PHP/CodeSniffer" name="ClassDeclarationUnitTest.php" role="test" />
897899
</dir>
898900
<dir name="Commenting">

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public function process(File $phpcsFile, $stackPtr)
7575
$phpcsFile->recordMetric($stackPtr, 'Class opening brace placement', 'new line');
7676

7777
if ($braceLine > ($classLine + 1)) {
78-
$error = 'Opening brace of a %s must be on the line following the %s declaration; found %s line(s)';
78+
$error = 'Opening brace of a %s must be on the line following the %s declaration; found %s blank line(s)';
7979
$data = [
8080
$tokens[$stackPtr]['content'],
8181
$tokens[$stackPtr]['content'],
@@ -101,9 +101,16 @@ public function process(File $phpcsFile, $stackPtr)
101101

102102
if ($tokens[($curlyBrace + 1)]['content'] !== $phpcsFile->eolChar) {
103103
$error = 'Opening %s brace must be on a line by itself';
104-
$fix = $phpcsFile->addFixableError($error, $curlyBrace, 'OpenBraceNotAlone', $errorData);
105-
if ($fix === true) {
106-
$phpcsFile->fixer->addNewline($curlyBrace);
104+
105+
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($curlyBrace + 1), null, true);
106+
if ($tokens[$nextNonWhitespace]['code'] === T_PHPCS_IGNORE) {
107+
// Don't auto-fix if the next thing is a PHPCS ignore annotation.
108+
$phpcsFile->addError($error, $curlyBrace, 'OpenBraceNotAlone', $errorData);
109+
} else {
110+
$fix = $phpcsFile->addFixableError($error, $curlyBrace, 'OpenBraceNotAlone', $errorData);
111+
if ($fix === true) {
112+
$phpcsFile->fixer->addNewline($curlyBrace);
113+
}
107114
}
108115
}
109116

@@ -112,8 +119,7 @@ public function process(File $phpcsFile, $stackPtr)
112119
if ($prevContent === $phpcsFile->eolChar) {
113120
$spaces = 0;
114121
} else {
115-
$blankSpace = substr($prevContent, strpos($prevContent, $phpcsFile->eolChar));
116-
$spaces = strlen($blankSpace);
122+
$spaces = $tokens[($curlyBrace - 1)]['length'];
117123
}
118124

119125
$first = $phpcsFile->findFirstOnLine(T_WHITESPACE, $stackPtr, true);

‎src/Standards/PEAR/Tests/Classes/ClassDeclarationUnitTest.1.inc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,33 @@ var_dump(new class(10) extends SomeClass implements SomeInterface {
8080

8181
use SomeTrait;
8282
});
83+
84+
class IncorrectClassDeclarationWithCommentAtEnd extends correctClassDeclaration /* Comment */ {
85+
}
86+
87+
class CorrectClassDeclarationWithCommentAtEnd extends correctClassDeclaration
88+
/* Comment */
89+
{
90+
}
91+
92+
// Don't move phpcs:ignore comments.
93+
class PHPCSIgnoreAnnotationAfterOpeningBrace
94+
{ // phpcs:ignore Standard.Cat.Sniff -- for reasons.
95+
}
96+
97+
// Moving any of the other trailing phpcs: comments is ok.
98+
class PHPCSAnnotationAfterOpeningBrace
99+
{ // phpcs:disable Standard.Cat.Sniff -- for reasons.
100+
}
101+
102+
if (!class_exists('ClassOpeningBraceShouldBeIndented')) {
103+
abstract class ClassOpeningBraceShouldBeIndented
104+
{
105+
}
106+
}
107+
108+
if (!class_exists('ClassOpeningBraceTooMuchIndentation')) {
109+
final class ClassOpeningBraceTooMuchIndentation
110+
{
111+
}
112+
}

‎src/Standards/PEAR/Tests/Classes/ClassDeclarationUnitTest.1.inc.fixed

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?php
2+
3+
// Correct declarations.
4+
class CorrectClassDeclaration
5+
{
6+
7+
}
8+
9+
abstract class CorrectClassDeclarationWithExtends extends correctClassDeclaration
10+
{
11+
12+
}
13+
14+
final class CorrectClassDeclarationWithImplements implements correctClassDeclaration
15+
{
16+
17+
}
18+
19+
20+
// Incorrect placement of opening braces
21+
class IncorrectBracePlacement
22+
{
23+
}
24+
class IncorrectBracePlacementWithExtends extends correctClassDeclaration
25+
{
26+
}
27+
class IncorrectBracePlacementWithImplements implements correctClassDeclaration
28+
{
29+
}
30+
31+
// Incorrect code placement for opening brace.
32+
class IncorrectCodeAfterOpeningBrace
33+
{
34+
echo phpinfo();
35+
36+
}//end class
37+
38+
class IncorrectClassDeclarationWithExtends extends correctClassDeclaration
39+
{
40+
41+
}
42+
43+
class IncorrectBracePlacement
44+
{
45+
}
46+
47+
abstract class CodeSnifferFail
48+
extends
49+
ArrayObject
50+
implements
51+
Serializable,
52+
Iterator,
53+
Countable,
54+
OuterIterator,
55+
RecursiveIterator
56+
{
57+
}
58+
59+
abstract class CodeSnifferFail
60+
extends
61+
ArrayObject
62+
implements
63+
Serializable,
64+
Iterator,
65+
Countable,
66+
OuterIterator,
67+
RecursiveIterator
68+
{
69+
}
70+
71+
namespace A
72+
{
73+
class B
74+
{
75+
}
76+
}
77+
78+
$util->setLogger(new class {});
79+
80+
var_dump(new class(10) extends SomeClass implements SomeInterface {
81+
private $num;
82+
83+
public function __construct($num)
84+
{
85+
$this->num = $num;
86+
}
87+
88+
use SomeTrait;
89+
});
90+
91+
class IncorrectClassDeclarationWithCommentAtEnd extends correctClassDeclaration /* Comment */
92+
{
93+
}
94+
95+
class CorrectClassDeclarationWithCommentAtEnd extends correctClassDeclaration
96+
/* Comment */
97+
{
98+
}
99+
100+
// Don't move phpcs:ignore comments.
101+
class PHPCSIgnoreAnnotationAfterOpeningBrace
102+
{ // phpcs:ignore Standard.Cat.Sniff -- for reasons.
103+
}
104+
105+
// Moving any of the other trailing phpcs: comments is ok.
106+
class PHPCSAnnotationAfterOpeningBrace
107+
{
108+
// phpcs:disable Standard.Cat.Sniff -- for reasons.
109+
}
110+
111+
if (!class_exists('ClassOpeningBraceShouldBeIndented')) {
112+
abstract class ClassOpeningBraceShouldBeIndented
113+
{
114+
}
115+
}
116+
117+
if (!class_exists('ClassOpeningBraceTooMuchIndentation')) {
118+
final class ClassOpeningBraceTooMuchIndentation
119+
{
120+
}
121+
}

‎src/Standards/PEAR/Tests/Classes/ClassDeclarationUnitTest.2.inc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
if (!class_exists('ClassOpeningBraceTabIndented')) {
4+
abstract class ClassOpeningBraceTabIndented
5+
{
6+
}
7+
}
8+
9+
10+
// Needs to be last test in the file. Intentional parse error.
11+
class MyParseError extends Exception

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

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,25 @@ class ClassDeclarationUnitTest extends AbstractSniffUnitTest
1515
{
1616

1717

18+
/**
19+
* Get a list of CLI values to set before the file is tested.
20+
*
21+
* @param string $testFile The name of the file being tested.
22+
* @param \PHP_CodeSniffer\Config $config The config data for the test run.
23+
*
24+
* @return void
25+
*/
26+
public function setCliValues($testFile, $config)
27+
{
28+
if ($testFile === 'ClassDeclarationUnitTest.1.inc') {
29+
return;
30+
}
31+
32+
$config->tabWidth = 4;
33+
34+
}//end setCliValues()
35+
36+
1837
/**
1938
* Returns the lines where errors should occur.
2039
*
@@ -25,18 +44,23 @@ class ClassDeclarationUnitTest extends AbstractSniffUnitTest
2544
*
2645
* @return array<int, int>
2746
*/
28-
public function getErrorList($testFile='ClassDeclarationUnitTest.1.inc')
47+
public function getErrorList($testFile='')
2948
{
3049
switch ($testFile) {
3150
case 'ClassDeclarationUnitTest.1.inc':
3251
return [
33-
21 => 1,
34-
22 => 1,
35-
23 => 1,
36-
27 => 1,
37-
33 => 1,
38-
38 => 1,
39-
49 => 1,
52+
21 => 1,
53+
22 => 1,
54+
23 => 1,
55+
27 => 1,
56+
33 => 1,
57+
38 => 1,
58+
49 => 1,
59+
84 => 1,
60+
94 => 1,
61+
99 => 1,
62+
104 => 1,
63+
110 => 1,
4064
];
4165

4266
default:
@@ -52,11 +76,17 @@ public function getErrorList($testFile='ClassDeclarationUnitTest.1.inc')
5276
* The key of the array should represent the line number and the value
5377
* should represent the number of warnings that should occur on that line.
5478
*
79+
* @param string $testFile The name of the file being tested.
80+
*
5581
* @return array<int, int>
5682
*/
57-
public function getWarningList()
83+
public function getWarningList($testFile='')
5884
{
59-
return [];
85+
if ($testFile === 'ClassDeclarationUnitTest.2.inc') {
86+
return [11 => 1];
87+
}
88+
89+
return[];
6090

6191
}//end getWarningList()
6292

0 commit comments

Comments
 (0)