From 9ea4f7481933bd5c21068d9e8696f9472a05866c Mon Sep 17 00:00:00 2001 From: Raul E Watson Date: Thu, 1 Aug 2019 23:52:37 +0100 Subject: [PATCH 1/5] #121 Magento2.Templates.HelperInTemplate Rule porposal --- .../Sniffs/Templates/HelperInTemplateSniff.md | 25 ++++++++++ .../Templates/HelperInTemplateSniff.php | 48 +++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 Magento2/Sniffs/Templates/HelperInTemplateSniff.md create mode 100644 Magento2/Sniffs/Templates/HelperInTemplateSniff.php diff --git a/Magento2/Sniffs/Templates/HelperInTemplateSniff.md b/Magento2/Sniffs/Templates/HelperInTemplateSniff.md new file mode 100644 index 00000000..a0e11528 --- /dev/null +++ b/Magento2/Sniffs/Templates/HelperInTemplateSniff.md @@ -0,0 +1,25 @@ +# Rule: Do not use `helpers` in templates +## Background +The use of helpers is in general discouraged. Consider using a ViewModel instead. + +## Reasoning +The use of helpers is in general discouraged therefore any `$this->helper()` code used in PHTML templates should be refactored. + +Consider using ViewModel instead. + +## How to fix + +Typical example of a helper being used in a PHTML: +```html +helper()->...; ?> +``` + +Once the ViewModel is created, call it in the PHTML as follow: + +```html +getViewModel(); ?> +``` +or +```html +getData('viewModel'); ?> +``` diff --git a/Magento2/Sniffs/Templates/HelperInTemplateSniff.php b/Magento2/Sniffs/Templates/HelperInTemplateSniff.php new file mode 100644 index 00000000..8a90908a --- /dev/null +++ b/Magento2/Sniffs/Templates/HelperInTemplateSniff.php @@ -0,0 +1,48 @@ +getTokens(); + if ($tokens[$stackPtr]['content'] === 'helper(') { + $phpcsFile->addWarning($this->warningMessage, $stackPtr, $this->warningCode); + } + } +} From 60c7d8739631c2553ac8ba460c599c93139fb2c7 Mon Sep 17 00:00:00 2001 From: Raul E Watson Date: Sat, 10 Aug 2019 10:43:16 +0100 Subject: [PATCH 2/5] #121 HelperInTemplate Rule porposal amends --- .../Sniffs/Templates/HelperInTemplateSniff.md | 25 ---------- .../Templates/HelperInTemplateSniff.php | 48 ------------------- .../Sniffs/Templates/ThisInTemplateSniff.md | 6 ++- .../Sniffs/Templates/ThisInTemplateSniff.php | 24 ++++++++-- 4 files changed, 26 insertions(+), 77 deletions(-) delete mode 100644 Magento2/Sniffs/Templates/HelperInTemplateSniff.md delete mode 100644 Magento2/Sniffs/Templates/HelperInTemplateSniff.php diff --git a/Magento2/Sniffs/Templates/HelperInTemplateSniff.md b/Magento2/Sniffs/Templates/HelperInTemplateSniff.md deleted file mode 100644 index a0e11528..00000000 --- a/Magento2/Sniffs/Templates/HelperInTemplateSniff.md +++ /dev/null @@ -1,25 +0,0 @@ -# Rule: Do not use `helpers` in templates -## Background -The use of helpers is in general discouraged. Consider using a ViewModel instead. - -## Reasoning -The use of helpers is in general discouraged therefore any `$this->helper()` code used in PHTML templates should be refactored. - -Consider using ViewModel instead. - -## How to fix - -Typical example of a helper being used in a PHTML: -```html -helper()->...; ?> -``` - -Once the ViewModel is created, call it in the PHTML as follow: - -```html -getViewModel(); ?> -``` -or -```html -getData('viewModel'); ?> -``` diff --git a/Magento2/Sniffs/Templates/HelperInTemplateSniff.php b/Magento2/Sniffs/Templates/HelperInTemplateSniff.php deleted file mode 100644 index 8a90908a..00000000 --- a/Magento2/Sniffs/Templates/HelperInTemplateSniff.php +++ /dev/null @@ -1,48 +0,0 @@ -getTokens(); - if ($tokens[$stackPtr]['content'] === 'helper(') { - $phpcsFile->addWarning($this->warningMessage, $stackPtr, $this->warningCode); - } - } -} diff --git a/Magento2/Sniffs/Templates/ThisInTemplateSniff.md b/Magento2/Sniffs/Templates/ThisInTemplateSniff.md index 071fada6..4b9284d4 100644 --- a/Magento2/Sniffs/Templates/ThisInTemplateSniff.md +++ b/Magento2/Sniffs/Templates/ThisInTemplateSniff.md @@ -14,6 +14,10 @@ Replace `$this` with `$block`. If you use private or protected methods, make the --- +# Rule: Do not use `helpers` in templates +## Background +The use of helpers is in general discouraged. For template files, consider using a ViewModel instead. + ## Reasoning The use of helpers is in general discouraged therefore any `$this->helper()` code used in PHTML templates should be refactored. @@ -23,7 +27,7 @@ Consider using ViewModel instead. Typical example of a helper being used in a PHTML: ```html -helper()->...; ?> +helper()->...; ?> ``` Once the ViewModel is created, call it in the PHTML as follow: diff --git a/Magento2/Sniffs/Templates/ThisInTemplateSniff.php b/Magento2/Sniffs/Templates/ThisInTemplateSniff.php index be3ec293..2c2a18cb 100644 --- a/Magento2/Sniffs/Templates/ThisInTemplateSniff.php +++ b/Magento2/Sniffs/Templates/ThisInTemplateSniff.php @@ -3,6 +3,7 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento2\Sniffs\Templates; use PHP_CodeSniffer\Sniffs\Sniff; @@ -18,14 +19,28 @@ class ThisInTemplateSniff implements Sniff * * @var string */ - protected $warningMessage = 'Usage of $this in template files is deprecated.'; + protected $warningMessageFoundHelper = 'Usage of helpers in templates is discouraged.'; + + /** + * Warning violation code. + * + * @var string + */ + protected $warningCodeFoundHelper = 'FoundHelper'; + + /** + * String representation of warning. + * + * @var string + */ + protected $warningMessageFoundThis = 'Usage of $this in template files is deprecated.'; /** * Warning violation code. * * @var string */ - protected $warningCode = 'FoundThis'; + protected $warningCodeFoundThis = 'FoundThis'; /** * @inheritdoc @@ -42,7 +57,10 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); if ($tokens[$stackPtr]['content'] === '$this') { - $phpcsFile->addWarning($this->warningMessage, $stackPtr, $this->warningCode); + $phpcsFile->addWarning($this->warningMessageFoundThis, $stackPtr, $this->warningCodeFoundThis); + } + if ($tokens[$stackPtr]['content'] === 'helper(') { + $phpcsFile->addWarning($this->warningMessageFoundHelper, $stackPtr, $this->warningCodeFoundHelper); } } } From a6c81841543867823cfd892113e8c5544fd70846 Mon Sep 17 00:00:00 2001 From: Raul E Watson Date: Tue, 13 Aug 2019 00:04:01 +0100 Subject: [PATCH 3/5] #121 HelperInTemplate Rule porposal amends --- .../Sniffs/Templates/ThisInTemplateSniff.php | 23 +++++++++++-------- .../Templates/ThisInTemplateUnitTest.php | 2 +- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/Magento2/Sniffs/Templates/ThisInTemplateSniff.php b/Magento2/Sniffs/Templates/ThisInTemplateSniff.php index 2c2a18cb..5a1ab69f 100644 --- a/Magento2/Sniffs/Templates/ThisInTemplateSniff.php +++ b/Magento2/Sniffs/Templates/ThisInTemplateSniff.php @@ -14,13 +14,6 @@ */ class ThisInTemplateSniff implements Sniff { - /** - * String representation of warning. - * - * @var string - */ - protected $warningMessageFoundHelper = 'Usage of helpers in templates is discouraged.'; - /** * Warning violation code. * @@ -33,7 +26,7 @@ class ThisInTemplateSniff implements Sniff * * @var string */ - protected $warningMessageFoundThis = 'Usage of $this in template files is deprecated.'; + protected $warningMessageFoundHelper = 'Usage of helpers in templates is discouraged.'; /** * Warning violation code. @@ -42,12 +35,22 @@ class ThisInTemplateSniff implements Sniff */ protected $warningCodeFoundThis = 'FoundThis'; + /** + * String representation of warning. + * + * @var string + */ + protected $warningMessageFoundThis = 'Usage of $this in template files is deprecated.'; + /** * @inheritdoc */ public function register() { - return [T_VARIABLE]; + return [ + T_VARIABLE, + T_STRING, + ]; } /** @@ -59,7 +62,7 @@ public function process(File $phpcsFile, $stackPtr) if ($tokens[$stackPtr]['content'] === '$this') { $phpcsFile->addWarning($this->warningMessageFoundThis, $stackPtr, $this->warningCodeFoundThis); } - if ($tokens[$stackPtr]['content'] === 'helper(') { + if ($tokens[$stackPtr]['content'] === 'helper') { $phpcsFile->addWarning($this->warningMessageFoundHelper, $stackPtr, $this->warningCodeFoundHelper); } } diff --git a/Magento2/Tests/Templates/ThisInTemplateUnitTest.php b/Magento2/Tests/Templates/ThisInTemplateUnitTest.php index 1e47590e..69c515c3 100644 --- a/Magento2/Tests/Templates/ThisInTemplateUnitTest.php +++ b/Magento2/Tests/Templates/ThisInTemplateUnitTest.php @@ -28,7 +28,7 @@ public function getWarningList() return [ 3 => 2, 4 => 1, - 5 => 1, + 5 => 2, ]; } } From abdfcf5a85536e73693caf3c0e2e92f8fd5380ed Mon Sep 17 00:00:00 2001 From: Raul E Watson Date: Thu, 15 Aug 2019 23:00:35 +0100 Subject: [PATCH 4/5] #121 HelperInTemplate amends2 --- Magento2/Sniffs/Templates/ThisInTemplateSniff.php | 10 ++++++---- Magento2/Tests/Templates/ThisInTemplateUnitTest.php | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Magento2/Sniffs/Templates/ThisInTemplateSniff.php b/Magento2/Sniffs/Templates/ThisInTemplateSniff.php index 5a1ab69f..8cc23fad 100644 --- a/Magento2/Sniffs/Templates/ThisInTemplateSniff.php +++ b/Magento2/Sniffs/Templates/ThisInTemplateSniff.php @@ -60,10 +60,12 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); if ($tokens[$stackPtr]['content'] === '$this') { - $phpcsFile->addWarning($this->warningMessageFoundThis, $stackPtr, $this->warningCodeFoundThis); - } - if ($tokens[$stackPtr]['content'] === 'helper') { - $phpcsFile->addWarning($this->warningMessageFoundHelper, $stackPtr, $this->warningCodeFoundHelper); + $position = $phpcsFile->findNext(T_STRING, $stackPtr, null, false, 'helper', true); + if ($position !== false) { + $phpcsFile->addWarning($this->warningMessageFoundHelper, $position, $this->warningCodeFoundHelper); + } else { + $phpcsFile->addWarning($this->warningMessageFoundThis, $stackPtr, $this->warningCodeFoundThis); + } } } } diff --git a/Magento2/Tests/Templates/ThisInTemplateUnitTest.php b/Magento2/Tests/Templates/ThisInTemplateUnitTest.php index 732328ad..590ae194 100644 --- a/Magento2/Tests/Templates/ThisInTemplateUnitTest.php +++ b/Magento2/Tests/Templates/ThisInTemplateUnitTest.php @@ -25,7 +25,7 @@ public function getWarningList() return [ 3 => 2, 4 => 1, - 5 => 2, + 5 => 1, ]; } } From 5182dcadad782774789d5999094712cff7305ee0 Mon Sep 17 00:00:00 2001 From: Lena Orobei Date: Fri, 16 Aug 2019 09:13:40 -0500 Subject: [PATCH 5/5] magento/magento-coding-standard#121: Magento2.Templates.HelperInTemplate Rule --- Magento2/Sniffs/Templates/ThisInTemplateSniff.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Magento2/Sniffs/Templates/ThisInTemplateSniff.php b/Magento2/Sniffs/Templates/ThisInTemplateSniff.php index 8cc23fad..95279236 100644 --- a/Magento2/Sniffs/Templates/ThisInTemplateSniff.php +++ b/Magento2/Sniffs/Templates/ThisInTemplateSniff.php @@ -3,7 +3,6 @@ * Copyright © Magento. All rights reserved. * See COPYING.txt for license details. */ - namespace Magento2\Sniffs\Templates; use PHP_CodeSniffer\Sniffs\Sniff; @@ -26,7 +25,7 @@ class ThisInTemplateSniff implements Sniff * * @var string */ - protected $warningMessageFoundHelper = 'Usage of helpers in templates is discouraged.'; + protected $warningMessageFoundHelper = 'The use of helpers in templates is discouraged. Use ViewModel instead.'; /** * Warning violation code. @@ -40,17 +39,14 @@ class ThisInTemplateSniff implements Sniff * * @var string */ - protected $warningMessageFoundThis = 'Usage of $this in template files is deprecated.'; + protected $warningMessageFoundThis = 'The use of $this in templates is deprecated. Use $block instead.'; /** * @inheritdoc */ public function register() { - return [ - T_VARIABLE, - T_STRING, - ]; + return [T_VARIABLE]; } /**