Skip to content

Commit a3b4e39

Browse files
committed
UselessIfConditionWithReturnSniff: Don't remove comments automatically
1 parent bab12ae commit a3b4e39

File tree

4 files changed

+76
-11
lines changed

4 files changed

+76
-11
lines changed

SlevomatCodingStandard/Sniffs/ControlStructures/UselessIfConditionWithReturnSniff.php

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHP_CodeSniffer\Files\File;
66
use PHP_CodeSniffer\Sniffs\Sniff;
7+
use PHP_CodeSniffer\Util\Tokens;
78
use SlevomatCodingStandard\Helpers\ConditionHelper;
89
use SlevomatCodingStandard\Helpers\TokenHelper;
910
use function array_key_exists;
@@ -60,14 +61,6 @@ public function process(File $phpcsFile, $ifPointer): void
6061
: ConditionHelper::getNegativeCondition($phpcsFile, $tokens[$ifPointer]['parenthesis_opener'] + 1, $tokens[$ifPointer]['parenthesis_closer'] - 1);
6162
};
6263

63-
$isFixable = function (int $ifPointer) use ($phpcsFile, $tokens): bool {
64-
if ($this->assumeAllConditionExpressionsAreAlreadyBoolean) {
65-
return true;
66-
}
67-
68-
return ConditionHelper::conditionReturnsBoolean($phpcsFile, $tokens[$ifPointer]['parenthesis_opener'] + 1, $tokens[$ifPointer]['parenthesis_closer'] - 1);
69-
};
70-
7164
$elsePointer = TokenHelper::findNextEffective($phpcsFile, $tokens[$ifPointer]['scope_closer'] + 1);
7265

7366
$errorParameters = [
@@ -90,7 +83,7 @@ public function process(File $phpcsFile, $ifPointer): void
9083
return;
9184
}
9285

93-
if (!$isFixable($ifPointer)) {
86+
if (!$this->isFixable($phpcsFile, $ifPointer, $tokens[$elsePointer]['scope_closer'])) {
9487
$phpcsFile->addError(...$errorParameters);
9588
return;
9689
}
@@ -119,7 +112,7 @@ public function process(File $phpcsFile, $ifPointer): void
119112
return;
120113
}
121114

122-
if (!$isFixable($ifPointer)) {
115+
if (!$this->isFixable($phpcsFile, $ifPointer, $semicolonPointer)) {
123116
$phpcsFile->addError(...$errorParameters);
124117
return;
125118
}
@@ -139,6 +132,21 @@ public function process(File $phpcsFile, $ifPointer): void
139132
}
140133
}
141134

135+
private function isFixable(File $phpcsFile, int $ifPointer, int $endPointer): bool
136+
{
137+
$tokens = $phpcsFile->getTokens();
138+
139+
if (TokenHelper::findNext($phpcsFile, Tokens::$commentTokens, $ifPointer + 1, $endPointer) !== null) {
140+
return false;
141+
}
142+
143+
if ($this->assumeAllConditionExpressionsAreAlreadyBoolean) {
144+
return true;
145+
}
146+
147+
return ConditionHelper::conditionReturnsBoolean($phpcsFile, $tokens[$ifPointer]['parenthesis_opener'] + 1, $tokens[$ifPointer]['parenthesis_closer'] - 1);
148+
}
149+
142150
private function findBooleanAfterReturnInScope(File $phpcsFile, int $scopeOpenerPointer): ?int
143151
{
144152
$tokens = $phpcsFile->getTokens();

tests/Sniffs/ControlStructures/UselessIfConditionWithReturnSniffTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public function testErrors(): void
1717
{
1818
$report = self::checkFile(__DIR__ . '/data/uselessIfConditionWithReturnErrors.php');
1919

20-
self::assertSame(9, $report->getErrorCount());
20+
self::assertSame(12, $report->getErrorCount());
2121

2222
self::assertSniffError($report, 4, UselessIfConditionWithReturnSniff::CODE_USELESS_IF_CONDITION);
2323
self::assertSniffError($report, 12, UselessIfConditionWithReturnSniff::CODE_USELESS_IF_CONDITION);
@@ -28,6 +28,9 @@ public function testErrors(): void
2828
self::assertSniffError($report, 52, UselessIfConditionWithReturnSniff::CODE_USELESS_IF_CONDITION);
2929
self::assertSniffError($report, 61, UselessIfConditionWithReturnSniff::CODE_USELESS_IF_CONDITION);
3030
self::assertSniffError($report, 70, UselessIfConditionWithReturnSniff::CODE_USELESS_IF_CONDITION);
31+
self::assertSniffError($report, 82, UselessIfConditionWithReturnSniff::CODE_USELESS_IF_CONDITION);
32+
self::assertSniffError($report, 91, UselessIfConditionWithReturnSniff::CODE_USELESS_IF_CONDITION);
33+
self::assertSniffError($report, 100, UselessIfConditionWithReturnSniff::CODE_USELESS_IF_CONDITION);
3134

3235
self::assertAllFixedInFile($report);
3336
}

tests/Sniffs/ControlStructures/data/uselessIfConditionWithReturnErrors.fixed.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,30 @@ function () {
4949
empty($day['from']) || !empty($day['to'])
5050
);
5151
};
52+
53+
function () {
54+
if (true) {
55+
// Comment in if
56+
return true;
57+
}
58+
59+
return false;
60+
};
61+
62+
function () {
63+
if (true) {
64+
return false;
65+
} else {
66+
// Comment in else
67+
return true;
68+
}
69+
};
70+
71+
function () {
72+
if (true) {
73+
return true;
74+
}
75+
76+
// Comment before return
77+
return false;
78+
};

tests/Sniffs/ControlStructures/data/uselessIfConditionWithReturnErrors.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,30 @@ function () {
7777

7878
return true;
7979
};
80+
81+
function () {
82+
if (true) {
83+
// Comment in if
84+
return true;
85+
}
86+
87+
return false;
88+
};
89+
90+
function () {
91+
if (true) {
92+
return false;
93+
} else {
94+
// Comment in else
95+
return true;
96+
}
97+
};
98+
99+
function () {
100+
if (true) {
101+
return true;
102+
}
103+
104+
// Comment before return
105+
return false;
106+
};

0 commit comments

Comments
 (0)