From dacb6d6bf4d7672361ebc0ce49bbd01a5d7b7367 Mon Sep 17 00:00:00 2001 From: Volodymyr Shelmuk Date: Wed, 14 Aug 2024 14:28:11 +0300 Subject: [PATCH 1/6] feat: add sniff to enforce alternative syntax for control structures having inline HTML --- .../AlternativeControlStructureSniff.php | 162 ++++++++++++++++++ tests/unit/fixtures/alternative-structure.php | 104 +++++++++++ 2 files changed, 266 insertions(+) create mode 100644 InpsydeTemplates/Sniffs/Formatting/AlternativeControlStructureSniff.php create mode 100644 tests/unit/fixtures/alternative-structure.php diff --git a/InpsydeTemplates/Sniffs/Formatting/AlternativeControlStructureSniff.php b/InpsydeTemplates/Sniffs/Formatting/AlternativeControlStructureSniff.php new file mode 100644 index 0000000..5d308bb --- /dev/null +++ b/InpsydeTemplates/Sniffs/Formatting/AlternativeControlStructureSniff.php @@ -0,0 +1,162 @@ + + */ + 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 $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( + $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 List of scope condition positions + */ + private function findChainedIssues(File $phpcsFile, int $stackPtr): array + { + /** @var array $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 $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; + } +} diff --git a/tests/unit/fixtures/alternative-structure.php b/tests/unit/fixtures/alternative-structure.php new file mode 100644 index 0000000..7bb222b --- /dev/null +++ b/tests/unit/fixtures/alternative-structure.php @@ -0,0 +1,104 @@ + + + +
Maybe.
+ + + +
Yes.
+ + + +
+ +
+ +
YES
+ Date: Thu, 15 Aug 2024 10:16:35 +0300 Subject: [PATCH 2/6] feat: add sniff to enforce short echo tag on single-line echoing --- .../Sniffs/Formatting/ShortEchoTagSniff.php | 99 +++++++++++++++++++ tests/unit/fixtures/short-echo-tag.php | 28 ++++++ 2 files changed, 127 insertions(+) create mode 100644 InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php create mode 100644 tests/unit/fixtures/short-echo-tag.php diff --git a/InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php b/InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php new file mode 100644 index 0000000..d573b3f --- /dev/null +++ b/InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php @@ -0,0 +1,99 @@ + + */ + 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 $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; + } + + $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 `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, 'endChangeset(); + } +} diff --git a/tests/unit/fixtures/short-echo-tag.php b/tests/unit/fixtures/short-echo-tag.php new file mode 100644 index 0000000..47620b1 --- /dev/null +++ b/tests/unit/fixtures/short-echo-tag.php @@ -0,0 +1,28 @@ + + + + + + + + + + + + + From 1127d57df9f0c9697d198ace890295b74a2781be Mon Sep 17 00:00:00 2001 From: Volodymyr Shelmuk Date: Thu, 15 Aug 2024 10:22:16 +0300 Subject: [PATCH 3/6] docs: ShortEchoTag and TrailingSemicolon description --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9b5ffcf..0349004 100644 --- a/README.md +++ b/README.md @@ -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 From ea485f599e0e546d4152a0488d97194da0eb2990 Mon Sep 17 00:00:00 2001 From: Volodymyr Shelmuk Date: Wed, 21 Aug 2024 13:50:12 +0300 Subject: [PATCH 4/6] chore: fix comment --- .../Sniffs/Formatting/AlternativeControlStructureSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/InpsydeTemplates/Sniffs/Formatting/AlternativeControlStructureSniff.php b/InpsydeTemplates/Sniffs/Formatting/AlternativeControlStructureSniff.php index 5d308bb..a097452 100644 --- a/InpsydeTemplates/Sniffs/Formatting/AlternativeControlStructureSniff.php +++ b/InpsydeTemplates/Sniffs/Formatting/AlternativeControlStructureSniff.php @@ -10,7 +10,7 @@ use PHPCSUtils\Utils\ControlStructures; /** - * The implementation is inspired by Universal.DisallowAlternativeSyntaxSniff. + * The implementation is inspired by Universal.ControlStructures.DisallowAlternativeSyntaxSniff. * * @link https://github.com/PHPCSStandards/PHPCSExtra/blob/ed86bb117c340f654eab603a06b95a437ac619c9/Universal/Sniffs/ControlStructures/DisallowAlternativeSyntaxSniff.php * From 1694d1c1294993715da22e923ff0c0c67f10f032 Mon Sep 17 00:00:00 2001 From: Volodymyr Shelmuk Date: Wed, 21 Aug 2024 13:53:42 +0300 Subject: [PATCH 5/6] refactor: create variable when we need it --- InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php b/InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php index d573b3f..f3c4f71 100644 --- a/InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php +++ b/InpsydeTemplates/Sniffs/Formatting/ShortEchoTagSniff.php @@ -39,7 +39,6 @@ public function process(File $phpcsFile, $stackPtr): void /** @var array $tokens */ $tokens = $phpcsFile->getTokens(); - $currentLine = $tokens[$stackPtr]['line']; $prevPtr = $phpcsFile->findPrevious( Tokens::$emptyTokens, @@ -53,6 +52,7 @@ public function process(File $phpcsFile, $stackPtr): void } $prevToken = $tokens[$prevPtr]; + $currentLine = $tokens[$stackPtr]['line']; if ($prevToken['line'] !== $currentLine) { return; From d2fb3b315aa3a06dcb9509c980b4a1f7af6188ce Mon Sep 17 00:00:00 2001 From: Volodymyr Shelmuk Date: Wed, 21 Aug 2024 13:56:06 +0300 Subject: [PATCH 6/6] tests: provide nested fixture for AlternativeControlStructureSniff --- tests/unit/fixtures/alternative-structure.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/fixtures/alternative-structure.php b/tests/unit/fixtures/alternative-structure.php index 7bb222b..33a6713 100644 --- a/tests/unit/fixtures/alternative-structure.php +++ b/tests/unit/fixtures/alternative-structure.php @@ -66,7 +66,11 @@ $flag = 'YES'; } } elseif ($flag === 'NO') { // @phpcsWarningOnThisLine - echo 'no'; + while ($flag !== 'YES') { // @phpcsWarningOnThisLine + $flag = 'YES'; + ?> +
No. Yes.
+