From 2c198047a3a8e8ce84a86cfa30c0b13ec87fa91f Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Thu, 25 May 2023 14:18:16 +0100 Subject: [PATCH 1/8] Auto-fix ...PhtmlTemplate.TextJavascriptTypeFound --- Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php | 16 +++++--- .../PhtmlTemplateUnitTest.3.phtml.fixed | 40 +++++++++++++++++++ .../PhtmlTemplateUnitTest.1.phtml.fixed | 40 +++++++++++++++++++ .../PhtmlTemplateUnitTest.2.phtml.fixed | 40 +++++++++++++++++++ 4 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed create mode 100644 Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed create mode 100644 Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed diff --git a/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php b/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php index fa5eb70e..05f72235 100644 --- a/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php +++ b/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php @@ -14,7 +14,7 @@ class PhtmlTemplateSniff implements Sniff { private const WARNING_CODE_TEXT_JAVASCRIPT = 'TextJavascriptTypeFound'; private const WARNING_CODE_PROTECTED_PRIVATE_BLOCK_ACCESS = 'ProtectedPrivateBlockAccess'; - + /** * @inheritdoc */ @@ -40,7 +40,7 @@ public function process(File $phpcsFile, $stackPtr) $this->checkHtml($phpcsFile, $stackPtr); } } - + /** * Check access to protected and private members of Block * @@ -74,13 +74,19 @@ private function checkBlockVariable(File $phpcsFile, int $stackPtr, array $token private function checkHtml(File $phpcsFile, int $stackPtr): void { $content = $phpcsFile->getTokensAsString($stackPtr, 1); - - if (preg_match('/type="text\/javascript"/', $content)) { - $phpcsFile->addWarning( + $pattern = '_\s+type="text/javascript"_'; + + if (preg_match($pattern, $content)) { + $fix = $phpcsFile->addFixableWarning( 'Please do not use "text/javascript" type attribute.', $stackPtr, self::WARNING_CODE_TEXT_JAVASCRIPT ); + + if ($fix) { + $content = preg_replace($pattern, '', $content); + $phpcsFile->fixer->replaceToken($stackPtr, $content); + } } } } diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed new file mode 100644 index 00000000..8620e06d --- /dev/null +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed @@ -0,0 +1,40 @@ + + + +
+_getTestFunction(); +$block->getTestFunction(); +$_something = $this->something(); +$block->_getTest(); +$block = _testing(); +?> +_getTestAnotherFunction(); +?> + +helper(); +?> + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed new file mode 100644 index 00000000..8620e06d --- /dev/null +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed @@ -0,0 +1,40 @@ + + + +
+_getTestFunction(); +$block->getTestFunction(); +$_something = $this->something(); +$block->_getTest(); +$block = _testing(); +?> +_getTestAnotherFunction(); +?> + +helper(); +?> + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed new file mode 100644 index 00000000..8620e06d --- /dev/null +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed @@ -0,0 +1,40 @@ + + + +
+_getTestFunction(); +$block->getTestFunction(); +$_something = $this->something(); +$block->_getTest(); +$block = _testing(); +?> +_getTestAnotherFunction(); +?> + +helper(); +?> + From a4863c6ee3980c1bc9ecf1d4907a5e251899ec33 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Mon, 26 Jun 2023 17:38:37 +0100 Subject: [PATCH 2/8] Add tests to demonstrate bugs --- Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php | 8 +++++++- .../adminhtml/templates/PhtmlTemplateUnitTest.3.phtml | 9 +++++++++ .../templates/PhtmlTemplateUnitTest.3.phtml.fixed | 9 +++++++++ .../view/base/templates/PhtmlTemplateUnitTest.1.phtml | 9 +++++++++ .../base/templates/PhtmlTemplateUnitTest.1.phtml.fixed | 9 +++++++++ .../frontend/templates/PhtmlTemplateUnitTest.2.phtml | 9 +++++++++ .../templates/PhtmlTemplateUnitTest.2.phtml.fixed | 9 +++++++++ 7 files changed, 61 insertions(+), 1 deletion(-) diff --git a/Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php b/Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php index 129229a3..2da81f0f 100644 --- a/Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php +++ b/Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php @@ -59,7 +59,10 @@ public function getWarningList($testFile = '') 9 => 1, 20 => 1, 23 => 1, - 27 => 1 + 27 => 1, + 44 => 1, + 45 => 1, + 46 => 1, ]; } if ($testFile === 'PhtmlTemplateUnitTest.3.phtml') { @@ -68,6 +71,9 @@ public function getWarningList($testFile = '') 20 => 1, 23 => 1, 27 => 1, + 44 => 1, + 45 => 1, + 46 => 1, ]; } return []; diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml index 772783c9..aa589039 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml @@ -38,3 +38,12 @@ $this->helper(); ?> + +

This sniff should remove the any type="text/javascript" attributes from <script> tags, but not from elsewhere in the document.

+ + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed index 8620e06d..1911cfbe 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed @@ -38,3 +38,12 @@ $this->helper(); ?> + +

This sniff should remove the any type="text/javascript" attributes from <script> tags, but not from elsewhere in the document.

+ + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml index 772783c9..aa589039 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml @@ -38,3 +38,12 @@ $this->helper(); ?> + +

This sniff should remove the any type="text/javascript" attributes from <script> tags, but not from elsewhere in the document.

+ + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed index 8620e06d..1911cfbe 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed @@ -38,3 +38,12 @@ $this->helper(); ?> + +

This sniff should remove the any type="text/javascript" attributes from <script> tags, but not from elsewhere in the document.

+ + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml index 772783c9..aa589039 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml @@ -38,3 +38,12 @@ $this->helper(); ?> + +

This sniff should remove the any type="text/javascript" attributes from <script> tags, but not from elsewhere in the document.

+ + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed index 8620e06d..1911cfbe 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed @@ -38,3 +38,12 @@ $this->helper(); ?> + +

This sniff should remove the any type="text/javascript" attributes from <script> tags, but not from elsewhere in the document.

+ + + + + From 7ed757175fdee2837d5de11e8e18c917452dd07b Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Mon, 26 Jun 2023 17:39:32 +0100 Subject: [PATCH 3/8] Remove duplication --- .../Tests/Legacy/PhtmlTemplateUnitTest.php | 34 ++++++------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php b/Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php index 2da81f0f..6b258ac1 100644 --- a/Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php +++ b/Magento2/Tests/Legacy/PhtmlTemplateUnitTest.php @@ -37,7 +37,7 @@ protected function getTestFiles($testFileBase): array // Put them in order. sort($testFiles); - + return $testFiles; } @@ -54,28 +54,14 @@ public function getErrorList() */ public function getWarningList($testFile = '') { - if ($testFile === 'PhtmlTemplateUnitTest.1.phtml' || $testFile === 'PhtmlTemplateUnitTest.2.phtml') { - return [ - 9 => 1, - 20 => 1, - 23 => 1, - 27 => 1, - 44 => 1, - 45 => 1, - 46 => 1, - ]; - } - if ($testFile === 'PhtmlTemplateUnitTest.3.phtml') { - return [ - 9 => 1, - 20 => 1, - 23 => 1, - 27 => 1, - 44 => 1, - 45 => 1, - 46 => 1, - ]; - } - return []; + return [ + 9 => 1, + 20 => 1, + 23 => 1, + 27 => 1, + 44 => 1, + 45 => 1, + 46 => 1, + ]; } } From 51b2ac833417228e3e969f8f4e4f1b32574417c0 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Mon, 26 Jun 2023 16:51:00 +0100 Subject: [PATCH 4/8] Catch alternative quote symbols --- Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php b/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php index 05f72235..4395b538 100644 --- a/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php +++ b/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php @@ -74,7 +74,7 @@ private function checkBlockVariable(File $phpcsFile, int $stackPtr, array $token private function checkHtml(File $phpcsFile, int $stackPtr): void { $content = $phpcsFile->getTokensAsString($stackPtr, 1); - $pattern = '_\s+type="text/javascript"_'; + $pattern = '_\s+type=(["\'])text/javascript\1_'; if (preg_match($pattern, $content)) { $fix = $phpcsFile->addFixableWarning( From 168121086a0420b84a75ec16aa510f43163c7673 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Mon, 26 Jun 2023 16:51:32 +0100 Subject: [PATCH 5/8] Ignore case --- Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php b/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php index 4395b538..f020b061 100644 --- a/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php +++ b/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php @@ -74,7 +74,7 @@ private function checkBlockVariable(File $phpcsFile, int $stackPtr, array $token private function checkHtml(File $phpcsFile, int $stackPtr): void { $content = $phpcsFile->getTokensAsString($stackPtr, 1); - $pattern = '_\s+type=(["\'])text/javascript\1_'; + $pattern = '_\s+type=(["\'])text/javascript\1_i'; if (preg_match($pattern, $content)) { $fix = $phpcsFile->addFixableWarning( From 1eebe6c8de7f366d2210dd7d88d2c6a0df39be6f Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Mon, 26 Jun 2023 17:33:17 +0100 Subject: [PATCH 6/8] Only detect and remove + + + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed index 1911cfbe..272b1668 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/adminhtml/templates/PhtmlTemplateUnitTest.3.phtml.fixed @@ -44,6 +44,12 @@ $this->helper(); + + + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml index aa589039..bba83c89 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml @@ -44,6 +44,12 @@ $this->helper(); + + + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed index 1911cfbe..272b1668 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/base/templates/PhtmlTemplateUnitTest.1.phtml.fixed @@ -44,6 +44,12 @@ $this->helper(); + + + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml index aa589039..bba83c89 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml @@ -44,6 +44,12 @@ $this->helper(); + + + + + + diff --git a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed index 1911cfbe..272b1668 100644 --- a/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed +++ b/Magento2/Tests/Legacy/_files/PhtmlTemplateUnitTest/view/frontend/templates/PhtmlTemplateUnitTest.2.phtml.fixed @@ -44,6 +44,12 @@ $this->helper(); + + + + + + From 485386912268141359eeefeffdd98f6489429ac3 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Mon, 26 Jun 2023 17:56:43 +0100 Subject: [PATCH 8/8] Fix regular expression --- Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php b/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php index bc44466e..1f897d38 100644 --- a/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php +++ b/Magento2/Sniffs/Legacy/PhtmlTemplateSniff.php @@ -74,7 +74,7 @@ private function checkBlockVariable(File $phpcsFile, int $stackPtr, array $token private function checkHtml(File $phpcsFile, int $stackPtr): void { $content = $phpcsFile->getTokensAsString($stackPtr, 1); - $pattern = '_(\s]*)\stype=(["\'])text/javascript\2_i'; + $pattern = '_(]*?)\s+type=(["\'])text/javascript\2_i'; if (preg_match($pattern, $content, $matches)) { $fix = $phpcsFile->addFixableWarning(