Skip to content

Commit

Permalink
Universal/RequireFinalClass: act on more cases
Browse files Browse the repository at this point in the history
As things were, the sniff would bow out if the opening brace for the class could not be found (yet).

This commit changes the sniff to act in more cases, i.e. it does require for a (non-empty) token after the `class` keyword, but once that token is found, the sniff will act.

Includes minor changes to how the message text is build up to prevent weird code snippets being show in the message.

Includes additional test.
  • Loading branch information
jrfnl committed Dec 1, 2022
1 parent 59bb205 commit 8bf00e3
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 7 deletions.
20 changes: 15 additions & 5 deletions Universal/Sniffs/Classes/RequireFinalClassSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\GetTokensAsString;
use PHPCSUtils\Utils\ObjectDeclarations;

Expand Down Expand Up @@ -73,18 +74,27 @@ public function process(File $phpcsFile, $stackPtr)

$phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'not abstract, not final');

$tokens = $phpcsFile->getTokens();
if (isset($tokens[$stackPtr]['scope_opener']) === false) {
$nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true);
if ($nextNonEmpty === false) {
// Live coding or parse error.
return;
}

$snippet = GetTokensAsString::compact($phpcsFile, $stackPtr, $tokens[$stackPtr]['scope_opener'], true);
$snippetEnd = $nextNonEmpty;
$classCloser = '';

$tokens = $phpcsFile->getTokens();
if (isset($tokens[$stackPtr]['scope_opener']) === true) {
$snippetEnd = $tokens[$stackPtr]['scope_opener'];
$classCloser = '}';
}

$snippet = GetTokensAsString::compact($phpcsFile, $stackPtr, $snippetEnd, true);
$fix = $phpcsFile->addFixableError(
'A non-abstract class should be declared as final. Found: %s}',
'A non-abstract class should be declared as final. Found: %s%s',
$stackPtr,
'NonFinalClassFound',
[$snippet]
[$snippet, $classCloser]
);

if ($fix === true) {
Expand Down
5 changes: 4 additions & 1 deletion Universal/Tests/Classes/RequireFinalClassUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@ class ClassImplementing implements MyInterface {}

readonly class ReadonlyClass {}

// Live coding. Ignore. This must be the last test in the file.
// Live coding/parse error, but not one which concerns us. Add the final keyword.
class LiveCoding

// Live coding. Ignore. This must be the last test in the file.
class
5 changes: 4 additions & 1 deletion Universal/Tests/Classes/RequireFinalClassUnitTest.inc.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@ final class ClassImplementing implements MyInterface {}

readonly final class ReadonlyClass {}

// Live coding/parse error, but not one which concerns us. Add the final keyword.
final class LiveCoding

// Live coding. Ignore. This must be the last test in the file.
class LiveCoding
class
1 change: 1 addition & 0 deletions Universal/Tests/Classes/RequireFinalClassUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public function getErrorList()
26 => 1,
28 => 1,
30 => 1,
33 => 1,
];
}

Expand Down

0 comments on commit 8bf00e3

Please sign in to comment.