Skip to content

Commit c34146f

Browse files
jrfnljrfnl
committed
Generic/SpaceAfterNot: make the sniff configurable
This PR makes the `Generic.Formatting.SpaceAfterNot` sniff configurable as discussed in PR 2057. Notes: * Adds a public `spacing` property. Defaults to `1` to maintain BC. * Adds a public `ignoreNewLines` property. Defaults to `false` to maintain BC. * Adds a new non-fixable `CommentFound` error for when non-whitespace tokens are found between the NOT operator and the next non-empty token. * Adjusts the error message for the `Incorrect` error to allow for the flexibility now needed, what with the configurable `spacing` property. Note: the error **code** has not been changed to prevent a BC-break. * Makes the fixer more efficient compared to before. Previously, if a whitespace + new line + whitespace would be found with `spacing` set to `1`, the fixer would need three loops to apply all the fixes, now this is fixed in one go. Includes extensive additional unit tests. Unit tests have been added to the PHP file, but the changes should work just as well for JS. Includes basic xml documentation for the sniff.
1 parent f693a3f commit c34146f

File tree

5 files changed

+308
-17
lines changed

5 files changed

+308
-17
lines changed

‎src/Standards/Generic/Docs/Formatting/SpaceAfterNotStandard.xml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<documentation title="Space After NOT operator">
2+
<standard>
3+
<![CDATA[
4+
Exactly one space is allowed after the NOT operator.
5+
]]>
6+
</standard>
7+
<code_comparison>
8+
<code title="Valid: A NOT operator followed by one space.">
9+
<![CDATA[
10+
if (!<em> </em>$someVar || !<em> </em>$x instanceOf stdClass) {};
11+
]]>
12+
</code>
13+
<code title="Invalid: A NOT operator not followed by whitespace.">
14+
<![CDATA[
15+
if (!<em></em>$someVar || !<em></em>$x instanceOf stdClass) {};
16+
]]>
17+
</code>
18+
<code title="Invalid: A NOT operator followed by a new line or more than one space.">
19+
<![CDATA[
20+
if (!<em> </em>$someVar || !<em>
21+
</em>$x instanceOf stdClass) {};
22+
]]>
23+
</code>
24+
</code_comparison>
25+
</documentation>

‎src/Standards/Generic/Sniffs/Formatting/SpaceAfterNotSniff.php

Lines changed: 78 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use PHP_CodeSniffer\Sniffs\Sniff;
1313
use PHP_CodeSniffer\Files\File;
14+
use PHP_CodeSniffer\Util\Tokens;
1415

1516
class SpaceAfterNotSniff implements Sniff
1617
{
@@ -25,6 +26,20 @@ class SpaceAfterNotSniff implements Sniff
2526
'JS',
2627
];
2728

29+
/**
30+
* The number of spaces desired after the NOT operator.
31+
*
32+
* @var integer
33+
*/
34+
public $spacing = 1;
35+
36+
/**
37+
* Allow newlines instead of spaces.
38+
*
39+
* @var boolean
40+
*/
41+
public $ignoreNewlines = false;
42+
2843

2944
/**
3045
* Returns an array of tokens this test wants to listen for.
@@ -49,25 +64,78 @@ public function register()
4964
*/
5065
public function process(File $phpcsFile, $stackPtr)
5166
{
52-
$tokens = $phpcsFile->getTokens();
67+
$tokens = $phpcsFile->getTokens();
68+
$this->spacing = (int) $this->spacing;
69+
70+
$nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true);
71+
if ($nextNonEmpty === false) {
72+
return;
73+
}
74+
75+
if ($this->ignoreNewlines === true
76+
&& $tokens[$stackPtr]['line'] !== $tokens[$nextNonEmpty]['line']
77+
) {
78+
return;
79+
}
80+
81+
if ($this->spacing === 0 && $nextNonEmpty === ($stackPtr + 1)) {
82+
return;
83+
}
5384

54-
$spacing = 0;
55-
if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
56-
$spacing = $tokens[($stackPtr + 1)]['length'];
85+
$maybePlural = '';
86+
if ($this->spacing !== 1) {
87+
$maybePlural = 's';
5788
}
5889

59-
if ($spacing === 1) {
90+
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true);
91+
if ($nextNonEmpty !== $nextNonWhitespace) {
92+
$error = 'Expected %s space%s after NOT operator; comment found';
93+
$data = [
94+
$this->spacing,
95+
$maybePlural,
96+
];
97+
$phpcsFile->addError($error, $stackPtr, 'CommentFound', $data);
98+
6099
return;
61100
}
62101

