From 7b6bd743937183d00390a9fdd9d3fa7af389ac30 Mon Sep 17 00:00:00 2001
From: Dan Wallis <dan@wallis.nz>
Date: Thu, 6 Jul 2023 14:48:29 +0100
Subject: [PATCH 1/2] Add sniff for deprecated use of $block->escape...

---
 .../Legacy/EscapeMethodsOnBlockClassSniff.php | 100 ++++++++++++++++++
 .../EscapeMethodsOnBlockClassUnitTest.inc     |  87 +++++++++++++++
 ...scapeMethodsOnBlockClassUnitTest.inc.fixed |  87 +++++++++++++++
 .../EscapeMethodsOnBlockClassUnitTest.php     |  45 ++++++++
 4 files changed, 319 insertions(+)
 create mode 100644 Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php
 create mode 100644 Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc
 create mode 100644 Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc.fixed
 create mode 100644 Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.php

diff --git a/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php b/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php
new file mode 100644
index 00000000..f44e3d0c
--- /dev/null
+++ b/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php
@@ -0,0 +1,100 @@
+<?php
+
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+
+namespace Magento2\Sniffs\Legacy;
+
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Util\Tokens;
+
+class EscapeMethodsOnBlockClassSniff implements Sniff
+{
+    private const ESCAPER_METHODS = [
+        'escapeCss' => true,
+        'escapeHtml' => true,
+        'escapeHtmlAttr' => true,
+        'escapeJs' => true,
+        'escapeJsQuote' => true,
+        'escapeQuote' => true,
+        'escapeUrl' => true,
+        'escapeXssInUrl' => true,
+    ];
+
+    public function register()
+    {
+        return [
+            T_OBJECT_OPERATOR,
+        ];
+    }
+
+    public function process(File $phpcsFile, $stackPtr)
+    {
+        $tokens = $phpcsFile->getTokens();
+
+        if ($stackPtr <= 1 || !isset($tokens[$stackPtr + 2])) {
+            return;
+        }
+
+        $objectPtr = $stackPtr - 1;
+        if ($tokens[$objectPtr]['code'] !== T_VARIABLE) {
+            $objectPtr = $phpcsFile->findPrevious(Tokens::$emptyTokens, $objectPtr, null, true);
+
+            if (!$objectPtr) {
+                return;
+            }
+        }
+
+        if ($tokens[$objectPtr]['code'] !== T_VARIABLE
+            || $tokens[$objectPtr]['content'] !== '$block'
+        ) {
+            return;
+        }
+
+        $methodPtr = $stackPtr + 1;
+        if ($tokens[$methodPtr]['code'] !== T_STRING) {
+            $methodPtr = $phpcsFile->findNext(Tokens::$emptyTokens, $methodPtr, null, true);
+
+            if (!$methodPtr) {
+                return;
+            }
+        }
+
+        if ($tokens[$methodPtr]['code'] !== T_STRING
+            || !isset(self::ESCAPER_METHODS[$tokens[$methodPtr]['content']])
+        ) {
+            return;
+        }
+
+        $openParenPtr = $methodPtr + 1;
+        if ($tokens[$openParenPtr]['code'] !== T_OPEN_PARENTHESIS) {
+            $openParenPtr = $phpcsFile->findNext(Tokens::$emptyTokens, $openParenPtr, null, true);
+
+            if (!$openParenPtr) {
+                return;
+            }
+        }
+
+        if ($tokens[$openParenPtr]['code'] !== T_OPEN_PARENTHESIS) {
+            return;
+        }
+
+        $fix = $phpcsFile->addFixableWarning(
+            'Using %s on $block is deprecated. Please use equivalent method on $escaper',
+            $methodPtr,
+            'Found',
+            [
+                $tokens[$methodPtr]['content'], // method name
+            ]
+        );
+
+        if ($fix) {
+            $phpcsFile->fixer->replaceToken($objectPtr, '$escaper');
+        }
+    }
+}
diff --git a/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc
new file mode 100644
index 00000000..260f0c93
--- /dev/null
+++ b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc
@@ -0,0 +1,87 @@
+<?php
+/** @var Magento\Framework\View\Element\Template $block */
+/** @var Magento\Framework\Escaper $escaper */
+?>
+
+<section>
+    <h1>This unescaped output is fine here; other sniffs will complain about it though.</h1>
+
+    <?php echo $block->getSomeString(); ?>
+    <?= $block->getSomeString(); ?>
+    <?= /** @noEscape */ $block->getSomeString(); ?>
+    <?= /** @escapeNotVerified */ $block->getSomeString(); ?>
+</section>
+
+<section>
+    <h1>These should be using equivalent methods on the `$escaper` class, not the `$block` class.</h1>
+
+    Note that I couldn't find any use of this method in any templates within Magento.
+    <?= $block->escapeCss($block->getSomeString()); ?>
+
+    <?= $block->escapeHtml(__($block->getSomeString())) ?>
+    <?= $block->escapeHtml(__($block->getSomeString())); ?>
+    <?= $block->escapeHtml(__($block->getSomeString()), ['strong', 'em', 'span']) ?>
+
+    <div class="<?= $block->escapeHtmlAttr($block->getSomeString()) ?>"></div>
+    <div class="<?= $block->escapeHtmlAttr($block->getSomeString(), true) ?>"></div>
+    <div class="<?= $block->escapeHtmlAttr($block->getSomeString(), false); ?>"></div>
+
+    <script type="text/x-magento-init">
+    {
+        "#chart_<?= $block->escapeJs($block->getData('html_id')) ?>_period": {
+            "Magento_Backend/js/dashboard/chart": {}
+        }
+    }
+    </script>
+
+    The only example of this method being used was in a block class, rather than a template.
+    <?php
+    foreach ($block->getItems() as $item) {
+        $item['sku'] = $block->escapeJsQuote($item['sku']);
+    }
+    ?>
+
+    The only example of this method being used was in a block class, rather than a template.
+    <?= $block->escapeQuote(__($block->getData('welcome'))); ?>
+
+    <a href="<?= $block->escapeUrl($block->getUrl('adminhtml/notification/index')) ?>"> link text </a>
+
+    Note that I couldn't find any use of this method in any templates within Magento.
+    <?= $block->escapeXssInUrl($block->getSomeString()); ?>
+</section>
+
+<section>
+    <h1>These are edge cases for formatting differences</h1>
+
+    <?php
+        $block->escapeHtml('');
+        $block ->escapeHtml('');
+        $block-> escapeHtml('');
+        $block
+            ->escapeHtml('');
+        $block
+
+            ->escapeHtml('');
+        $block->
+            escapeHtml('');
+        $block-> // comment
+            escapeHtml('');
+        $block /* comment */
+            ->escapeHtml('');
+
+        $block /* comment */ -> /* comment */ escapeHtml('');
+    ?>
+</section>
+
+<section>
+    <h1>These close-matches shouldn't be flagged by this sniff.</h1>
+
+    <?= $block->escapeHTML(__($block->getSomeString())) ?>
+    <?= $block->escapeHtmlString(__($block->getSomeString())) ?>
+    <?= $block->escapeHtmlAttribute($block->getSomeString()) ?>
+    <?= $block->escapeCSS($block->getSomeString()); ?>
+    <?= $block->escapeJS($block->getData('html_id')) ?>
+    <?= $block->escapeJavaScript($block->getData('html_id')) ?>
+    <?= $block->escapeQuotes(__($block->getData('welcome'))); ?>
+    <?= $block->escapeURL($block->getUrl('adminhtml/notification/index')) ?>
+</section>
diff --git a/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc.fixed b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc.fixed
new file mode 100644
index 00000000..80c4f22c
--- /dev/null
+++ b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc.fixed
@@ -0,0 +1,87 @@
+<?php
+/** @var Magento\Framework\View\Element\Template $block */
+/** @var Magento\Framework\Escaper $escaper */
+?>
+
+<section>
+    <h1>This unescaped output is fine here; other sniffs will complain about it though.</h1>
+
+    <?php echo $block->getSomeString(); ?>
+    <?= $block->getSomeString(); ?>
+    <?= /** @noEscape */ $block->getSomeString(); ?>
+    <?= /** @escapeNotVerified */ $block->getSomeString(); ?>
+</section>
+
+<section>
+    <h1>These should be using equivalent methods on the `$escaper` class, not the `$block` class.</h1>
+
+    Note that I couldn't find any use of this method in any templates within Magento.
+    <?= $escaper->escapeCss($block->getSomeString()); ?>
+
+    <?= $escaper->escapeHtml(__($block->getSomeString())) ?>
+    <?= $escaper->escapeHtml(__($block->getSomeString())); ?>
+    <?= $escaper->escapeHtml(__($block->getSomeString()), ['strong', 'em', 'span']) ?>
+
+    <div class="<?= $escaper->escapeHtmlAttr($block->getSomeString()) ?>"></div>
+    <div class="<?= $escaper->escapeHtmlAttr($block->getSomeString(), true) ?>"></div>
+    <div class="<?= $escaper->escapeHtmlAttr($block->getSomeString(), false); ?>"></div>
+
+    <script type="text/x-magento-init">
+    {
+        "#chart_<?= $escaper->escapeJs($block->getData('html_id')) ?>_period": {
+            "Magento_Backend/js/dashboard/chart": {}
+        }
+    }
+    </script>
+
+    The only example of this method being used was in a block class, rather than a template.
+    <?php
+    foreach ($block->getItems() as $item) {
+        $item['sku'] = $escaper->escapeJsQuote($item['sku']);
+    }
+    ?>
+
+    The only example of this method being used was in a block class, rather than a template.
+    <?= $escaper->escapeQuote(__($block->getData('welcome'))); ?>
+
+    <a href="<?= $escaper->escapeUrl($block->getUrl('adminhtml/notification/index')) ?>"> link text </a>
+
+    Note that I couldn't find any use of this method in any templates within Magento.
+    <?= $escaper->escapeXssInUrl($block->getSomeString()); ?>
+</section>
+
+<section>
+    <h1>These are edge cases for formatting differences</h1>
+
+    <?php
+        $escaper->escapeHtml('');
+        $escaper ->escapeHtml('');
+        $escaper-> escapeHtml('');
+        $escaper
+            ->escapeHtml('');
+        $escaper
+
+            ->escapeHtml('');
+        $escaper->
+            escapeHtml('');
+        $escaper-> // comment
+            escapeHtml('');
+        $escaper /* comment */
+            ->escapeHtml('');
+
+        $escaper /* comment */ -> /* comment */ escapeHtml('');
+    ?>
+</section>
+
+<section>
+    <h1>These close-matches shouldn't be flagged by this sniff.</h1>
+
+    <?= $block->escapeHTML(__($block->getSomeString())) ?>
+    <?= $block->escapeHtmlString(__($block->getSomeString())) ?>
+    <?= $block->escapeHtmlAttribute($block->getSomeString()) ?>
+    <?= $block->escapeCSS($block->getSomeString()); ?>
+    <?= $block->escapeJS($block->getData('html_id')) ?>
+    <?= $block->escapeJavaScript($block->getData('html_id')) ?>
+    <?= $block->escapeQuotes(__($block->getData('welcome'))); ?>
+    <?= $block->escapeURL($block->getUrl('adminhtml/notification/index')) ?>
+</section>
diff --git a/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.php b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.php
new file mode 100644
index 00000000..feead3d1
--- /dev/null
+++ b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.php
@@ -0,0 +1,45 @@
+<?php
+
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento2\Tests\Legacy;
+
+use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
+
+class EscapeMethodsOnBlockClassUnitTest extends AbstractSniffUnitTest
+{
+    protected function getErrorList()
+    {
+        return [];
+    }
+
+    protected function getWarningList()
+    {
+        return [
+            19 => 1,
+            21 => 1,
+            22 => 1,
+            23 => 1,
+            25 => 1,
+            26 => 1,
+            27 => 1,
+            31 => 1,
+            40 => 1,
+            45 => 1,
+            47 => 1,
+            50 => 1,
+            57 => 1,
+            58 => 1,
+            59 => 1,
+            61 => 1,
+            64 => 1,
+            66 => 1,
+            68 => 1,
+            70 => 1,
+            72 => 1,
+        ];
+    }
+}

From b8acb6c61a735b9770470ea44cefbebf70139f69 Mon Sep 17 00:00:00 2001
From: Dan Wallis <dan@wallis.nz>
Date: Thu, 6 Jul 2023 15:25:49 +0100
Subject: [PATCH 2/2] Add docblock to appease coding standard

Note that these are useless, as it's the default behaviour to inherit from the
parent docblock.
---
 Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php b/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php
index f44e3d0c..4d6d8288 100644
--- a/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php
+++ b/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php
@@ -26,6 +26,9 @@ class EscapeMethodsOnBlockClassSniff implements Sniff
         'escapeXssInUrl' => true,
     ];
 
+    /**
+     * @inheritDoc
+     */
     public function register()
     {
         return [
@@ -33,6 +36,9 @@ public function register()
         ];
     }
 
+    /**
+     * @inheritDoc
+     */
     public function process(File $phpcsFile, $stackPtr)
     {
         $tokens = $phpcsFile->getTokens();