Skip to content

Commit

Permalink
EarlyExit: don't break the complete PHPCS run for unsupported syntaxes
Browse files Browse the repository at this point in the history
While it is your prerogative, of course, to choose not to support certain syntaxes, when these are encountered the sniff should exit silently.

As things were, the sniff would exit with an _uncaught exception_ which within PHPCS was then translated to an `Internal.Exception` error which stops the PHPCS dead for the file being scanned, this includes breaking the run for all other sniffs in a project ruleset.

Exceptions are intended for developers, not for end-users.

The end-user of PHPCS can do nothing with an uncaught exception. Instead, this exception should be caught within the sniff and he sniff should return silently as it couldn't be determined whether something is or isn't an error for the rules the sniff is checking for.

Includes adjusted unit tests + a new unit test for control structures using alternative syntax, which clearly isn't supported either and would - until now - cause an `Undefined index: scope_condition` notice, which - again - would stop the PHPCS scan dead.
  • Loading branch information
jrfnl authored and kukulich committed Jan 16, 2020
1 parent 15df878 commit d20970e
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 20 deletions.
26 changes: 22 additions & 4 deletions SlevomatCodingStandard/Sniffs/ControlStructures/EarlyExitSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use SlevomatCodingStandard\Helpers\ConditionHelper;
use SlevomatCodingStandard\Helpers\IndentationHelper;
use SlevomatCodingStandard\Helpers\TokenHelper;
use Throwable;
use function array_key_exists;
use function in_array;
use function sort;
Expand Down Expand Up @@ -73,10 +74,16 @@ private function processElse(File $phpcsFile, int $elsePointer): void
$tokens = $phpcsFile->getTokens();

if (!array_key_exists('scope_opener', $tokens[$elsePointer])) {
throw new Exception('"else" without curly braces is not supported.');
// Else without curly braces is not supported.
return;
}

$allConditionsPointers = $this->getAllConditionsPointers($phpcsFile, $elsePointer);
try {
$allConditionsPointers = $this->getAllConditionsPointers($phpcsFile, $elsePointer);
} catch (Throwable $e) {
// Else without curly braces is not supported.
return;
}

