Skip to content

Commit fcf374a

Browse files
committed
Fix sniff.
1 parent 788978d commit fcf374a

File tree

3 files changed

+189
-4
lines changed

3 files changed

+189
-4
lines changed
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
<?php
2+
3+
namespace PSR2R\Sniffs\ControlStructures;
4+
5+
use PHP_CodeSniffer\Files\File;
6+
use PSR2R\Tools\AbstractSniff;
7+
8+
/**
9+
* Too much else is considered a code-smell and can often be resolved by returning early.
10+
*
11+
* @author Mark Scherer
12+
* @license MIT
13+
*/
14+
class UnneededElseSniff extends AbstractSniff {
15+
16+
/**
17+
* @inheritDoc
18+
*/
19+
public function register(): array {
20+
return [T_ELSE, T_ELSEIF];
21+
}
22+
23+
/**
24+
* @inheritDoc
25+
*/
26+
public function process(File $phpcsFile, $stackPtr) {
27+
$tokens = $phpcsFile->getTokens();
28+
29+
// Check that ALL preceding branches in the if/elseif chain have early exits
30+
if (!$this->allPrecedingBranchesHaveEarlyExits($phpcsFile, $stackPtr)) {
31+
return;
32+
}
33+
34+
$fix = $phpcsFile->addFixableError(
35+
'Unneeded ' . $tokens[$stackPtr]['type'] . ' detected.',
36+
$stackPtr,
37+
'UnneededElse',
38+
);
39+
if (!$fix) {
40+
return;
41+
}
42+
43+
if ($tokens[$stackPtr]['code'] === T_ELSEIF) {
44+
$this->fixElseIfToIf($phpcsFile, $stackPtr);
45+
46+
return;
47+
}
48+
49+
if (empty($tokens[$stackPtr]['scope_opener']) || empty($tokens[$stackPtr]['scope_closer'])) {
50+
return;
51+
}
52+
53+
$phpcsFile->fixer->beginChangeset();
54+
55+
$prevIndex = $phpcsFile->findPrevious(T_WHITESPACE, $stackPtr - 1, null, true);
56+
$line = $tokens[$prevIndex]['line'];
57+
58+
for ($i = $prevIndex + 1; $i < $stackPtr; $i++) {
59+
$phpcsFile->fixer->replaceToken($i, '');
60+
}
61+
62+
$phpcsFile->fixer->addNewline($prevIndex);
63+
64+
$phpcsFile->fixer->replaceToken($stackPtr, '');
65+
66+
$nextScopeStartIndex = $tokens[$stackPtr]['scope_opener'];
67+
$nextScopeEndIndex = $tokens[$stackPtr]['scope_closer'];
68+
69+
for ($i = $stackPtr + 1; $i < $nextScopeStartIndex; $i++) {
70+
$phpcsFile->fixer->replaceToken($i, '');
71+
}
72+
73+
$prevEndIndex = $phpcsFile->findPrevious(T_WHITESPACE, $nextScopeEndIndex - 1, null, true);
74+
75+
$phpcsFile->fixer->replaceToken($nextScopeStartIndex, '');
76+
$phpcsFile->fixer->replaceToken($nextScopeEndIndex, '');
77+
78+
for ($i = $prevEndIndex + 1; $i < $nextScopeEndIndex; $i++) {
79+
$phpcsFile->fixer->replaceToken($i, '');
80+
}
81+
82+
// Fix indentation
83+
for ($i = $nextScopeStartIndex + 1; $i < $prevEndIndex; $i++) {
84+
if ($tokens[$i]['line'] === $line || $tokens[$i]['type'] !== 'T_WHITESPACE') {
85+
continue;
86+
}
87+
$this->outdent($phpcsFile, $i);
88+
}
89+
90+
$phpcsFile->fixer->endChangeset();
91+
}
92+
93+
/**
94+
* Check if all preceding branches in the if/elseif chain have early exits.
95+
*
96+
* @param \PHP_CodeSniffer\Files\File $phpcsFile
97+
* @param int $stackPtr
98+
*
99+
* @return bool
100+
*/
101+
protected function allPrecedingBranchesHaveEarlyExits(File $phpcsFile, int $stackPtr): bool {
102+
$tokens = $phpcsFile->getTokens();
103+
104+
// Walk backwards through the if/elseif chain
105+
$currentPtr = $stackPtr;
106+
107+
while (true) {
108+
$prevScopeEndIndex = $phpcsFile->findPrevious(T_WHITESPACE, $currentPtr - 1, null, true);
109+
if (!$prevScopeEndIndex || empty($tokens[$prevScopeEndIndex]['scope_opener'])) {
110+
return false;
111+
}
112+
113+
$scopeStartIndex = $tokens[$prevScopeEndIndex]['scope_opener'];
114+
$prevScopeLastTokenIndex = $phpcsFile->findPrevious(T_WHITESPACE, $prevScopeEndIndex - 1, null, true);
115+
116+
if ($tokens[$prevScopeLastTokenIndex]['type'] !== 'T_SEMICOLON') {
117+
return false;
118+
}
119+
120+
// Check if this branch has an early exit
121+
$returnEarlyIndex = $phpcsFile->findPrevious(
122+
[T_RETURN, T_CONTINUE, T_BREAK],
123+
$prevScopeLastTokenIndex - 1,
124+
$scopeStartIndex + 1,
125+
);
126+
127+
if (!$returnEarlyIndex) {
128+
return false;
129+
}
130+
131+
// Make sure it's the last statement (no other semicolons after the early exit)
132+
for ($i = $returnEarlyIndex + 1; $i < $prevScopeLastTokenIndex; $i++) {
133+
if ($tokens[$i]['type'] === 'T_SEMICOLON') {
134+
return false;
135+
}
136+
}
137+
138+
// Find the condition token (if/elseif)
139+
$prevParenthesisEndIndex = $phpcsFile->findPrevious(T_WHITESPACE, $scopeStartIndex - 1, null, true);
140+
if (!$prevParenthesisEndIndex || !array_key_exists('parenthesis_opener', $tokens[$prevParenthesisEndIndex])) {
141+
return false;
142+
}
143+
144+
$parenthesisStartIndex = $tokens[$prevParenthesisEndIndex]['parenthesis_opener'];
145+
$prevConditionIndex = $phpcsFile->findPrevious(T_WHITESPACE, $parenthesisStartIndex - 1, null, true);
146+
147+
if (!in_array($tokens[$prevConditionIndex]['code'], [T_IF, T_ELSEIF], true)) {
148+
return false;
149+
}
150+
151+
// If we found an IF, we're done checking (this is the start of the chain)
152+
if ($tokens[$prevConditionIndex]['code'] === T_IF) {
153+
return true;
154+
}
155+
156+
// Otherwise, it's an ELSEIF, continue checking the previous branch
157+
$currentPtr = $prevConditionIndex;
158+
}
159+
}
160+
161+
/**
162+
* @param \PHP_CodeSniffer\Files\File $phpcsFile
163+
* @param int $stackPtr
164+
*
165+
* @return void
166+
*/
167+
protected function fixElseIfToIf(File $phpcsFile, int $stackPtr): void {
168+
$tokens = $phpcsFile->getTokens();
169+
170+
$phpcsFile->fixer->beginChangeset();
171+
172+
$prevIndex = $phpcsFile->findPrevious(T_WHITESPACE, $stackPtr - 1, null, true);
173+
$indentation = $this->getIndentationWhitespace($phpcsFile, $prevIndex);
174+
175+
for ($i = $prevIndex + 1; $i < $stackPtr; $i++) {
176+
$phpcsFile->fixer->replaceToken($i, '');
177+
}
178+
179+
$phpcsFile->fixer->addNewline($prevIndex);
180+
181+
$phpcsFile->fixer->replaceToken($stackPtr, $indentation . 'if');
182+
183+
$phpcsFile->fixer->endChangeset();
184+
}
185+
186+
}

