EarlyExitSniff: Improved fixer

SlevomatCodingStandard/Helpers/IndentationHelper.php

@@ -25,13 +25,17 @@ class IndentationHelper
 
 	public static function getIndentation(File $phpcsFile, int $pointer): string
 	{
-		$endOfLinePointer = TokenHelper::findPreviousContent($phpcsFile, T_WHITESPACE, $phpcsFile->eolChar, $pointer - 1);
+		$tokens = $phpcsFile->getTokens();
+
+		$nonWhitespacePointer = TokenHelper::findPreviousExcluding($phpcsFile, T_WHITESPACE, $pointer - 1);
 
-		if ($endOfLinePointer === null) {
+		if ($tokens[$nonWhitespacePointer]['line'] === $tokens[$pointer]['line']) {
 			return '';
 		}
 
-		return TokenHelper::getContent($phpcsFile, $endOfLinePointer + 1, $pointer - 1);
+		$firstPointerOnLine = TokenHelper::findFirstTokenOnLine($phpcsFile, $pointer);
+
+		return TokenHelper::getContent($phpcsFile, $firstPointerOnLine, $pointer - 1);
 	}
 
 	public static function addIndentation(string $identation): string

SlevomatCodingStandard/Helpers/TokenHelper.php

@@ -297,6 +297,28 @@ public static function findPreviousLocal(File $phpcsFile, $types, int $startPoin
 		return $token === false ? null : $token;
 	}
 
+	/**
+	 * @param File $phpcsFile
+	 * @param int $pointer search starts at this token, inclusive
+	 * @return int
+	 */
+	public static function findFirstTokenOnLine(File $phpcsFile, int $pointer): int
+	{
+		if ($pointer === 0) {
+			return $pointer;
+		}
+
+		$tokens = $phpcsFile->getTokens();
+
+		$line = $tokens[$pointer]['line'];
+
+		do {
+			$pointer--;
+		} while ($tokens[$pointer]['line'] === $line);
+
+		return $pointer + 1;
+	}
+
 	/**
 	 * @param File $phpcsFile
 	 * @param int $pointer search starts at this token, inclusive

SlevomatCodingStandard/Sniffs/ControlStructures/EarlyExitSniff.php

@@ -178,24 +178,24 @@ private function processElse(File $phpcsFile, int $elsePointer): void
 			return;
 		}
 
-		$phpcsFile->fixer->beginChangeset();
-
-		for ($i = $tokens[$previousConditionPointer]['scope_closer'] + 1; $i <= $tokens[$elsePointer]['scope_closer']; $i++) {
-			$phpcsFile->fixer->replaceToken($i, '');
-		}
-
 		$elseCodePointers = $this->getScopeCodePointers($phpcsFile, $elsePointer);
 		$afterIfCode = IndentationHelper::fixIndentation($phpcsFile, $elseCodePointers, IndentationHelper::addIndentation(IndentationHelper::getIndentation($phpcsFile, $previousConditionPointer)));
 
-		$phpcsFile->fixer->addContent(
-			$tokens[$elsePointer]['scope_closer'],
+		$phpcsFile->fixer->beginChangeset();
+
+		$phpcsFile->fixer->replaceToken(
+			$tokens[$previousConditionPointer]['scope_closer'] + 1,
 			sprintf(
 				'%s%s',
 				$phpcsFile->eolChar,
 				$afterIfCode
 			)
 		);
 
+		for ($i = $tokens[$previousConditionPointer]['scope_closer'] + 2; $i <= $tokens[$elsePointer]['scope_closer']; $i++) {
+			$phpcsFile->fixer->replaceToken($i, '');
+		}
+
 		$phpcsFile->fixer->endChangeset();
 	}
 
@@ -308,16 +308,11 @@ private function processIf(File $phpcsFile, int $ifPointer): void
 		$earlyExitCodeIndentation = IndentationHelper::addIndentation($ifIndentation);
 
 		$negativeIfCondition = ConditionHelper::getNegativeCondition($phpcsFile, $tokens[$ifPointer]['parenthesis_opener'], $tokens[$ifPointer]['parenthesis_closer']);
+		$afterIfCode = IndentationHelper::fixIndentation($phpcsFile, $ifCodePointers, $ifIndentation);
 
 		$phpcsFile->fixer->beginChangeset();
 
-		for ($i = $ifPointer; $i <= $tokens[$ifPointer]['scope_closer']; $i++) {
-			$phpcsFile->fixer->replaceToken($i, '');
-		}
-
-		$afterIfCode = IndentationHelper::fixIndentation($phpcsFile, $ifCodePointers, $ifIndentation);
-
-		$phpcsFile->fixer->addContent(
+		$phpcsFile->fixer->replaceToken(
 			$ifPointer,
 			sprintf(
 				'if %s {%s%s%s;%s%s}%s%s',
@@ -332,6 +327,10 @@ private function processIf(File $phpcsFile, int $ifPointer): void
 			)
 		);
 
+		for ($i = $ifPointer + 1; $i <= $tokens[$ifPointer]['scope_closer']; $i++) {
+			$phpcsFile->fixer->replaceToken($i, '');
+		}
+
 		$phpcsFile->fixer->endChangeset();
 	}
 

tests/Helpers/TokenHelperTest.php

@@ -212,6 +212,26 @@ public function testFindNothingPreviousExcludingWithSpecifiedEndPointer(): void
 		], $lastTokenPointer, $openParenthesisTokenPointer));
 	}
 
+	public function testFindFirstTokenOnLine(): void
+	{
+		$phpcsFile = $this->getCodeSnifferFile(
+			__DIR__ . '/data/sampleOne.php'
+		);
+		$functionCallPointer = TokenHelper::findNext($phpcsFile, T_STRING, 0);
+		self::assertTokenPointer(T_STRING, 3, $phpcsFile, $functionCallPointer);
+		self::assertTokenPointer(T_VARIABLE, 3, $phpcsFile, TokenHelper::findFirstTokenOnLine($phpcsFile, $functionCallPointer));
+	}
+
+	public function testFindFirstTokenOnLineForFirstToken(): void
+	{
+		$phpcsFile = $this->getCodeSnifferFile(
+			__DIR__ . '/data/sampleOne.php'
+		);
+		$openTagPointer = TokenHelper::findNext($phpcsFile, T_OPEN_TAG, 0);
+		self::assertTokenPointer(T_OPEN_TAG, 1, $phpcsFile, $openTagPointer);
+		self::assertSame($openTagPointer, TokenHelper::findFirstTokenOnLine($phpcsFile, $openTagPointer));
+	}
+
 	public function testFindFirstTokenOnNextLine(): void
 	{
 		$phpcsFile = $this->getCodeSnifferFile(

tests/Sniffs/ControlStructures/EarlyExitSniffTest.php

@@ -17,21 +17,21 @@ public function testErrors(): void
 	{
 		$report = self::checkFile(__DIR__ . '/data/earlyExitErrors.php');
 
-		self::assertSame(57, $report->getErrorCount());
+		self::assertSame(60, $report->getErrorCount());
 
 		foreach ([6, 15, 24, 33, 42, 50, 58, 66, 74, 82, 90, 98, 108, 149, 157, 165, 191, 199, 207, 376] as $line) {
 			self::assertSniffError($report, $line, EarlyExitSniff::CODE_EARLY_EXIT_NOT_USED, 'Use early exit instead of else.');
 		}
 
-		foreach ([115, 122, 129, 135, 141, 213, 222, 229, 235, 241, 247, 256, 262, 271, 287, 305, 361, 368, 388, 415, 425] as $line) {
+		foreach ([115, 122, 129, 135, 141, 213, 222, 229, 235, 241, 247, 256, 262, 271, 287, 305, 361, 368, 388, 415, 425, 450] as $line) {
 			self::assertSniffError($report, $line, EarlyExitSniff::CODE_EARLY_EXIT_NOT_USED, 'Use early exit to reduce code nesting.');
 		}
 
-		foreach ([173, 182, 328, 353, 398, 440] as $line) {
+		foreach ([173, 182, 328, 353, 398, 440, 462] as $line) {
 			self::assertSniffError($report, $line, EarlyExitSniff::CODE_USELESS_ELSE, 'Remove useless else to reduce code nesting.');
 		}
 
-		foreach ([322, 324, 326, 336, 338, 340, 351, 396, 406, 436] as $line) {
+		foreach ([322, 324, 326, 336, 338, 340, 351, 396, 406, 436, 460] as $line) {
 			self::assertSniffError($report, $line, EarlyExitSniff::CODE_USELESS_ELSEIF, 'Use if instead of elseif.');
 		}
 

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

@@ -507,3 +507,27 @@ function uselessElseWithHeredoc() {
 	XYZ
 EOF;
 }
+
+function inlineCommentAfterForeach()
+{
+	foreach ([] as $_) { // Comment
+		if (false) {
+			continue;
+		}
+
+		$x = 1;
+	}
+}
+
+function moreInlineComments()
+{
+	if (true) { // Comment
+		return 1;
+	}
+
+	if (true) { // Comment
+		return 2;
+	}
+	// Comment
+	return 3;
+}

tests/Sniffs/ControlStructures/data/earlyExitErrors.php

@@ -443,3 +443,23 @@ function uselessElseWithHeredoc() {
 EOF;
 	}
 }
+
+function inlineCommentAfterForeach()
+{
+	foreach ([] as $_) { // Comment
+		if (true) {
+			$x = 1;
+		}
+	}
+}
+
+function moreInlineComments()
+{
+	if (true) { // Comment
+		return 1;
+	} elseif (true) { // Comment
+		return 2;
+	} else { // Comment
+		return 3;
+	}
+}