Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Template] AlternativeControlStructure and ShortEchoTag sniffs #97

Merged
merged 6 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
<?php

declare(strict_types=1);

namespace InpsydeTemplates\Sniffs\Formatting;

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

/**
* The implementation is inspired by Universal.DisallowAlternativeSyntaxSniff.
shvlv marked this conversation as resolved.
Show resolved Hide resolved
*
* @link https://github.com/PHPCSStandards/PHPCSExtra/blob/ed86bb117c340f654eab603a06b95a437ac619c9/Universal/Sniffs/ControlStructures/DisallowAlternativeSyntaxSniff.php
*
* @psalm-type Token = array{
* type: string,
* code: string|int,
* line: int,
* scope_opener?: int,
* scope_closer?: int,
* scope_condition?: int,
* content: string,
* }
*/
final class AlternativeControlStructureSniff implements Sniff
{
/**
* @return list<int|string>
*/
public function register(): array
{
return [
T_IF,
T_WHILE,
T_FOR,
T_FOREACH,
T_SWITCH,
];
}

/**
* @param File $phpcsFile
* @param int $stackPtr
*
* phpcs:disable Inpsyde.CodeQuality.ArgumentTypeDeclaration
*/
public function process(File $phpcsFile, $stackPtr): void
{
if (ControlStructures::hasBody($phpcsFile, $stackPtr) === false) {
// Single line control structure is out of scope.
return;
}

/** @var array<int, Token> $tokens */
$tokens = $phpcsFile->getTokens();
/** @var int | null $scopeOpener */
$openerPtr = $tokens[$stackPtr]['scope_opener'] ?? null;
/** @var int | null $scopeCloser */
$closerPtr = $tokens[$stackPtr]['scope_closer'] ?? null;

if (!isset($openerPtr, $closerPtr, $tokens[$openerPtr])) {
// Inline control structure or parse error.
return;
}

if ($tokens[$openerPtr]['code'] === T_COLON) {
// Alternative control structure.
return;
}

$chainedIssues = $this->findChainedIssues($phpcsFile, $stackPtr);

$message = 'Control structure having inline HTML should use alternative syntax.'
. ' Found "%s".';
foreach ($chainedIssues as $conditionPtr) {
$phpcsFile->addWarning(
shvlv marked this conversation as resolved.
Show resolved Hide resolved
$message,
$conditionPtr,
'Encouraged',
[$tokens[$conditionPtr]['content']]
);
}
}

/**
* We consider if - else (else if) chain as the single structure
* as they should be replaced with alternative syntax altogether.
*
* @return list<int> List of scope condition positions
*/
private function findChainedIssues(File $phpcsFile, int $stackPtr): array
{
/** @var array<int, Token> $tokens */
$tokens = $phpcsFile->getTokens();
$hasInlineHtml = false;
$currentPtr = $stackPtr;
$chainedIssues = [];

do {
$openerPtr = $tokens[$currentPtr]['scope_opener'] ?? null;
$closerPtr = $tokens[$currentPtr]['scope_closer'] ?? null;
if (!isset($openerPtr, $closerPtr)) {
// Something went wrong.
break;
}

$chainedIssues[] = $currentPtr;
if (!$hasInlineHtml) {
$hasInlineHtml = $phpcsFile->findNext(T_INLINE_HTML, ($currentPtr + 1), $closerPtr) !== false;
}

$currentPtr = $this->findNextChainPointer($phpcsFile, $closerPtr);
} while (
is_int($currentPtr)
);

return $hasInlineHtml ? $chainedIssues : [];
}

/**
* Find 3 possible options:
* - else
* - elseif
* - else if
*/
private function findNextChainPointer(File $phpcsFile, int $closerPtr): ?int
{
/** @var array<int, Token> $tokens */
$tokens = $phpcsFile->getTokens();
$firstPtr = $phpcsFile->findNext(
Tokens::$emptyTokens,
($closerPtr + 1),
null,
true
);

if (!is_int($firstPtr) || !isset($tokens[$firstPtr])) {
return null;
}

if ($tokens[$firstPtr]['code'] === T_ELSEIF) {
return $firstPtr;
}

if ($tokens[$firstPtr]['code'] !== T_ELSE) {
return null;
}

$secondPtr = $phpcsFile->findNext(
Tokens::$emptyTokens,
($firstPtr + 1),
null,
true
);

$isIfOpenerPtr = is_int($secondPtr) && isset($tokens[$secondPtr]) && $tokens[$secondPtr]['code'] === T_IF;

return $isIfOpenerPtr ? $secondPtr : $firstPtr;
}
}
99 changes: 99 additions & 0 deletions InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php

declare(strict_types=1);

namespace InpsydeTemplates\Sniffs\Formatting;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;

