From f0b61be7a03a002ac5dcf9ce8027fb64c1ed5ded Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Wed, 25 Aug 2021 13:03:06 +0200 Subject: [PATCH 1/9] AC-660: Create phpcs static check for DiConfigTest - DiConfigTest: Test for obsolete nodes in di.xml --- Magento2/Sniffs/Legacy/DiConfigSniff.php | 80 ++++++++++++++++++++++ Magento2/Tests/Legacy/DiConfigUnitTest.php | 34 +++++++++ Magento2/Tests/Legacy/DiConfigUnitTest.xml | 19 +++++ Magento2/ruleset.xml | 5 ++ composer.json | 4 +- 5 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 Magento2/Sniffs/Legacy/DiConfigSniff.php create mode 100644 Magento2/Tests/Legacy/DiConfigUnitTest.php create mode 100644 Magento2/Tests/Legacy/DiConfigUnitTest.xml diff --git a/Magento2/Sniffs/Legacy/DiConfigSniff.php b/Magento2/Sniffs/Legacy/DiConfigSniff.php new file mode 100644 index 00000000..680eb318 --- /dev/null +++ b/Magento2/Sniffs/Legacy/DiConfigSniff.php @@ -0,0 +1,80 @@ + 'The node is obsolete. Instead, use the ', + 'instance' => 'The node is obsolete. Instead, use the ', + 'array' => 'The node is obsolete. Instead, use the ', + 'item[@key]' => 'The node is obsolete. Instead, use the ', + 'value' => 'The node is obsolete. Instead, provide the actual value as a text literal.' + ]; + + public function register(): array + { + return [ + T_INLINE_HTML + ]; + } + + public function process(File $phpcsFile, $stackPtr): int + { + $xml = simplexml_load_string($this->getFormattedXML($phpcsFile)); + if ($xml === false) { + $phpcsFile->addError( + sprintf( + "Couldn't parse contents of '%s', check that they are in valid XML format", + $phpcsFile->getFilename(), + ), + $stackPtr, + self::ERROR_CODE + ); + } + + foreach ($this->xpathObsoleteElems as $obsoleteElem) { + $found = $xml->xpath($obsoleteElem); + if ($found === true) { + $phpcsFile->addWarning( + $this->messages[$obsoleteElem], + $stackPtr, + self::WARNING_CODE + ); + } + } + } + + /** + * Format the incoming XML to avoid tags split into several lines. + * + * @param File $phpcsFile + * @return false|string + */ + private function getFormattedXML(File $phpcsFile) + { + $doc = new DomDocument('1.0'); + $doc->formatOutput = true; + $doc->loadXML($phpcsFile->getTokensAsString(0, 999999)); + return $doc->saveXML(); + } +} \ No newline at end of file diff --git a/Magento2/Tests/Legacy/DiConfigUnitTest.php b/Magento2/Tests/Legacy/DiConfigUnitTest.php new file mode 100644 index 00000000..8df8fa91 --- /dev/null +++ b/Magento2/Tests/Legacy/DiConfigUnitTest.php @@ -0,0 +1,34 @@ + 1, + 10 => 1, + 11 => 1, + 12 => 1, + 13 => 1, + 15 => 1 + ]; + } +} \ No newline at end of file diff --git a/Magento2/Tests/Legacy/DiConfigUnitTest.xml b/Magento2/Tests/Legacy/DiConfigUnitTest.xml new file mode 100644 index 00000000..83b29690 --- /dev/null +++ b/Magento2/Tests/Legacy/DiConfigUnitTest.xml @@ -0,0 +1,19 @@ + + + + + + + + scalar20 + + 50.00 + + + + \ No newline at end of file diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 3ea0c969..0a0621f9 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -223,6 +223,11 @@ 8 warning + + 8 + warning + *\/DiConfigUnitTest.xml$ + diff --git a/composer.json b/composer.json index d6004a88..f5037b17 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,9 @@ "require": { "php": ">=7.3", "squizlabs/php_codesniffer": "^3.6", - "webonyx/graphql-php": "^14.9" + "webonyx/graphql-php": "^14.9", + "ext-simplexml": "*", + "ext-dom": "*" }, "require-dev": { "phpunit/phpunit": "^9.5.8" From 7be675fbbf7c0126fac003faa4a7dcadf5be965d Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Wed, 25 Aug 2021 14:49:10 +0200 Subject: [PATCH 2/9] AC-660: Create phpcs static check for DiConfigTest - Add missing composer.lock --- composer.lock | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/composer.lock b/composer.lock index 72a2ad6a..04231d90 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "1ed3638000f0e172d4e01c45333fcfb6", + "content-hash": "8c3c7509df274fdeaf09edfd7eac6cf5", "packages": [ { "name": "squizlabs/php_codesniffer", @@ -2228,7 +2228,9 @@ "prefer-stable": false, "prefer-lowest": false, "platform": { - "php": ">=7.3" + "php": ">=7.3", + "ext-simplexml": "*", + "ext-dom": "*" }, "platform-dev": [], "plugin-api-version": "2.1.0" From d58d4b5d0dc9724acd5dfdd000d7b6974f9295ad Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Wed, 25 Aug 2021 15:01:40 +0200 Subject: [PATCH 3/9] Update Magento2/ruleset.xml Co-authored-by: Sergii Ivashchenko --- Magento2/ruleset.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 74d59535..04731e74 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -226,7 +226,7 @@ 8 warning - *\/DiConfigUnitTest.xml$ + *\/di.xml$ From d0039ffc610630ce5bfc46e2c1aecc882f11fcc3 Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Wed, 25 Aug 2021 15:05:04 +0200 Subject: [PATCH 4/9] AC-660: Create phpcs static check for DiConfigTest - Run composer update --lock --- composer.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.lock b/composer.lock index 04231d90..fa12108d 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "8c3c7509df274fdeaf09edfd7eac6cf5", + "content-hash": "503b3dff10c4885d5f5b196d159c1fdd", "packages": [ { "name": "squizlabs/php_codesniffer", From d8d8cb78a516a9267ede95096d5134e22e39552c Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Wed, 25 Aug 2021 15:40:10 +0200 Subject: [PATCH 5/9] AC-660: Create phpcs static check for DiConfigTest - Fix return type --- Magento2/Sniffs/Legacy/DiConfigSniff.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Magento2/Sniffs/Legacy/DiConfigSniff.php b/Magento2/Sniffs/Legacy/DiConfigSniff.php index 680eb318..164036f7 100644 --- a/Magento2/Sniffs/Legacy/DiConfigSniff.php +++ b/Magento2/Sniffs/Legacy/DiConfigSniff.php @@ -38,7 +38,7 @@ public function register(): array ]; } - public function process(File $phpcsFile, $stackPtr): int + public function process(File $phpcsFile, $stackPtr) { $xml = simplexml_load_string($this->getFormattedXML($phpcsFile)); if ($xml === false) { From 5e4b15b4f61331ea8f877a5fc412ac52b0086deb Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Wed, 25 Aug 2021 18:08:42 +0200 Subject: [PATCH 6/9] AC-660: Create phpcs static check for DiConfigTest - Update XML files --- Magento2/Tests/Legacy/DiConfigUnitTest.xml | 24 +++++++++++++--------- Magento2/ruleset.xml | 6 ++++-- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/Magento2/Tests/Legacy/DiConfigUnitTest.xml b/Magento2/Tests/Legacy/DiConfigUnitTest.xml index 83b29690..a1328a22 100644 --- a/Magento2/Tests/Legacy/DiConfigUnitTest.xml +++ b/Magento2/Tests/Legacy/DiConfigUnitTest.xml @@ -5,15 +5,19 @@ * See COPYING.txt for license details. */ --> - + + + + + Magento\Catalog\Pricing\Price\RegularPrice + + + - - - - scalar20 - - 50.00 - - + + + scalar5 + + - \ No newline at end of file + \ No newline at end of file diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 04731e74..7d2c9fc1 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -3,7 +3,7 @@ Magento Coding Standard - + @@ -224,9 +224,10 @@ warning + *\/di.xml$ + *\/DiConfigUnitTest.xml$ 8 warning - *\/di.xml$ @@ -263,6 +264,7 @@ warning + *\.xml$ 7 warning From d08c40f4cfcfc61db58f55b3791b3b3666b7b0c4 Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Fri, 27 Aug 2021 16:26:24 +0200 Subject: [PATCH 7/9] AC-660: Create phpcs static check for DiConfigTest - Refactor: Simplify approach --- Magento2/Sniffs/Legacy/DiConfigSniff.php | 55 ++++------------------ Magento2/Tests/Legacy/DiConfigUnitTest.php | 11 ++--- Magento2/Tests/Legacy/DiConfigUnitTest.xml | 2 +- Magento2/ruleset.xml | 1 - composer.json | 4 +- composer.lock | 6 +-- 6 files changed, 19 insertions(+), 60 deletions(-) diff --git a/Magento2/Sniffs/Legacy/DiConfigSniff.php b/Magento2/Sniffs/Legacy/DiConfigSniff.php index 164036f7..98f072f0 100644 --- a/Magento2/Sniffs/Legacy/DiConfigSniff.php +++ b/Magento2/Sniffs/Legacy/DiConfigSniff.php @@ -6,29 +6,19 @@ namespace Magento2\Sniffs\Legacy; -use DOMDocument; use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; class DiConfigSniff implements Sniff { private const WARNING_CODE = 'FoundObsoleteAttribute'; - private const ERROR_CODE = 'WrongXML'; private $xpathObsoleteElems = [ - 'param', - 'instance', - 'array', - 'item[@key]', - 'value' - ]; - - private $messages = [ - 'param' => 'The node is obsolete. Instead, use the ', - 'instance' => 'The node is obsolete. Instead, use the ', - 'array' => 'The node is obsolete. Instead, use the ', - 'item[@key]' => 'The node is obsolete. Instead, use the ', - 'value' => 'The node is obsolete. Instead, provide the actual value as a text literal.' + ' 'The node is obsolete. Instead, use the ', + ' 'The node is obsolete. Instead, use the ', + ' 'The node is obsolete. Instead, use the ', + ' node is obsolete. Instead, use the ', + ' 'The node is obsolete. Instead, provide the actual value as a text literal.' ]; public function register(): array @@ -40,41 +30,16 @@ public function register(): array public function process(File $phpcsFile, $stackPtr) { - $xml = simplexml_load_string($this->getFormattedXML($phpcsFile)); - if ($xml === false) { - $phpcsFile->addError( - sprintf( - "Couldn't parse contents of '%s', check that they are in valid XML format", - $phpcsFile->getFilename(), - ), - $stackPtr, - self::ERROR_CODE - ); - } + $lineContent = $phpcsFile->getTokensAsString($stackPtr, 1); - foreach ($this->xpathObsoleteElems as $obsoleteElem) { - $found = $xml->xpath($obsoleteElem); - if ($found === true) { + foreach ($this->xpathObsoleteElems as $elem => $message ) { + if (strpos($lineContent, $elem) !== false) { $phpcsFile->addWarning( - $this->messages[$obsoleteElem], + $message, $stackPtr, self::WARNING_CODE ); } } } - - /** - * Format the incoming XML to avoid tags split into several lines. - * - * @param File $phpcsFile - * @return false|string - */ - private function getFormattedXML(File $phpcsFile) - { - $doc = new DomDocument('1.0'); - $doc->formatOutput = true; - $doc->loadXML($phpcsFile->getTokensAsString(0, 999999)); - return $doc->saveXML(); - } -} \ No newline at end of file +} diff --git a/Magento2/Tests/Legacy/DiConfigUnitTest.php b/Magento2/Tests/Legacy/DiConfigUnitTest.php index 8df8fa91..d5dad883 100644 --- a/Magento2/Tests/Legacy/DiConfigUnitTest.php +++ b/Magento2/Tests/Legacy/DiConfigUnitTest.php @@ -23,12 +23,11 @@ public function getErrorList(): array public function getWarningList(): array { return [ - 9 => 1, - 10 => 1, - 11 => 1, 12 => 1, - 13 => 1, - 15 => 1 + 16 => 1, + 17 => 1, + 18 => 1, + 19 => 1 ]; } -} \ No newline at end of file +} diff --git a/Magento2/Tests/Legacy/DiConfigUnitTest.xml b/Magento2/Tests/Legacy/DiConfigUnitTest.xml index a1328a22..048e31b4 100644 --- a/Magento2/Tests/Legacy/DiConfigUnitTest.xml +++ b/Magento2/Tests/Legacy/DiConfigUnitTest.xml @@ -20,4 +20,4 @@ - \ No newline at end of file + diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 7d2c9fc1..7b1e78f7 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -225,7 +225,6 @@ *\/di.xml$ - *\/DiConfigUnitTest.xml$ 8 warning diff --git a/composer.json b/composer.json index 267fcaa3..f30fd7fd 100644 --- a/composer.json +++ b/composer.json @@ -10,9 +10,7 @@ "require": { "php": ">=7.3", "squizlabs/php_codesniffer": "^3.6", - "webonyx/graphql-php": "^14.9", - "ext-simplexml": "*", - "ext-dom": "*" + "webonyx/graphql-php": "^14.9" }, "require-dev": { "phpunit/phpunit": "^9.5.8" diff --git a/composer.lock b/composer.lock index fa12108d..c26f9e74 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "503b3dff10c4885d5f5b196d159c1fdd", + "content-hash": "93bec1b3a36cf67f2511aec0219bcc06", "packages": [ { "name": "squizlabs/php_codesniffer", @@ -2228,9 +2228,7 @@ "prefer-stable": false, "prefer-lowest": false, "platform": { - "php": ">=7.3", - "ext-simplexml": "*", - "ext-dom": "*" + "php": ">=7.3" }, "platform-dev": [], "plugin-api-version": "2.1.0" From 6031c1e7e9e4fa570e125290cbef22bae4c4487d Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Fri, 27 Aug 2021 16:29:48 +0200 Subject: [PATCH 8/9] AC-660: Create phpcs static check for DiConfigTest - Fix phpcs --- Magento2/Sniffs/Legacy/DiConfigSniff.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Magento2/Sniffs/Legacy/DiConfigSniff.php b/Magento2/Sniffs/Legacy/DiConfigSniff.php index 98f072f0..41dea6e7 100644 --- a/Magento2/Sniffs/Legacy/DiConfigSniff.php +++ b/Magento2/Sniffs/Legacy/DiConfigSniff.php @@ -21,6 +21,9 @@ class DiConfigSniff implements Sniff ' 'The node is obsolete. Instead, provide the actual value as a text literal.' ]; + /** + * @inheritDoc + */ public function register(): array { return [ @@ -28,11 +31,14 @@ public function register(): array ]; } + /** + * @inheritDoc + */ public function process(File $phpcsFile, $stackPtr) { $lineContent = $phpcsFile->getTokensAsString($stackPtr, 1); - foreach ($this->xpathObsoleteElems as $elem => $message ) { + foreach ($this->xpathObsoleteElems as $elem => $message) { if (strpos($lineContent, $elem) !== false) { $phpcsFile->addWarning( $message, From 8c600476101fb590a797b697810b4d7111d58aed Mon Sep 17 00:00:00 2001 From: Marc Ginesta Date: Sat, 28 Aug 2021 10:42:27 +0200 Subject: [PATCH 9/9] AC-660: Create phpcs static check for DiConfigTest - Refactor: Rename variables and add phpdoc --- Magento2/Sniffs/Legacy/DiConfigSniff.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Magento2/Sniffs/Legacy/DiConfigSniff.php b/Magento2/Sniffs/Legacy/DiConfigSniff.php index 41dea6e7..954c32e0 100644 --- a/Magento2/Sniffs/Legacy/DiConfigSniff.php +++ b/Magento2/Sniffs/Legacy/DiConfigSniff.php @@ -13,7 +13,10 @@ class DiConfigSniff implements Sniff { private const WARNING_CODE = 'FoundObsoleteAttribute'; - private $xpathObsoleteElems = [ + /** + * @var string[] Associative array containing the obsolete nodes and the message to display when they are found. + */ + private $obsoleteDiNodes = [ ' 'The node is obsolete. Instead, use the ', ' 'The node is obsolete. Instead, use the ', ' 'The node is obsolete. Instead, use the ', @@ -38,8 +41,8 @@ public function process(File $phpcsFile, $stackPtr) { $lineContent = $phpcsFile->getTokensAsString($stackPtr, 1); - foreach ($this->xpathObsoleteElems as $elem => $message) { - if (strpos($lineContent, $elem) !== false) { + foreach ($this->obsoleteDiNodes as $element => $message) { + if (strpos($lineContent, $element) !== false) { $phpcsFile->addWarning( $message, $stackPtr,