diff --git a/Universal/Docs/ControlStructures/DisallowLonelyIfStandard.xml b/Universal/Docs/ControlStructures/DisallowLonelyIfStandard.xml new file mode 100644 index 00000000..4c72980e --- /dev/null +++ b/Universal/Docs/ControlStructures/DisallowLonelyIfStandard.xml @@ -0,0 +1,46 @@ + + + + + + + + elseif ($bar) { + // ... +} + +if ($foo) { + // ... +} else { + if ($bar) { + // ... + } + + doSomethingElse(); + +} + + ]]> + + + if ($bar) { + // ... + } else { + // ... + } +} + ]]> + + + diff --git a/Universal/Sniffs/ControlStructures/DisallowLonelyIfSniff.php b/Universal/Sniffs/ControlStructures/DisallowLonelyIfSniff.php new file mode 100644 index 00000000..397a9e13 --- /dev/null +++ b/Universal/Sniffs/ControlStructures/DisallowLonelyIfSniff.php @@ -0,0 +1,346 @@ +getTokens(); + + /* + * Deal with `else if`. + */ + if (ControlStructures::isElseIf($phpcsFile, $stackPtr) === true) { + // Ignore, not our real target. + return; + } + + if (isset($tokens[$stackPtr]['scope_opener'], $tokens[$stackPtr]['scope_closer']) === false) { + // Either an else without curly braces or a parse error. Ignore. + return; + } + + $outerScopeOpener = $tokens[$stackPtr]['scope_opener']; + $outerScopeCloser = $tokens[$stackPtr]['scope_closer']; + + $nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($outerScopeOpener + 1), $outerScopeCloser, true); + if ($nextNonEmpty === false || $tokens[$nextNonEmpty]['code'] !== \T_IF) { + // Definitely not a lonely if statement. + return; + } + + if (isset($tokens[$nextNonEmpty]['scope_closer']) === false) { + // Either a control structure without curly braces or a parse error. Ignore. + return; + } + + /* + * Find the end of an if - else chain. + */ + $innerIfPtr = $nextNonEmpty; + $innerIfToken = $tokens[$innerIfPtr]; + $autoFixable = true; + $innerScopeCloser = $innerIfToken['scope_closer']; + + /* + * For alternative syntax fixer only. + * Remember the individual inner scope opener and closers so the fixer doesn't need + * to do the same walking over the if/else chain again. + */ + $innerScopes = [ + $innerIfToken['scope_opener'] => $innerScopeCloser, + ]; + + do { + /* + * Handle control structures using alternative syntax. + */ + if ($tokens[$innerScopeCloser]['code'] !== \T_CLOSE_CURLY_BRACKET) { + if ($tokens[$innerScopeCloser]['code'] === \T_ENDIF) { + $nextAfter = $phpcsFile->findNext( + Tokens::$emptyTokens, + ($innerScopeCloser + 1), + $outerScopeCloser, + true + ); + + if ($tokens[$nextAfter]['code'] === \T_SEMICOLON) { + $innerScopeCloser = $nextAfter; + } else { + // Missing semi-colon. Report, but don't auto-fix. + $autoFixable = false; + } + } else { + // This must be an else[if]. + --$innerScopeCloser; + } + } + + $innerNextNonEmpty = $phpcsFile->findNext( + Tokens::$emptyTokens, + ($innerScopeCloser + 1), + $outerScopeCloser, + true + ); + if ($innerNextNonEmpty === false) { + // This was the last closer. + break; + } + + if ($tokens[$innerNextNonEmpty]['code'] !== \T_ELSE + && $tokens[$innerNextNonEmpty]['code'] !== \T_ELSEIF + ) { + // Found another statement after the control structure. The "if" is not lonely. + return; + } + + if (isset($tokens[$innerNextNonEmpty]['scope_closer']) === false) { + // This may still be an "else if"... + $nextAfter = $phpcsFile->findNext( + Tokens::$emptyTokens, + ($innerNextNonEmpty + 1), + $outerScopeCloser, + true + ); + + if ($nextAfter === false + || $tokens[$nextAfter]['code'] !== \T_IF + || isset($tokens[$nextAfter]['scope_closer']) === false + ) { + // Defense in depth. Either a control structure without curly braces or a parse error. Ignore. + return; + } + + $innerNextNonEmpty = $nextAfter; + } + + $innerScopeCloser = $tokens[$innerNextNonEmpty]['scope_closer']; + $innerScopes[$tokens[$innerNextNonEmpty]['scope_opener']] = $innerScopeCloser; + } while (true); + + /* + * As of now, we know we have an error. Check if it can be auto-fixed. + */ + if ($phpcsFile->findNext(\T_WHITESPACE, ($innerScopeCloser + 1), $outerScopeCloser, true) !== false) { + // Comment between the inner and outer closers. + $autoFixable = false; + } + + if ($tokens[$innerScopeCloser]['code'] === \T_SEMICOLON) { + $hasComment = $phpcsFile->findPrevious(\T_WHITESPACE, ($innerScopeCloser - 1), null, true); + if ($tokens[$hasComment]['code'] !== \T_ENDIF) { + // Comment between the "endif" and the semi-colon. + $autoFixable = false; + } + } + + if ($tokens[$outerScopeOpener]['line'] !== $innerIfToken['line']) { + for ($startOfNextLine = ($outerScopeOpener + 1); $startOfNextLine < $innerIfPtr; $startOfNextLine++) { + if ($tokens[$outerScopeOpener]['line'] !== $tokens[$startOfNextLine]['line']) { + break; + } + } + + if ($phpcsFile->findNext(\T_WHITESPACE, $startOfNextLine, $innerIfPtr, true) !== false) { + // Comment between the inner and outer openers. + $autoFixable = false; + } + } + + if (isset($innerIfToken['parenthesis_opener'], $innerIfToken['parenthesis_closer']) === false) { + // Start/end of the condition of the if unclear. Most likely a parse error. + $autoFixable = false; + } + + /* + * Throw the error and potentially fix it. + */ + $error = 'If control structure block found as the only statement within an "else" block. Use elseif instead.'; + $code = 'Found'; + + if ($autoFixable === false) { + $phpcsFile->addError($error, $stackPtr, $code); + return; + } + + $fix = $phpcsFile->addFixableError($error, $stackPtr, $code); + if ($fix === false) { + return; + } + + /* + * Fix it. + */ + $outerInnerSameType = false; + if (($tokens[$outerScopeCloser]['code'] === \T_CLOSE_CURLY_BRACKET + && $tokens[$innerScopeCloser]['code'] === \T_CLOSE_CURLY_BRACKET) + || ($tokens[$outerScopeCloser]['code'] === \T_ENDIF + && $tokens[$innerScopeCloser]['code'] === \T_SEMICOLON) + ) { + $outerInnerSameType = true; + } + + $targetIsCurly = ($tokens[$outerScopeCloser]['code'] === \T_CLOSE_CURLY_BRACKET); + + $innerScopeCount = \count($innerScopes); + + $condition = GetTokensAsString::origContent($phpcsFile, ($innerIfPtr + 1), ($innerIfToken['scope_opener'] - 1)); + if ($targetIsCurly === true) { + $condition = \rtrim($condition) . ' '; + } + + $phpcsFile->fixer->beginChangeset(); + + // Remove the inner if + condition up to and including the scope opener. + for ($i = $innerIfPtr; $i <= $innerIfToken['scope_opener']; $i++) { + $phpcsFile->fixer->replaceToken($i, ''); + } + + // Potentially remove trailing whitespace/new line if there is no comment after the inner condition. + while ($tokens[$i]['line'] === $innerIfToken['line'] + && $tokens[$i]['code'] === \T_WHITESPACE + ) { + $phpcsFile->fixer->replaceToken($i, ''); + ++$i; + } + + // Remove any potential indentation whitespace for the inner if. + if ($tokens[$outerScopeOpener]['line'] !== $innerIfToken['line'] + && $tokens[$i]['line'] !== $innerIfToken['line'] + ) { + $i = ($nextNonEmpty - 1); + while ($tokens[$i]['line'] === $innerIfToken['line'] + && $tokens[$i]['code'] === \T_WHITESPACE + ) { + $phpcsFile->fixer->replaceToken($i, ''); + --$i; + } + } + + // Remove the inner scope closer. + $phpcsFile->fixer->replaceToken($innerScopeCloser, ''); + $i = ($innerScopeCloser - 1); + + // Handle alternative syntax for the closer. + if ($tokens[$innerScopeCloser]['code'] === \T_SEMICOLON) { + // Remove potential whitespace between the "endif" and the semicolon. + while ($tokens[$i]['code'] === \T_WHITESPACE) { + $phpcsFile->fixer->replaceToken($i, ''); + --$i; + } + + // Remove the "endif". + $phpcsFile->fixer->replaceToken($i, ''); + --$i; + } + + // Remove superfluous whitespace before the inner scope closer. + while ($tokens[$i]['code'] === \T_WHITESPACE) { + $phpcsFile->fixer->replaceToken($i, ''); + --$i; + } + + // Replace the else. + $phpcsFile->fixer->replaceToken($stackPtr, 'elseif' . $condition); + + // Remove potential superfluous whitespace between the new condition and the scope opener. + $i = ($stackPtr + 1); + while ($tokens[$i]['line'] === $tokens[$stackPtr]['line'] + && $tokens[$i]['code'] === \T_WHITESPACE + ) { + $phpcsFile->fixer->replaceToken($i, ''); + ++$i; + } + + if ($outerInnerSameType === false + && $innerScopeCount > 1 + ) { + $loop = 1; + foreach ($innerScopes as $opener => $closer) { + if ($targetIsCurly === true) { + if ($loop !== 1) { + // Only handle the opener when it's not the first of the chain as that's already handled above. + $phpcsFile->fixer->replaceToken($opener, ' {'); + } + + if ($loop !== $innerScopeCount) { + // Only handle the closer when it's not the last of the chain as that's already handled above. + $phpcsFile->fixer->addContentBefore($closer, '} '); + } + } else { + if ($loop !== 1) { + // Only handle the opener when it's not the first of the chain as that's already handled above. + $phpcsFile->fixer->replaceToken($opener, ':'); + } + + if ($loop !== $innerScopeCount) { + // Only handle the closer when it's not the last of the chain as that's already handled above. + $phpcsFile->fixer->replaceToken($closer, ''); + + $j = ($closer + 1); + while ($tokens[$j]['code'] === \T_WHITESPACE) { + $phpcsFile->fixer->replaceToken($j, ''); + ++$j; + } + } + } + + ++$loop; + } + } + + $phpcsFile->fixer->endChangeset(); + } +} diff --git a/Universal/Tests/ControlStructures/DisallowLonelyIfUnitTest.inc b/Universal/Tests/ControlStructures/DisallowLonelyIfUnitTest.inc new file mode 100644 index 00000000..c50a9d2c --- /dev/null +++ b/Universal/Tests/ControlStructures/DisallowLonelyIfUnitTest.inc @@ -0,0 +1,275 @@ + => + */ + public function getErrorList() + { + return [ + // Basic checks. + 82 => 1, + 86 => 1, + 94 => 1, + 104 => 1, + 116 => 1, + + // Alternative control structure syntax. + 127 => 1, + 135 => 1, + 145 => 1, + 158 => 1, + 170 => 1, + + // Fixer specific tests with comments. + 183 => 1, + 187 => 1, + 195 => 1, + 203 => 1, + 212 => 1, + 221 => 1, + 230 => 1, + 239 => 1, + 248 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() + { + return []; + } +}