PSR2R/ruleset.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@
8787
<rule ref="SlevomatCodingStandard.ControlStructures.RequireShortTernaryOperator"/>
8888
<rule ref="SlevomatCodingStandard.ControlStructures.RequireNullCoalesceOperator"/>
8989
<rule ref="SlevomatCodingStandard.ControlStructures.AssignmentInCondition"/>
90-
<rule ref="SlevomatCodingStandard.ControlStructures.EarlyExit"/>
9190

9291
<rule ref="SlevomatCodingStandard.Arrays.DisallowImplicitArrayCreation"/>
9392

docs/sniffs.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ PSR2 (6 sniffs)
108108
- PSR2.Namespaces.NamespaceDeclaration
109109
- PSR2.Namespaces.UseDeclaration
110110

111-
PSR2R (41 sniffs)
111+
PSR2R (42 sniffs)
112112
-----------------
113113
- PSR2R.Classes.BraceOnSameLine
114114
- PSR2R.Classes.InterfaceName
@@ -132,6 +132,7 @@ PSR2R (41 sniffs)
132132
- PSR2R.ControlStructures.ElseIfDeclaration
133133
- PSR2R.ControlStructures.NoInlineAssignment
134134
- PSR2R.ControlStructures.TernarySpacing
135+
- PSR2R.ControlStructures.UnneededElse
135136
- PSR2R.Files.ClosingTag
136137
- PSR2R.Files.EndFileNewline
137138
- PSR2R.Methods.MethodDeclaration
@@ -162,7 +163,7 @@ PSR12 (7 sniffs)
162163
- PSR12.Namespaces.CompoundNamespaceDepth
163164
- PSR12.Operators.OperatorSpacing
164165

165-
SlevomatCodingStandard (46 sniffs)
166+
SlevomatCodingStandard (45 sniffs)
166167
----------------------------------
167168
- SlevomatCodingStandard.Arrays.DisallowImplicitArrayCreation
168169
- SlevomatCodingStandard.Arrays.MultiLineArrayEndBracketPlacement
@@ -178,7 +179,6 @@ SlevomatCodingStandard (46 sniffs)
178179
- SlevomatCodingStandard.ControlStructures.AssignmentInCondition
179180
- SlevomatCodingStandard.ControlStructures.DisallowContinueWithoutIntegerOperandInSwitch
180181
- SlevomatCodingStandard.ControlStructures.DisallowYodaComparison
181-
- SlevomatCodingStandard.ControlStructures.EarlyExit
182182
- SlevomatCodingStandard.ControlStructures.JumpStatementsSpacing
183183
- SlevomatCodingStandard.ControlStructures.NewWithParentheses
184184
- SlevomatCodingStandard.ControlStructures.RequireNullCoalesceOperator

0 commit comments

Comments
 (0)