/**
* @psalm-type Token = array{
* type: string,
* code: string|int,
* line: int
* }
*/
final class ShortEchoTagSniff implements Sniff
{
/**
* @return list<int|string>
*/
public function register(): array
{
return [
T_ECHO,
];
}

/**
* @param File $phpcsFile
* @param int $stackPtr
*
* phpcs:disable Inpsyde.CodeQuality.ArgumentTypeDeclaration
*/
public function process(File $phpcsFile, $stackPtr): void
{
// phpcs:enable Inpsyde.CodeQuality.ArgumentTypeDeclaration

/** @var array<int, Token> $tokens */
$tokens = $phpcsFile->getTokens();
$currentLine = $tokens[$stackPtr]['line'];

$prevPtr = $phpcsFile->findPrevious(
Tokens::$emptyTokens,
($stackPtr - 1),
null,
true
);

if (!is_int($prevPtr) || !isset($tokens[$prevPtr])) {
return;
}

$prevToken = $tokens[$prevPtr];

if ($prevToken['line'] !== $currentLine) {
return;
}

if ($prevToken['code'] !== T_OPEN_TAG) {
return;
}
shvlv marked this conversation as resolved.
Show resolved Hide resolved

$closeTagPtr = $phpcsFile->findNext(
T_CLOSE_TAG,
($stackPtr + 1),
);

if (
!is_int($closeTagPtr)
|| !isset($tokens[$closeTagPtr])
|| $tokens[$closeTagPtr]['line'] !== $currentLine
) {
return;
}

$message = sprintf(
'Single line output on line %d'
. ' should use short echo tag `<?= ` instead of `<?php echo`.',
$currentLine
);

if ($phpcsFile->addFixableWarning($message, $stackPtr, 'Encouraged')) {
$this->fix($prevPtr, $stackPtr, $phpcsFile);
}
}

private function fix(int $openTagPtr, int $echoPtr, File $file): void
{
$fixer = $file->fixer;
$fixer->beginChangeset();

$fixer->replaceToken($echoPtr, '');
$fixer->replaceToken($openTagPtr, '<?=');

$fixer->endChangeset();
}
}
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,11 @@ The recommended way to use the `InpsydeTemplates` ruleset is as follows:

The following template-specific rules are available:

| Sniff Name | Description | Has Config | Auto-Fixable |
|:--------------------|:--------------------------------------------------|:----------:|:------------:|
| `TrailingSemicolon` | Remove trailing semicolon before closing PHP tag. | | ✓ |
| Sniff Name | Description | Has Config | Auto-Fixable |
|:------------------------------|:------------------------------------------------------------|:----------:|:------------:|
| `AlternativeControlStructure` | Encourage usage of alternative syntax with inline HTML. | | |
| `ShortEchoTag` | Replace echo with short echo tag in single-line statements. | | ✓ |
| `TrailingSemicolon` | Remove trailing semicolon before closing PHP tag. | | ✓ |

# Removing or Disabling Rules

Expand Down
104 changes: 104 additions & 0 deletions tests/unit/fixtures/alternative-structure.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?php

declare(strict_types=1);

// @phpcsSniff InpsydeTemplates.Formatting.AlternativeControlStructure

const FLAGS = [
'YES',
'NO',
'MAYBE',
];

$flag = FLAGS[rand(0, 2)];

if ($flag === 'MAYBE') {
echo 'maybe';

while ($flag !== 'YES') {
$flag = 'YES';
}
} elseif ($flag === 'NO') {
echo 'no';
} else if ($flag === 'YES') {
echo 'yes';
} else {
echo 'Non Empty value';
}

if ($flag === 'MAYBE') :
echo 'maybe';

while ($flag !== 'YES') {
$flag = 'YES';
}
elseif ($flag === 'NO') :
echo 'no';
else :
echo 'Non Empty value';
endif;


$arrayOfFlags = [];
for ($i = 1; $i <= 10; $i++) {
$arrayOfFlags[] = FLAGS[rand(0, 2)];
}

foreach ($arrayOfFlags as &$item) {
$item = false;
}
unset($item);

switch ($flag) {
case 'YES':
echo 'It is true';
break;
case 'NO':
echo 'It is false';
break;
}

?>

<?php if ($flag === 'MAYBE') { // @phpcsWarningOnThisLine ?>
<div>Maybe.</div>
<?php while ($flag !== 'YES') {
$flag = 'YES';
}
} elseif ($flag === 'NO') { // @phpcsWarningOnThisLine
echo 'no';
shvlv marked this conversation as resolved.
Show resolved Hide resolved
} else if ($flag === 'YES') { // @phpcsWarningOnThisLine
echo 'yes';
} else { // @phpcsWarningOnThisLine
echo 'Non Empty value';
} ?>

<?php if ($flag === 'MAYBE') { // @phpcsWarningOnThisLine
return;
} else if ($flag === 'NO') { // @phpcsWarningOnThisLine
echo 'no';
} elseif ($flag === 'YES') { // @phpcsWarningOnThisLine ?>
<div>Yes.</div>
<?php } else { // @phpcsWarningOnThisLine
echo 'Non Empty value';
} ?>

<?php
for ($i = 1; $i <= 10; $i++) { // @phpcsWarningOnThisLine ?>
<div><?= $i ?></div>
<?php }

foreach ($arrayOfFlags as $item) { // @phpcsWarningOnThisLine ?>
<div><?= $item ?></div>
<?php }

switch ($flag) { // @phpcsWarningOnThisLine
case 'YES':
?>
<div>YES</div>
<?php
break;
case 'NO':
echo 'It is false';
break;
}
Loading