$ifPointer = $allConditionsPointers[0];
$ifEarlyExitPointer = null;
Expand Down Expand Up @@ -195,7 +202,12 @@ private function processElseIf(File $phpcsFile, int $elseIfPointer): void
{
$tokens = $phpcsFile->getTokens();

$allConditionsPointers = $this->getAllConditionsPointers($phpcsFile, $elseIfPointer);
try {
$allConditionsPointers = $this->getAllConditionsPointers($phpcsFile, $elseIfPointer);
} catch (Throwable $e) {
// Elseif without curly braces is not supported.
return;
}

$elseIfEarlyExitPointer = null;
$previousConditionEarlyExitPointer = null;
Expand Down Expand Up @@ -256,7 +268,8 @@ private function processIf(File $phpcsFile, int $ifPointer): void
$tokens = $phpcsFile->getTokens();

if (!array_key_exists('scope_closer', $tokens[$ifPointer])) {
throw new Exception('"if" without curly braces is not supported.');
// If without curly braces is not supported.
return;
}

$nextPointer = TokenHelper::findNextExcluding($phpcsFile, T_WHITESPACE, $tokens[$ifPointer]['scope_closer'] + 1);
Expand Down Expand Up @@ -364,6 +377,11 @@ private function getAllConditionsPointers(File $phpcsFile, int $conditionPointer

$conditionsPointers = [$conditionPointer];

if (isset($tokens[$conditionPointer]['scope_opener']) && $tokens[$tokens[$conditionPointer]['scope_opener']]['code'] === T_COLON) {
// Alternative control structure syntax.
throw new Exception(sprintf('"%s" without curly braces is not supported.', $tokens[$conditionPointer]['content']));
}

if ($tokens[$conditionPointer]['code'] !== T_IF) {
$currentConditionPointer = $conditionPointer;
do {
Expand Down
38 changes: 22 additions & 16 deletions tests/Sniffs/ControlStructures/EarlyExitSniffTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace SlevomatCodingStandard\Sniffs\ControlStructures;

use SlevomatCodingStandard\Sniffs\TestCase;
use Throwable;

class EarlyExitSniffTest extends TestCase
{
Expand Down Expand Up @@ -41,37 +40,44 @@ public function testErrors(): void

public function testIfWithoutCurlyBraces(): void
{
self::expectException(Throwable::class);
self::expectExceptionMessage('"if" without curly braces is not supported.');
self::checkFile(__DIR__ . '/data/earlyExitIfWithoutCurlyBraces.php', [], [EarlyExitSniff::CODE_EARLY_EXIT_NOT_USED]);
$report = self::checkFile(__DIR__ . '/data/earlyExitIfWithoutCurlyBraces.php', [], [EarlyExitSniff::CODE_EARLY_EXIT_NOT_USED]);

self::assertSame(0, $report->getErrorCount());
}

public function testElseifWithoutCurlyBraces(): void
{
self::expectException(Throwable::class);
self::expectExceptionMessage('"elseif" without curly braces is not supported.');
self::checkFile(__DIR__ . '/data/earlyExitElseifWithoutCurlyBraces.php', [], [EarlyExitSniff::CODE_USELESS_ELSEIF]);
$report = self::checkFile(__DIR__ . '/data/earlyExitElseifWithoutCurlyBraces.php', [], [EarlyExitSniff::CODE_USELESS_ELSEIF]);

self::assertSame(0, $report->getErrorCount());
}

public function testElseifWithSpace(): void
{
self::expectException(Throwable::class);
self::expectExceptionMessage('"else" without curly braces is not supported.');
self::checkFile(__DIR__ . '/data/earlyExitElseifWithSpace.php', [], [EarlyExitSniff::CODE_USELESS_ELSEIF]);
$report = self::checkFile(__DIR__ . '/data/earlyExitElseifWithSpace.php', [], [EarlyExitSniff::CODE_USELESS_ELSEIF]);

self::assertSame(0, $report->getErrorCount());
}

public function testElseWithoutCurlyBraces(): void
{
self::expectException(Throwable::class);
self::expectExceptionMessage('"else" without curly braces is not supported.');
self::checkFile(__DIR__ . '/data/earlyExitElseWithoutCurlyBraces.php', [], [EarlyExitSniff::CODE_USELESS_ELSE]);
$report = self::checkFile(__DIR__ . '/data/earlyExitElseWithoutCurlyBraces.php', [], [EarlyExitSniff::CODE_USELESS_ELSE]);

self::assertSame(0, $report->getErrorCount());
}

public function testInvalidElseif(): void
{
self::expectException(Throwable::class);
self::expectExceptionMessage('"else" without curly braces is not supported.');
self::checkFile(__DIR__ . '/data/earlyExitInvalidElseif.php', [], [EarlyExitSniff::CODE_USELESS_ELSEIF]);
$report = self::checkFile(__DIR__ . '/data/earlyExitInvalidElseif.php', [], [EarlyExitSniff::CODE_USELESS_ELSEIF]);

self::assertSame(0, $report->getErrorCount());
}

public function testAlternativeSyntax(): void
{
$report = self::checkFile(__DIR__ . '/data/earlyExitAlternativeSyntax.php', [], [EarlyExitSniff::CODE_USELESS_ELSEIF]);

self::assertSame(0, $report->getErrorCount());
}

public function testIgnoredStandaloneIfInScopeNoErrors(): void
Expand Down
12 changes: 12 additions & 0 deletions tests/Sniffs/ControlStructures/data/earlyExitAlternativeSyntax.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

function () {
if (true) : ?>
<div></div>
<?php elseif (false) : ?>
<div></div>
<?php else : ?>
<div></div>
<?php
endif;
};

0 comments on commit d20970e

Please sign in to comment.