63-
$message = 'There must be a single space after a NOT operator; %s found';
64-
$fix = $phpcsFile->addFixableError($message, $stackPtr, 'Incorrect', [$spacing]);
102+
$found = 0;
103+
if ($tokens[$stackPtr]['line'] !== $tokens[$nextNonEmpty]['line']) {
104+
$found = 'newline';
105+
} else if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
106+
$found = $tokens[($stackPtr + 1)]['length'];
107+
}
108+
109+
if ($found === $this->spacing) {
110+
return;
111+
}
112+
113+
$error = 'Expected %s space%s after NOT operator; %s found';
114+
$data = [
115+
$this->spacing,
116+
$maybePlural,
117+
$found,
118+
];
119+
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'Incorrect', $data);
65120

66121
if ($fix === true) {
67-
if ($spacing === 0) {
68-
$phpcsFile->fixer->addContent($stackPtr, ' ');
122+
$padding = str_repeat(' ', $this->spacing);
123+
if ($found === 0) {
124+
$phpcsFile->fixer->addContent($stackPtr, $padding);
69125
} else {
70-
$phpcsFile->fixer->replaceToken(($stackPtr + 1), ' ');
126+
$phpcsFile->fixer->beginChangeset();
127+
$start = ($stackPtr + 1);
128+
129+
if ($this->spacing > 0) {
130+
$phpcsFile->fixer->replaceToken($start, $padding);
131+
++$start;
132+
}
133+
134+
for ($i = $start; $i < $nextNonWhitespace; $i++) {
135+
$phpcsFile->fixer->replaceToken($i, '');
136+
}
137+
138+
$phpcsFile->fixer->endChangeset();
71139
}
72140
}
73141

‎src/Standards/Generic/Tests/Formatting/SpaceAfterNotUnitTest.inc

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,86 @@
11
<?php
2-
if (!$someVar || !$x instanceOf stdClass) {}
32
if (! $someVar || ! $x instanceOf stdClass) {}
3+
if (!$someVar || !$x instanceOf stdClass) {}
4+
if (! $someVar || ! $x instanceOf stdClass) {}
45
if (!foo() && (!$x || true)) {}
56
$var = !($x || $y);
7+
$var = ! ($x || $y);
8+
$var = ! /*comment*/ ($x || $y);
9+
10+
$baz = function () {
11+
return ! $bar;
12+
};
13+
14+
if ( !
15+
($x || $y)
16+
) {
17+
return !$bar;
18+
}
19+
20+
if ( ! // phpcs:ignore Standard.Cat.SniffName -- for reasons.
21+
($x || $y)
22+
) {}
23+
24+
// phpcs:set Generic.Formatting.SpaceAfterNot ignoreNewlines true
25+
if ( !
26+
($x || $y)
27+
) {
28+
return !$bar;
29+
}
30+
31+
if ( ! // phpcs:ignore Standard.Cat.SniffName -- for reasons.
32+
($x || $y)
33+
) {}
34+
// phpcs:set Generic.Formatting.SpaceAfterNot ignoreNewlines false
35+
36+
// phpcs:set Generic.Formatting.SpaceAfterNot spacing 2
37+
if (! $someVar || ! $x instanceOf stdClass) {}
38+
if (!$someVar || !$x instanceOf stdClass) {}
39+
if (! $someVar || ! $x instanceOf stdClass) {}
40+
if (!foo() && (! $x || true)) {}
41+
$var = ! ($x || $y);
42+
$var = ! ($x || $y);
43+
44+
$baz = function () {
45+
return ! $bar;
46+
};
47+
48+
if ( !
49+
($x || $y)
50+
) {
51+
return !$bar;
52+
}
53+
54+
// phpcs:set Generic.Formatting.SpaceAfterNot spacing 0
55+
if (!$someVar || !$x instanceOf stdClass) {}
56+
if (! $someVar || ! $x instanceOf stdClass) {}
57+
if (! foo() && (!$x || true)) {}
58+
$var = ! ($x || $y);
59+
$var = ! /*comment*/ ($x || $y);
60+
61+
$baz = function () {
62+
return ! $bar;
63+
};
64+
65+
if ( !
66+
($x || $y)
67+
) {
68+
return ! $bar;
69+
}
70+
71+
if ( ! // phpcs:ignore Standard.Cat.SniffName -- for reasons.
72+
($x || $y)
73+
) {}
74+
75+
// phpcs:set Generic.Formatting.SpaceAfterNot ignoreNewlines true
76+
if ( !
77+
($x || $y)
78+
) {
79+
return ! $bar;
80+
}
81+
82+
if ( ! // phpcs:ignore Standard.Cat.SniffName -- for reasons.
83+
($x || $y)
84+
) {}
85+
// phpcs:set Generic.Formatting.SpaceAfterNot ignoreNewlines false
86+
// phpcs:set Generic.Formatting.SpaceAfterNot spacing 1

‎src/Standards/Generic/Tests/Formatting/SpaceAfterNotUnitTest.inc.fixed

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,83 @@
11
<?php
22
if (! $someVar || ! $x instanceOf stdClass) {}
33
if (! $someVar || ! $x instanceOf stdClass) {}
4+
if (! $someVar || ! $x instanceOf stdClass) {}
45
if (! foo() && (! $x || true)) {}
56
$var = ! ($x || $y);
7+
$var = ! ($x || $y);
8+
$var = ! /*comment*/ ($x || $y);
9+
10+
$baz = function () {
11+
return ! $bar;
12+
};
13+
14+
if ( ! ($x || $y)
15+
) {
16+
return ! $bar;
17+
}
18+
19+
if ( ! // phpcs:ignore Standard.Cat.SniffName -- for reasons.
20+
($x || $y)
21+
) {}
22+
23+
// phpcs:set Generic.Formatting.SpaceAfterNot ignoreNewlines true
24+
if ( !
25+
($x || $y)
26+
) {
27+
return ! $bar;
28+
}
29+
30+
if ( ! // phpcs:ignore Standard.Cat.SniffName -- for reasons.
31+
($x || $y)
32+
) {}
33+
// phpcs:set Generic.Formatting.SpaceAfterNot ignoreNewlines false
34+
35+
// phpcs:set Generic.Formatting.SpaceAfterNot spacing 2
36+
if (! $someVar || ! $x instanceOf stdClass) {}
37+
if (! $someVar || ! $x instanceOf stdClass) {}
38+
if (! $someVar || ! $x instanceOf stdClass) {}
39+
if (! foo() && (! $x || true)) {}
40+
$var = ! ($x || $y);
41+
$var = ! ($x || $y);
42+
43+
$baz = function () {
44+
return ! $bar;
45+
};
46+
47+
if ( ! ($x || $y)
48+
) {
49+
return ! $bar;
50+
}
51+
52+
// phpcs:set Generic.Formatting.SpaceAfterNot spacing 0
53+
if (!$someVar || !$x instanceOf stdClass) {}
54+
if (!$someVar || !$x instanceOf stdClass) {}
55+
if (!foo() && (!$x || true)) {}
56+
$var = !($x || $y);
57+
$var = ! /*comment*/ ($x || $y);
58+
59+
$baz = function () {
60+
return !$bar;
61+
};
62+
63+
if ( !($x || $y)
64+
) {
65+
return !$bar;
66+
}
67+
68+
if ( ! // phpcs:ignore Standard.Cat.SniffName -- for reasons.
69+
($x || $y)
70+
) {}
71+
72+
// phpcs:set Generic.Formatting.SpaceAfterNot ignoreNewlines true
73+
if ( !
74+
($x || $y)
75+
) {
76+
return !$bar;
77+
}
78+
79+
if ( ! // phpcs:ignore Standard.Cat.SniffName -- for reasons.
80+
($x || $y)
81+
) {}
82+
// phpcs:set Generic.Formatting.SpaceAfterNot ignoreNewlines false
83+
// phpcs:set Generic.Formatting.SpaceAfterNot spacing 1

‎src/Standards/Generic/Tests/Formatting/SpaceAfterNotUnitTest.php

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,54 @@ class SpaceAfterNotUnitTest extends AbstractSniffUnitTest
2121
* The key of the array should represent the line number and the value
2222
* should represent the number of errors that should occur on that line.
2323
*
24+
* @param string $testFile The name of the file being tested.
25+
*
2426
* @return array<int, int>
2527
*/
26-
public function getErrorList()
28+
public function getErrorList($testFile='')
2729
{
28-
return [
29-
2 => 2,
30-
4 => 2,
31-
5 => 1,
32-
];
30+
switch ($testFile) {
31+
case 'SpaceAfterNotUnitTest.inc':
32+
return [
33+
3 => 2,
34+
4 => 2,
35+
5 => 2,
36+
6 => 1,
37+
7 => 1,
38+
8 => 1,
39+
11 => 1,
40+
14 => 1,
41+
17 => 1,
42+
20 => 1,
43+
28 => 1,
44+
38 => 2,
45+
39 => 2,
46+
40 => 1,
47+
41 => 1,
48+
42 => 1,
49+
48 => 1,
50+
51 => 1,
51+
56 => 2,
52+
57 => 1,
53+
58 => 1,
54+
59 => 1,
55+
62 => 1,
56+
65 => 1,
57+
68 => 1,
58+
71 => 1,
59+
79 => 1,
60+
];
61+
62+
case 'SpaceAfterNotUnitTest.js':
63+
return [
64+
2 => 2,
65+
4 => 2,
66+
5 => 1,
67+
];
68+
69+
default:
70+
return [];
71+
}//end switch
3372

3473
}//end getErrorList()
3574

0 commit comments

Comments
 (0)