diff --git a/Magento2/Sniffs/Commenting/ClassAndInterfacePHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassAndInterfacePHPDocFormattingSniff.php index 20c12851..b23b2858 100644 --- a/Magento2/Sniffs/Commenting/ClassAndInterfacePHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassAndInterfacePHPDocFormattingSniff.php @@ -63,8 +63,10 @@ public function process(File $phpcsFile, $stackPtr) return; } + $commentCloserPtr = $tokens[$commentStartPtr]['comment_closer']; + if ($this->PHPDocFormattingValidator->providesMeaning($namePtr, $commentStartPtr, $tokens) !== true) { - $phpcsFile->addWarning( + $fix = $phpcsFile->addFixableWarning( sprintf( '%s description must contain meaningful information beyond what its name provides or be removed.', ucfirst($tokens[$stackPtr]['content']) @@ -72,6 +74,18 @@ public function process(File $phpcsFile, $stackPtr) $stackPtr, 'InvalidDescription' ); + + if ($fix) { + for ($i = $commentStartPtr; $i <= $commentCloserPtr; $i++) { + $phpcsFile->fixer->replaceToken($i, ''); + } + + if ($tokens[$commentStartPtr - 1]['code'] === T_WHITESPACE + && $tokens[$commentCloserPtr + 1]['code'] === T_WHITESPACE + ) { + $phpcsFile->fixer->replaceToken($commentCloserPtr + 1, ''); + } + } } if ($this->PHPDocFormattingValidator->hasDeprecatedWellFormatted($commentStartPtr, $tokens) !== true) { @@ -105,11 +119,35 @@ private function validateTags(File $phpcsFile, $commentStartPtr, $tokens) } if (in_array($tokens[$i]['content'], $this->forbiddenTags) === true) { - $phpcsFile->addWarning( + $fix = $phpcsFile->addFixableWarning( sprintf('Tag %s MUST NOT be used.', $tokens[$i]['content']), $i, 'ForbiddenTags' ); + + if ($fix) { + for ($j = $i - 1; $j > $commentStartPtr; $j--) { + if (!in_array($tokens[$j]['code'], [T_DOC_COMMENT_STAR, T_DOC_COMMENT_WHITESPACE], true)) { + break; + } + + if ($tokens[$j]['code'] === T_DOC_COMMENT_WHITESPACE && $tokens[$j]['content'] === "\n") { + break; + } + + $phpcsFile->fixer->replaceToken($j, ''); + } + + $phpcsFile->fixer->replaceToken($i, ''); + + for ($j = $i + 1; $j < $commentCloserPtr; $j++) { + $phpcsFile->fixer->replaceToken($j, ''); + + if ($tokens[$j]['code'] === T_DOC_COMMENT_WHITESPACE && $tokens[$j]['content'] === "\n") { + break; + } + } + } } } diff --git a/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php b/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php new file mode 100644 index 00000000..4d6d8288 --- /dev/null +++ b/Magento2/Sniffs/Legacy/EscapeMethodsOnBlockClassSniff.php @@ -0,0 +1,106 @@ + 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'); + } + } +} diff --git a/Magento2/Sniffs/Security/XssTemplateSniff.php b/Magento2/Sniffs/Security/XssTemplateSniff.php index 2ba8e8c0..ba200daf 100644 --- a/Magento2/Sniffs/Security/XssTemplateSniff.php +++ b/Magento2/Sniffs/Security/XssTemplateSniff.php @@ -147,11 +147,11 @@ public function process(File $phpcsFile, $stackPtr) private function findSpecialAnnotation($stackPtr) { if ($this->tokens[$stackPtr]['code'] === T_ECHO) { - $startOfStatement = $this->file->findPrevious(T_OPEN_TAG, $stackPtr); + $startOfStatement = $this->file->findPrevious([T_OPEN_TAG, T_SEMICOLON], $stackPtr); return $this->file->findPrevious(T_COMMENT, $stackPtr, $startOfStatement); } if ($this->tokens[$stackPtr]['code'] === T_OPEN_TAG_WITH_ECHO) { - $endOfStatement = $this->file->findNext(T_CLOSE_TAG, $stackPtr); + $endOfStatement = $this->file->findNext([T_CLOSE_TAG, T_SEMICOLON], $stackPtr); return $this->file->findNext(T_COMMENT, $stackPtr, $endOfStatement); } return false; diff --git a/Magento2/Tests/Commenting/ClassAndInterfacePHPDocFormattingUnitTest.1.inc b/Magento2/Tests/Commenting/ClassAndInterfacePHPDocFormattingUnitTest.1.inc index 7644f436..658c88a0 100644 --- a/Magento2/Tests/Commenting/ClassAndInterfacePHPDocFormattingUnitTest.1.inc +++ b/Magento2/Tests/Commenting/ClassAndInterfacePHPDocFormattingUnitTest.1.inc @@ -194,3 +194,11 @@ class AlsoDeprecatedButHandlerLongVersion { } + +/** + * @package this tag should not be used + */ +class OnlyUselessCommentContent +{ + +} diff --git a/Magento2/Tests/Commenting/ClassAndInterfacePHPDocFormattingUnitTest.1.inc.fixed b/Magento2/Tests/Commenting/ClassAndInterfacePHPDocFormattingUnitTest.1.inc.fixed new file mode 100644 index 00000000..3467adfc --- /dev/null +++ b/Magento2/Tests/Commenting/ClassAndInterfacePHPDocFormattingUnitTest.1.inc.fixed @@ -0,0 +1,181 @@ + 1, 109 => 1, 118 => 1, - 127 => 1 + 127 => 1, + 199 => 1, ]; } } diff --git a/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc new file mode 100644 index 00000000..260f0c93 --- /dev/null +++ b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc @@ -0,0 +1,87 @@ + + +
+

This unescaped output is fine here; other sniffs will complain about it though.

+ + getSomeString(); ?> + getSomeString(); ?> + getSomeString(); ?> + getSomeString(); ?> +
+ +
+

These should be using equivalent methods on the `$escaper` class, not the `$block` class.

+ + Note that I couldn't find any use of this method in any templates within Magento. + escapeCss($block->getSomeString()); ?> + + escapeHtml(__($block->getSomeString())) ?> + escapeHtml(__($block->getSomeString())); ?> + escapeHtml(__($block->getSomeString()), ['strong', 'em', 'span']) ?> + +
+
+
+ + + + The only example of this method being used was in a block class, rather than a template. + 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. + escapeQuote(__($block->getData('welcome'))); ?> + + link text + + Note that I couldn't find any use of this method in any templates within Magento. + escapeXssInUrl($block->getSomeString()); ?> +
+ +
+

These are edge cases for formatting differences

+ + escapeHtml(''); + $block ->escapeHtml(''); + $block-> escapeHtml(''); + $block + ->escapeHtml(''); + $block + + ->escapeHtml(''); + $block-> + escapeHtml(''); + $block-> // comment + escapeHtml(''); + $block /* comment */ + ->escapeHtml(''); + + $block /* comment */ -> /* comment */ escapeHtml(''); + ?> +
+ +
+

These close-matches shouldn't be flagged by this sniff.

+ + escapeHTML(__($block->getSomeString())) ?> + escapeHtmlString(__($block->getSomeString())) ?> + escapeHtmlAttribute($block->getSomeString()) ?> + escapeCSS($block->getSomeString()); ?> + escapeJS($block->getData('html_id')) ?> + escapeJavaScript($block->getData('html_id')) ?> + escapeQuotes(__($block->getData('welcome'))); ?> + escapeURL($block->getUrl('adminhtml/notification/index')) ?> +
diff --git a/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc.fixed b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc.fixed new file mode 100644 index 00000000..80c4f22c --- /dev/null +++ b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.inc.fixed @@ -0,0 +1,87 @@ + + +
+

This unescaped output is fine here; other sniffs will complain about it though.

+ + getSomeString(); ?> + getSomeString(); ?> + getSomeString(); ?> + getSomeString(); ?> +
+ +
+

These should be using equivalent methods on the `$escaper` class, not the `$block` class.

+ + Note that I couldn't find any use of this method in any templates within Magento. + escapeCss($block->getSomeString()); ?> + + escapeHtml(__($block->getSomeString())) ?> + escapeHtml(__($block->getSomeString())); ?> + escapeHtml(__($block->getSomeString()), ['strong', 'em', 'span']) ?> + +
+
+
+ + + + The only example of this method being used was in a block class, rather than a template. + 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. + escapeQuote(__($block->getData('welcome'))); ?> + + link text + + Note that I couldn't find any use of this method in any templates within Magento. + escapeXssInUrl($block->getSomeString()); ?> +
+ +
+

These are edge cases for formatting differences

+ + escapeHtml(''); + $escaper ->escapeHtml(''); + $escaper-> escapeHtml(''); + $escaper + ->escapeHtml(''); + $escaper + + ->escapeHtml(''); + $escaper-> + escapeHtml(''); + $escaper-> // comment + escapeHtml(''); + $escaper /* comment */ + ->escapeHtml(''); + + $escaper /* comment */ -> /* comment */ escapeHtml(''); + ?> +
+ +
+

These close-matches shouldn't be flagged by this sniff.

+ + escapeHTML(__($block->getSomeString())) ?> + escapeHtmlString(__($block->getSomeString())) ?> + escapeHtmlAttribute($block->getSomeString()) ?> + escapeCSS($block->getSomeString()); ?> + escapeJS($block->getData('html_id')) ?> + escapeJavaScript($block->getData('html_id')) ?> + escapeQuotes(__($block->getData('welcome'))); ?> + escapeURL($block->getUrl('adminhtml/notification/index')) ?> +
diff --git a/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.php b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.php new file mode 100644 index 00000000..feead3d1 --- /dev/null +++ b/Magento2/Tests/Legacy/EscapeMethodsOnBlockClassUnitTest.php @@ -0,0 +1,45 @@ + 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, + ]; + } +} diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index fc5dadde..e7ce2ee8 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -94,6 +94,10 @@ 10 error + + 10 + error + 10 error diff --git a/README.md b/README.md index 3cfd6c0a..8ca973e1 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ npm run eslint -- path/to/analyze ``` ### RECTOR PHP -From `magento-condign-standard` project, you can execute rector php as follows: +From `magento-coding-standard` project, you can execute rector php as follows: ```bash vendor/bin/rector process Magento2 Magento2Framework PHP_CodeSniffer --dry-run --autoload-file vendor/squizlabs/php_codesniffer/autoload.php ``` diff --git a/eslint/.eslintrc-magento b/eslint/.eslintrc-magento index 5d403da3..69b47e83 100644 --- a/eslint/.eslintrc-magento +++ b/eslint/.eslintrc-magento @@ -18,7 +18,8 @@ "eol-last": 2, "eqeqeq": [2, "smart"], "guard-for-in": 2, - "keyword-spacing": [2, {}], + "indent": [2, 4], + "keyword-spacing": [2, {"after": true, "before": true}], "lines-around-comment": [ 2, { @@ -50,6 +51,7 @@ "no-fallthrough": 2, "no-floating-decimal": 2, "no-func-assign": 2, + "no-global-assign": 2, "no-implied-eval": 2, "no-inner-declarations": 2, "no-invalid-regexp": 2, @@ -81,10 +83,16 @@ } ], "no-use-before-define": 2, + "no-useless-call": 2, + "no-useless-computed-key": 2, + "no-useless-constructor": 2, + "no-useless-escape": 2, + "no-useless-rename": 2, + "no-useless-return": 2, "no-with": 2, "one-var": [2, "always"], "operator-assignment": [2, "always"], - "quotes": [2, "single"], + "quotes": [2, "single", {"allowTemplateLiterals": true}], "radix": 2, "semi": [2, "always"], "semi-spacing": 2, diff --git a/eslint/rules/utils.js b/eslint/rules/utils.js index ae181210..398e2b86 100644 --- a/eslint/rules/utils.js +++ b/eslint/rules/utils.js @@ -75,18 +75,18 @@ function getExpressionId(node) { while (node) { switch (node.type) { - case 'CallExpression': - node = node.callee; - break; - - case 'MemberExpression': - node = node.object; - break; - - case 'Identifier': - return node; - default: - return null; + case 'CallExpression': + node = node.callee; + break; + + case 'MemberExpression': + node = node.object; + break; + + case 'Identifier': + return node; + default: + return null; } } }