Skip to content

Commit 478360d

Browse files
author
Oleksandr Gorkun
committed
MC-17489: Require specific suffix for HTML binding
1 parent 91e9a51 commit 478360d

File tree

5 files changed

+114
-3
lines changed

5 files changed

+114
-3
lines changed

dev/tests/static/framework/Magento/Sniffs/Html/HtmlBindingSniff.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
use PHP_CodeSniffer\Sniffs\Sniff;
1010
use PHP_CodeSniffer\Files\File;
1111

12+
/**
13+
* Sniffing improper HTML bindings.
14+
*/
1215
class HtmlBindingSniff implements Sniff
1316
{
1417
/**
@@ -30,6 +33,7 @@ public function process(File $phpcsFile, $stackPtr)
3033
$html = $phpcsFile->getTokensAsString($stackPtr, count($phpcsFile->getTokens()));
3134
$dom = new \DOMDocument();
3235
try {
36+
//phpcs: ignore
3337
@$dom->loadHTML($html);
3438
$loaded = true;
3539
} catch (\Throwable $exception) {
@@ -41,13 +45,14 @@ public function process(File $phpcsFile, $stackPtr)
4145
$dataBindAttributes = $domXpath->query('//@*[name() = "data-bind"]');
4246
foreach ($dataBindAttributes as $dataBindAttribute) {
4347
$knockoutBinding = $dataBindAttribute->nodeValue;
44-
preg_match('/^(.+\s+)*?html\:\s*?([a-z0-9\.\_\(\)]+)/i', $knockoutBinding, $htmlBinding);
48+
preg_match('/^(.+\s*?)?html\s*?\:\s*?([a-z0-9\.\(\)\_]+)/ims', $knockoutBinding, $htmlBinding);
4549
if ($htmlBinding && !preg_match('/UnsanitizedHtml[\(\)]*?$/', $htmlBinding[2])) {
4650
$phpcsFile->addError(
47-
'Variables/functions used for HTML binding must have UnsanitizedHtml suffix, '
51+
'Variables/functions used for HTML binding must have UnsanitizedHtml suffix'
52+
.' - "' .$htmlBinding[2] .'" doesn\'t,' .PHP_EOL
4853
.'consider using text binding if the value is supposed to be text',
4954
null,
50-
'UIComponentTemplate.HtmlSuffix'
55+
'UIComponentTemplate.KnockoutBinding.HtmlSuffix'
5156
);
5257
}
5358
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\Sniffs\Less;
10+
11+
use Magento\TestFramework\CodingStandard\Tool\CodeSniffer\HtmlWrapper;
12+
use PHPUnit\Framework\TestCase;
13+
use Magento\TestFramework\CodingStandard\Tool\CodeSniffer;
14+
15+
/**
16+
* Test the html binding sniff on real files.
17+
*/
18+
class HtmlBindingSniffTest extends TestCase
19+
{
20+
/**
21+
* Files to sniff and expected reports.
22+
*
23+
* @return array
24+
*/
25+
public function processDataProvider(): array
26+
{
27+
return [
28+
[
29+
'test-html-binding.html',
30+
'test-html-binding-errors.txt'
31+
]
32+
];
33+
}
34+
35+
/**
36+
* Run CS on provided files.
37+
*
38+
* @param string $fileUnderTest
39+
* @param string $expectedReportFile
40+
* @return void
41+
* @dataProvider processDataProvider
42+
*/
43+
public function testProcess(string $fileUnderTest, string $expectedReportFile): void
44+
{
45+
$reportFile = __DIR__ . DIRECTORY_SEPARATOR . '_files' . DIRECTORY_SEPARATOR . 'phpcs_report.txt';
46+
$ruleSetDir = __DIR__ . DIRECTORY_SEPARATOR . '_files';
47+
$wrapper = new HtmlWrapper();
48+
$codeSniffer = new CodeSniffer($ruleSetDir, $reportFile, $wrapper);
49+
$codeSniffer->setExtensions([HtmlWrapper::FILE_EXTENSION]);
50+
$result = $codeSniffer->run(
51+
[__DIR__ . DIRECTORY_SEPARATOR . '_files' . DIRECTORY_SEPARATOR . $fileUnderTest]
52+
);
53+
// Remove the absolute path to the file from the output
54+
$actual = preg_replace('/^.+\n/', '', ltrim(file_get_contents($reportFile)));
55+
$expected = file_get_contents(
56+
__DIR__ . DIRECTORY_SEPARATOR . '_files' . DIRECTORY_SEPARATOR . $expectedReportFile
57+
);
58+
unlink($reportFile);
59+
$this->assertEquals(1, $result);
60+
$this->assertEquals($expected, $actual);
61+
}
62+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<ruleset name="Magento">
9+
<description>UI Component Coding Standard</description>
10+
<rule ref="Internal.NoCodeFound">
11+
<severity>0</severity>
12+
</rule>
13+
<rule ref="../../../../../../../Magento/Sniffs/Html"/>
14+
</ruleset>
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
2+
FOUND 3 ERRORS AFFECTING 1 LINE
3+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
4+
1 | ERROR | Variables/functions used for HTML binding must have UnsanitizedHtml suffix - "testError()" doesn't,
5+
| | consider using text binding if the value is supposed to be text
6+
1 | ERROR | Variables/functions used for HTML binding must have UnsanitizedHtml suffix - "test.getSomething().value.error()" doesn't,
7+
| | consider using text binding if the value is supposed to be text
8+
1 | ERROR | Variables/functions used for HTML binding must have UnsanitizedHtml suffix - "bind_stuff()" doesn't,
9+
| | consider using text binding if the value is supposed to be text
10+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
11+
12+
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<!--
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
-->
7+
8+
<div data-bind="attr: test.value"></div>
9+
<p>Test</p>
10+
<span data-bind="html: testError()"></span>
11+
<div data-bind="
12+
attr : tst,
13+
html: test.getSomething().value.error()
14+
"></div>
15+
<p data-bind="html: '<b>Some html</b>'"></p>
16+
<div data-bind="html: valueUnsanitizedHtml"></div>
17+
<div data-bind="attr: testhtml, html: valueUnsanitizedHtml()"></div>
18+
<p data-bind="other_html: bind, html: bind_stuff()"></p>

0 commit comments

Comments
 (0)