Skip to content

Commit

Permalink
Merge pull request #242 from magento-commerce/imported-fredden-magent…
Browse files Browse the repository at this point in the history
…o-coding-standard-458

[Imported] Add sniff for deprecated use of $block->escape... methods
  • Loading branch information
sidolov authored Sep 20, 2023
2 parents 6197724 + b8acb6c commit 153e14a
Show file tree
Hide file tree
Showing 4 changed files with 325 additions and 0 deletions.
106 changes: 106 additions & 0 deletions Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
<?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,
];

/**
* @inheritDoc
*/
public function register()
{
return [
T_OBJECT_OPERATOR,
];
}

/**
* @inheritDoc
*/
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');
}
}
}
87 changes: 87 additions & 0 deletions Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc
Original file line number Diff line number Diff line change
@@ -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>
87 changes: 87 additions & 0 deletions Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc.fixed
Original file line number Diff line number Diff line change
@@ -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>
45 changes: 45 additions & 0 deletions Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.php
Original file line number Diff line number Diff line change
@@ -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,
];
}
}

0 comments on commit 153e14a

Please sign in to comment.