From 5fd88b518cd9d40121ac14e75dd55ae063376ed5 Mon Sep 17 00:00:00 2001 From: konarshankar07 Date: Mon, 9 Sep 2019 22:21:44 +0530 Subject: [PATCH 01/12] New rule added for class property PHP Doc block --- .../ClassPropertyPHPDocFormattingSniff.php | 72 +++++++++++++++++++ .../ClassPropertyPHPDocFormattingUnitTest.inc | 35 +++++++++ .../ClassPropertyPHPDocFormattingUnitTest.php | 32 +++++++++ Magento2/ruleset.xml | 4 ++ 4 files changed, 143 insertions(+) create mode 100644 Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php create mode 100644 Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc create mode 100644 Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php new file mode 100644 index 00000000..3a08852d --- /dev/null +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -0,0 +1,72 @@ +getTokens(); + + $commentEnd = $phpcsFile->findPrevious($this->ignoreTokens, ($stackPtr - 1), null, true); + if ($commentEnd === false + || ($tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG + && $tokens[$commentEnd]['code'] !== T_COMMENT) + ) { + $phpcsFile->addWarning('Missing class property doc comment', $stackPtr, 'Missing'); + return; + } + + $commentStart = $tokens[$commentEnd]['comment_opener']; + + $foundVar = null; + foreach ($tokens[$commentStart]['comment_tags'] as $tag) { + if ($tokens[$tag]['content'] === '@var') { + if ($foundVar !== null) { + $error = 'Only one @var tag is allowed in a class property comment'; + $phpcsFile->addWarning($error, $tag, 'DuplicateVar'); + } else { + $foundVar = $tag; + } + } + } + + if ($foundVar === null) { + $error = 'Missing @var tag in class property comment'; + $phpcsFile->addWarning($error, $commentEnd, 'MissingVar'); + return; + } + + $string = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $foundVar, $commentEnd); + if ($string === false || $tokens[$string]['line'] !== $tokens[$foundVar]['line']) { + $error = 'Content missing for @var tag in class property comment'; + $phpcsFile->addWarning($error, $foundVar, 'EmptyVar'); + return; + } + } +} diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc new file mode 100644 index 00000000..2abc3e8f --- /dev/null +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc @@ -0,0 +1,35 @@ + 1, + 23 => 1, + 30 => 1, + 34 => 1 + ]; + } +} diff --git a/Magento2/ruleset.xml b/Magento2/ruleset.xml index 246b6fe1..e8e34d12 100644 --- a/Magento2/ruleset.xml +++ b/Magento2/ruleset.xml @@ -528,6 +528,10 @@ 5 warning + + 5 + warning + 5 warning From 740410bb46a45405b065e8f63ff5d8feed9f4756 Mon Sep 17 00:00:00 2001 From: konarshankar07 Date: Wed, 11 Sep 2019 23:03:19 +0530 Subject: [PATCH 02/12] Fixed false positive finding --- .../ClassPropertyPHPDocFormattingSniff.php | 46 +++++++++++++------ .../Sniffs/Exceptions/DirectThrowSniff.php | 1 + Magento2/Sniffs/Security/SuperglobalSniff.php | 4 +- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index 3a08852d..a9765278 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -2,11 +2,17 @@ namespace Magento2\Sniffs\Commenting; use PHP_CodeSniffer\Files\File; -use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Sniffs\AbstractVariableSniff; -class ClassPropertyPHPDocFormattingSniff implements Sniff +/** + * Class ClassPropertyPHPDocFormattingSniff + */ +class ClassPropertyPHPDocFormattingSniff extends AbstractVariableSniff { + /** + * @var array + */ private $ignoreTokens = [ T_PUBLIC, T_PRIVATE, @@ -17,19 +23,11 @@ class ClassPropertyPHPDocFormattingSniff implements Sniff ]; /** - * @inheritDoc + * @param File $phpcsFile + * @param int $stackPtr + * @return int|void */ - public function register() - { - return [ - T_VARIABLE - ]; - } - - /** - * @inheritDoc - */ - public function process(File $phpcsFile, $stackPtr) + public function processMemberVar(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); @@ -69,4 +67,24 @@ public function process(File $phpcsFile, $stackPtr) return; } } + + /** + * @param File $phpcsFile + * @param int $stackPtr + * @return int|void + * phpcs:disable Magento2.CodeAnalysis.EmptyBlock + */ + protected function processVariable(File $phpcsFile, $stackPtr) + { + } + + /** + * @param File $phpcsFile + * @param int $stackPtr + * @return int|void + * phpcs:disable Magento2.CodeAnalysis.EmptyBlock + */ + protected function processVariableInString(File $phpcsFile, $stackPtr) + { + } } diff --git a/Magento2/Sniffs/Exceptions/DirectThrowSniff.php b/Magento2/Sniffs/Exceptions/DirectThrowSniff.php index 9612d84c..b50ce5d1 100644 --- a/Magento2/Sniffs/Exceptions/DirectThrowSniff.php +++ b/Magento2/Sniffs/Exceptions/DirectThrowSniff.php @@ -16,6 +16,7 @@ class DirectThrowSniff implements Sniff /** * String representation of warning. * phpcs:disable Generic.Files.LineLength.TooLong + * @var string */ protected $warningMessage = 'Direct throw of generic Exception is discouraged. Use context specific instead.'; //phpcs:enable diff --git a/Magento2/Sniffs/Security/SuperglobalSniff.php b/Magento2/Sniffs/Security/SuperglobalSniff.php index 466c1cba..c9f58300 100644 --- a/Magento2/Sniffs/Security/SuperglobalSniff.php +++ b/Magento2/Sniffs/Security/SuperglobalSniff.php @@ -42,7 +42,7 @@ class SuperglobalSniff implements Sniff protected $errorCode = 'SuperglobalUsageError'; /** - * @inheritdoc + * @var array */ protected $superGlobalErrors = [ '$GLOBALS', @@ -55,7 +55,7 @@ class SuperglobalSniff implements Sniff ]; /** - * @inheritdoc + * @var array */ protected $superGlobalWarning = [ '$_COOKIE', //sometimes need to get list of all cookies array and there are no methods to do that in M2 From 97cc6618592380361685a7da507cdfcad6dc4c7c Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Sun, 15 Sep 2019 13:17:17 +0530 Subject: [PATCH 03/12] added correct formatted examples --- .../ClassPropertyPHPDocFormattingSniff.php | 2 +- .../ClassPropertyPHPDocFormattingUnitTest.inc | 28 +++++++++++++++++-- .../ClassPropertyPHPDocFormattingUnitTest.php | 2 +- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index a9765278..a057e5b0 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -56,7 +56,7 @@ public function processMemberVar(File $phpcsFile, $stackPtr) if ($foundVar === null) { $error = 'Missing @var tag in class property comment'; - $phpcsFile->addWarning($error, $commentEnd, 'MissingVar'); + $phpcsFile->addWarning($error, $stackPtr, 'MissingVar'); return; } diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc index 2abc3e8f..dc40d74c 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc @@ -10,14 +10,14 @@ class Foo /** * Foo */ - private $_withoutClassAttribute = ''; + public $_withoutClassAttribute = ''; /** * @var Test * * Short Description */ - private $_classAttributeWithShortDescription = ''; + protected $_classAttributeWithShortDescription = ''; /** * @var @@ -33,3 +33,27 @@ class Foo private $_missingDocBlockClassAttribute = ''; } + +class Bar { + + /** + * @var correctlyFormattedPublicClassMember + * + * Short Description + */ + public $correctlyFormattedPublicClassMember; + + /** + * @var correctlyFormattedPrivateClassMember + * + * Short Description + */ + private $correctlyFormattedPrivateClassMember; + + /** + * @var correctlyFormattedProtectedClassMember + * + * Short Description + */ + protected $correctlyFormattedProtectedClassMember; +} diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php index 6672d3f0..675eef4f 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php @@ -23,7 +23,7 @@ public function getErrorList() public function getWarningList() { return [ - 12 => 1, + 13 => 1, 23 => 1, 30 => 1, 34 => 1 From 519fc72f414838d5ad64f5ace2f2cc77c81836f2 Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Sun, 29 Sep 2019 19:42:53 +0530 Subject: [PATCH 04/12] Warning raised if class member already have meaningful description --- .../ClassPropertyPHPDocFormattingSniff.php | 8 ++++++ .../ClassPropertyPHPDocFormattingUnitTest.inc | 28 +++++++++++++++---- .../ClassPropertyPHPDocFormattingUnitTest.php | 8 ++++-- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index a057e5b0..846c3cfd 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -66,6 +66,14 @@ public function processMemberVar(File $phpcsFile, $stackPtr) $phpcsFile->addWarning($error, $foundVar, 'EmptyVar'); return; } + + // Check if class has already have meaningful description + $isShortDescription = $phpcsFile->findPrevious(T_DOC_COMMENT_STRING, $commentEnd, $foundVar, false); + if ($tokens[$string]['line'] !== $tokens[$isShortDescription]['line']) { + $error = 'Variable member already have meaningful name'; + $phpcsFile->addWarning($error, $isShortDescription, 'AlreadyMeaningFulNameVar'); + return; + } } /** diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc index dc40d74c..24e84eab 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc @@ -37,23 +37,41 @@ class Foo class Bar { /** - * @var correctlyFormattedPublicClassMember + * @var variablehasAlreadyhaveProtectedClassMember * * Short Description */ - public $correctlyFormattedPublicClassMember; + public $variablehasAlreadyhavePublicClassMember; /** - * @var correctlyFormattedPrivateClassMember + * @var variablehasAlreadyhavePrivateClassMember * * Short Description */ - private $correctlyFormattedPrivateClassMember; + private $variablehasAlreadyhavePrivateClassMember; /** - * @var correctlyFormattedProtectedClassMember + * @var variablehasAlreadyhaveProtectedClassMember * * Short Description */ + protected $variablehasAlreadyhaveProtectedClassMember; +} + +class correctlyFormattedClassMemberDocBlock +{ + /** + * @var correctlyFormattedPublicClassMember + */ + public $correctlyFormattedPublicClassMember; + + /** + * @var correctlyFormattedPrivateClassMember + */ + private $correctlyFormattedPrivateClassMember; + + /** + * @var correctlyFormattedProtectedClassMember + */ protected $correctlyFormattedProtectedClassMember; } diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php index 675eef4f..a1c4e068 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php @@ -24,9 +24,13 @@ public function getWarningList() { return [ 13 => 1, + 18 => 1, 23 => 1, - 30 => 1, - 34 => 1 + 30 => 2, + 34 => 1, + 42 => 1, + 49 => 1, + 56 => 1 ]; } } From 93ceb188ed53713b6ef7b44f58a67f4f6e39faa4 Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Sun, 13 Oct 2019 12:29:51 +0530 Subject: [PATCH 05/12] use of PHPDocFormattingValidator --- .../ClassPropertyPHPDocFormattingSniff.php | 29 ++++++++++++++++--- .../ClassPropertyPHPDocFormattingUnitTest.php | 4 +-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index 846c3cfd..72e6967f 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -3,6 +3,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\AbstractVariableSniff; +use PHP_CodeSniffer\Util\Tokens; /** * Class ClassPropertyPHPDocFormattingSniff @@ -22,6 +23,28 @@ class ClassPropertyPHPDocFormattingSniff extends AbstractVariableSniff T_WHITESPACE, ]; + /** + * @var PHPDocFormattingValidator + */ + private $PHPDocFormattingValidator; + + /** + * Constructs an AbstractVariableTest. + */ + public function __construct() + { + $scopes = Tokens::$ooScopeTokens; + $this->PHPDocFormattingValidator = new PHPDocFormattingValidator(); + $listen = [ + T_VARIABLE, + T_DOUBLE_QUOTED_STRING, + T_HEREDOC, + ]; + + parent::__construct($scopes, $listen, true); + + } + /** * @param File $phpcsFile * @param int $stackPtr @@ -39,9 +62,7 @@ public function processMemberVar(File $phpcsFile, $stackPtr) $phpcsFile->addWarning('Missing class property doc comment', $stackPtr, 'Missing'); return; } - $commentStart = $tokens[$commentEnd]['comment_opener']; - $foundVar = null; foreach ($tokens[$commentStart]['comment_tags'] as $tag) { if ($tokens[$tag]['content'] === '@var') { @@ -69,9 +90,9 @@ public function processMemberVar(File $phpcsFile, $stackPtr) // Check if class has already have meaningful description $isShortDescription = $phpcsFile->findPrevious(T_DOC_COMMENT_STRING, $commentEnd, $foundVar, false); - if ($tokens[$string]['line'] !== $tokens[$isShortDescription]['line']) { + if ($this->PHPDocFormattingValidator->providesMeaning($isShortDescription, $commentStart, $tokens) !== true) { $error = 'Variable member already have meaningful name'; - $phpcsFile->addWarning($error, $isShortDescription, 'AlreadyMeaningFulNameVar'); + $phpcsFile->addWarning($error, $isShortDescription, 'AlreadyHaveMeaningFulNameVar'); return; } } diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php index a1c4e068..ba70a2d1 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php @@ -26,11 +26,11 @@ public function getWarningList() 13 => 1, 18 => 1, 23 => 1, - 30 => 2, + 30 => 1, 34 => 1, 42 => 1, 49 => 1, - 56 => 1 + 56 => 1, ]; } } From 7f3a89fce97582671e06da9d755affb889812e0c Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Sun, 13 Oct 2019 15:24:39 +0530 Subject: [PATCH 06/12] Fixed static build tests --- .../Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index 72e6967f..ebd3105c 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -29,7 +29,7 @@ class ClassPropertyPHPDocFormattingSniff extends AbstractVariableSniff private $PHPDocFormattingValidator; /** - * Constructs an AbstractVariableTest. + * Constructs an ClassPropertyPHPDocFormattingSniff. */ public function __construct() { @@ -42,7 +42,6 @@ public function __construct() ]; parent::__construct($scopes, $listen, true); - } /** From 705cac5d2a472ef1083d2b050de6be6cc0b69a1b Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Wed, 16 Oct 2019 23:44:06 +0530 Subject: [PATCH 07/12] Feedback changes --- .../ClassPropertyPHPDocFormattingUnitTest.inc | 19 +++++++++++++------ .../ClassPropertyPHPDocFormattingUnitTest.php | 1 + 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc index 24e84eab..e166a0e0 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc @@ -37,25 +37,32 @@ class Foo class Bar { /** - * @var variablehasAlreadyhaveProtectedClassMember + * @var variableHasAlreadyHaveProtectedClassMember * * Short Description */ - public $variablehasAlreadyhavePublicClassMember; + public $variableHasAlreadyHavePublicClassMember; /** - * @var variablehasAlreadyhavePrivateClassMember + * @var variableHasAlreadyHavePrivateClassMember * * Short Description */ - private $variablehasAlreadyhavePrivateClassMember; + private $variableHasAlreadyHavePrivateClassMember; /** - * @var variablehasAlreadyhaveProtectedClassMember + * @var variableHasAlreadyHaveProtectedClassMember * * Short Description */ - protected $variablehasAlreadyhaveProtectedClassMember; + protected $variableHasAlreadyHaveProtectedClassMember; + + /** + * @var className + * + * Variable name + */ + private $variableName; } class correctlyFormattedClassMemberDocBlock diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php index ba70a2d1..6974eabe 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php @@ -31,6 +31,7 @@ public function getWarningList() 42 => 1, 49 => 1, 56 => 1, + 63 => 1 ]; } } From 785f0c32a6aa424303430ebbb8d141208bca9d6b Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Mon, 28 Oct 2019 13:50:12 +0530 Subject: [PATCH 08/12] Fixed PHPDocFormattingValidator class not found issue --- .../Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php | 1 + 1 file changed, 1 insertion(+) diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index ebd3105c..b77a3761 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -4,6 +4,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\AbstractVariableSniff; use PHP_CodeSniffer\Util\Tokens; +use Magento2\Helpers\Commenting\PHPDocFormattingValidator; /** * Class ClassPropertyPHPDocFormattingSniff From 17e993be460db53c4e83baedeb19f6580a41effc Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Sat, 9 Nov 2019 00:36:38 +0530 Subject: [PATCH 09/12] Feedback suggestions --- .../Commenting/ClassPropertyPHPDocFormattingSniff.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index b77a3761..b488438e 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -59,7 +59,7 @@ public function processMemberVar(File $phpcsFile, $stackPtr) || ($tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG && $tokens[$commentEnd]['code'] !== T_COMMENT) ) { - $phpcsFile->addWarning('Missing class property doc comment', $stackPtr, 'Missing'); + $phpcsFile->addWarning('Missing PHP DocBlock for class property.', $stackPtr, 'Missing'); return; } $commentStart = $tokens[$commentEnd]['comment_opener']; @@ -67,7 +67,7 @@ public function processMemberVar(File $phpcsFile, $stackPtr) foreach ($tokens[$commentStart]['comment_tags'] as $tag) { if ($tokens[$tag]['content'] === '@var') { if ($foundVar !== null) { - $error = 'Only one @var tag is allowed in a class property comment'; + $error = 'Only one @var tag is allowed for class property declaration.'; $phpcsFile->addWarning($error, $tag, 'DuplicateVar'); } else { $foundVar = $tag; @@ -76,14 +76,14 @@ public function processMemberVar(File $phpcsFile, $stackPtr) } if ($foundVar === null) { - $error = 'Missing @var tag in class property comment'; + $error = 'Class properties must have type declaration using @var tag.'; $phpcsFile->addWarning($error, $stackPtr, 'MissingVar'); return; } $string = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $foundVar, $commentEnd); if ($string === false || $tokens[$string]['line'] !== $tokens[$foundVar]['line']) { - $error = 'Content missing for @var tag in class property comment'; + $error = 'Content missing for @var tag in class property declaration.'; $phpcsFile->addWarning($error, $foundVar, 'EmptyVar'); return; } @@ -91,7 +91,7 @@ public function processMemberVar(File $phpcsFile, $stackPtr) // Check if class has already have meaningful description $isShortDescription = $phpcsFile->findPrevious(T_DOC_COMMENT_STRING, $commentEnd, $foundVar, false); if ($this->PHPDocFormattingValidator->providesMeaning($isShortDescription, $commentStart, $tokens) !== true) { - $error = 'Variable member already have meaningful name'; + $error = 'Short description duplicates class property name.'; $phpcsFile->addWarning($error, $isShortDescription, 'AlreadyHaveMeaningFulNameVar'); return; } From 88a5e7568540111ba2d127731c38008d207fc93d Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Sat, 16 Nov 2019 19:42:20 +0530 Subject: [PATCH 10/12] Feedback changes --- .../ClassPropertyPHPDocFormattingSniff.php | 27 ++++++++++++++++--- .../ClassPropertyPHPDocFormattingUnitTest.inc | 18 +++++++++++++ .../ClassPropertyPHPDocFormattingUnitTest.php | 3 ++- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index b488438e..cf006d56 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -88,11 +88,30 @@ public function processMemberVar(File $phpcsFile, $stackPtr) return; } - // Check if class has already have meaningful description - $isShortDescription = $phpcsFile->findPrevious(T_DOC_COMMENT_STRING, $commentEnd, $foundVar, false); - if ($this->PHPDocFormattingValidator->providesMeaning($isShortDescription, $commentStart, $tokens) !== true) { + // Check if class has already have meaningful description after @var tag + $isShortDescriptionAfterVar = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $foundVar + 4, $commentEnd, false, + null, + false); + if ($this->PHPDocFormattingValidator->providesMeaning($isShortDescriptionAfterVar, $commentStart, $tokens) !== true) { + preg_match('`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i', $tokens[($foundVar + 2)]['content'], $varParts); + if ($varParts[1]) { + return; + } + $error = 'Short description duplicates class property name.'; + $phpcsFile->addWarning($error, $isShortDescriptionAfterVar, 'AlreadyHaveMeaningFulNameVar'); + return; + } + // Check if class has already have meaningful description before @var tag + $isShortDescriptionPreviousVar = $phpcsFile->findPrevious(T_DOC_COMMENT_STRING, $foundVar, $commentStart, false, + null, + false); + if ($this->PHPDocFormattingValidator->providesMeaning($isShortDescriptionPreviousVar, $commentStart, $tokens) !== true) { + preg_match('`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i', $tokens[($foundVar + 2)]['content'], $varParts); + if ($varParts[1]) { + return; + } $error = 'Short description duplicates class property name.'; - $phpcsFile->addWarning($error, $isShortDescription, 'AlreadyHaveMeaningFulNameVar'); + $phpcsFile->addWarning($error, $isShortDescriptionPreviousVar, 'AlreadyHaveMeaningFulNameVar'); return; } } diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc index e166a0e0..6dcccf65 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc @@ -63,6 +63,13 @@ class Bar { * Variable name */ private $variableName; + + /** + * Some more invalid description + * + * @var test + */ + protected $test; } class correctlyFormattedClassMemberDocBlock @@ -81,4 +88,15 @@ class correctlyFormattedClassMemberDocBlock * @var correctlyFormattedProtectedClassMember */ protected $correctlyFormattedProtectedClassMember; + + /** + * @var Array|\Foo_BAR + */ + protected $array; + + /** + * @var \FOO\BAR\TEST\Class\\FooBarInterface| + * \FooObject_TEST_C + */ + private $testObject; } diff --git a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php index 6974eabe..8817ab3b 100644 --- a/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php +++ b/Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php @@ -31,7 +31,8 @@ public function getWarningList() 42 => 1, 49 => 1, 56 => 1, - 63 => 1 + 63 => 1, + 68 => 1 ]; } } From 146bfb099583f7ee26bbafec66fe6560eab4373a Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Sun, 22 Aug 2021 16:51:28 +0530 Subject: [PATCH 11/12] Resolved test failure --- .../Classes/DiscouragedDependenciesSniff.php | 6 --- .../ClassPropertyPHPDocFormattingSniff.php | 42 +++++++++++++++---- .../Methods/DeprecatedModelMethodSniff.php | 2 - Magento2/Sniffs/Security/XssTemplateSniff.php | 2 - 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/Magento2/Sniffs/Classes/DiscouragedDependenciesSniff.php b/Magento2/Sniffs/Classes/DiscouragedDependenciesSniff.php index ed106d4b..85c1f103 100644 --- a/Magento2/Sniffs/Classes/DiscouragedDependenciesSniff.php +++ b/Magento2/Sniffs/Classes/DiscouragedDependenciesSniff.php @@ -32,22 +32,16 @@ class DiscouragedDependenciesSniff implements Sniff protected $warningCode = 'ConstructorProxyInterceptor'; /** - * Aliases of proxies or plugins from use statements - * * @var string[] */ private $aliases = []; /** - * The current file - used for clearing USE aliases when file changes - * * @var null|string */ private $currentFile = null; /** - * Terms to search for in variables and namespaces - * * @var string[] */ public $incorrectClassNames = ['proxy','interceptor']; diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index cf006d56..268e4696 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -89,11 +89,24 @@ public function processMemberVar(File $phpcsFile, $stackPtr) } // Check if class has already have meaningful description after @var tag - $isShortDescriptionAfterVar = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $foundVar + 4, $commentEnd, false, + $isShortDescriptionAfterVar = $phpcsFile->findNext( + T_DOC_COMMENT_STRING, + $foundVar + 4, + $commentEnd, + false, null, - false); - if ($this->PHPDocFormattingValidator->providesMeaning($isShortDescriptionAfterVar, $commentStart, $tokens) !== true) { - preg_match('`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i', $tokens[($foundVar + 2)]['content'], $varParts); + false + ); + if ($this->PHPDocFormattingValidator->providesMeaning( + $isShortDescriptionAfterVar, + $commentStart, + $tokens + ) !== true) { + preg_match( + '`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i', + $tokens[($foundVar + 2)]['content'], + $varParts + ); if ($varParts[1]) { return; } @@ -102,11 +115,24 @@ public function processMemberVar(File $phpcsFile, $stackPtr) return; } // Check if class has already have meaningful description before @var tag - $isShortDescriptionPreviousVar = $phpcsFile->findPrevious(T_DOC_COMMENT_STRING, $foundVar, $commentStart, false, + $isShortDescriptionPreviousVar = $phpcsFile->findPrevious( + T_DOC_COMMENT_STRING, + $foundVar, + $commentStart, + false, null, - false); - if ($this->PHPDocFormattingValidator->providesMeaning($isShortDescriptionPreviousVar, $commentStart, $tokens) !== true) { - preg_match('`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i', $tokens[($foundVar + 2)]['content'], $varParts); + false + ); + if ($this->PHPDocFormattingValidator->providesMeaning( + $isShortDescriptionPreviousVar, + $commentStart, + $tokens + ) !== true) { + preg_match( + '`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i', + $tokens[($foundVar + 2)]['content'], + $varParts + ); if ($varParts[1]) { return; } diff --git a/Magento2/Sniffs/Methods/DeprecatedModelMethodSniff.php b/Magento2/Sniffs/Methods/DeprecatedModelMethodSniff.php index 94bfc2d5..df8fba14 100644 --- a/Magento2/Sniffs/Methods/DeprecatedModelMethodSniff.php +++ b/Magento2/Sniffs/Methods/DeprecatedModelMethodSniff.php @@ -40,8 +40,6 @@ class DeprecatedModelMethodSniff implements Sniff 'delete' ]; - protected $severity = 0; - /** * @inheritdoc */ diff --git a/Magento2/Sniffs/Security/XssTemplateSniff.php b/Magento2/Sniffs/Security/XssTemplateSniff.php index aa2accbe..dc17f03b 100644 --- a/Magento2/Sniffs/Security/XssTemplateSniff.php +++ b/Magento2/Sniffs/Security/XssTemplateSniff.php @@ -50,8 +50,6 @@ class XssTemplateSniff implements Sniff ]; /** - * Allowed method name - {suffix}Html{postfix}() - * * @var string */ protected $methodNameContains = 'html'; From 83f7609a946596a6305c5122abe54740ba87fa56 Mon Sep 17 00:00:00 2001 From: Shankar Konar Date: Sun, 22 Aug 2021 17:13:05 +0530 Subject: [PATCH 12/12] Resolved test failure --- Magento2/Helpers/Tokenizer/AbstractTokenizer.php | 4 ---- Magento2/Helpers/Tokenizer/Variable.php | 2 -- .../ClassPropertyPHPDocFormattingSniff.php | 12 +++--------- Magento2/Sniffs/Less/AvoidIdSniff.php | 2 -- Magento2/Sniffs/Less/IndentationSniff.php | 3 +-- Magento2/Sniffs/Less/PropertiesSortingSniff.php | 4 ---- Magento2/Sniffs/Less/SemicolonSpacingSniff.php | 5 +---- Magento2/Sniffs/Less/TypeSelectorsSniff.php | 2 -- Magento2/Sniffs/Less/ZeroUnitsSniff.php | 2 -- 9 files changed, 5 insertions(+), 31 deletions(-) diff --git a/Magento2/Helpers/Tokenizer/AbstractTokenizer.php b/Magento2/Helpers/Tokenizer/AbstractTokenizer.php index f49c753f..1a3c4a63 100644 --- a/Magento2/Helpers/Tokenizer/AbstractTokenizer.php +++ b/Magento2/Helpers/Tokenizer/AbstractTokenizer.php @@ -11,15 +11,11 @@ abstract class AbstractTokenizer { /** - * Current index in string - * * @var int */ protected $_currentIndex; /** - * String for tokenize - * * @var string */ protected $_string; diff --git a/Magento2/Helpers/Tokenizer/Variable.php b/Magento2/Helpers/Tokenizer/Variable.php index 81c8b1d8..43938c03 100644 --- a/Magento2/Helpers/Tokenizer/Variable.php +++ b/Magento2/Helpers/Tokenizer/Variable.php @@ -12,8 +12,6 @@ class Variable extends AbstractTokenizer { /** - * Internal counter used to keep track of how deep in array parsing we are - * * @var int */ protected $arrayDepth = 0; diff --git a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php index 268e4696..a402fbd8 100644 --- a/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php +++ b/Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php @@ -46,9 +46,7 @@ public function __construct() } /** - * @param File $phpcsFile - * @param int $stackPtr - * @return int|void + * @inheritDoc */ public function processMemberVar(File $phpcsFile, $stackPtr) { @@ -143,9 +141,7 @@ public function processMemberVar(File $phpcsFile, $stackPtr) } /** - * @param File $phpcsFile - * @param int $stackPtr - * @return int|void + * @inheritDoc * phpcs:disable Magento2.CodeAnalysis.EmptyBlock */ protected function processVariable(File $phpcsFile, $stackPtr) @@ -153,9 +149,7 @@ protected function processVariable(File $phpcsFile, $stackPtr) } /** - * @param File $phpcsFile - * @param int $stackPtr - * @return int|void + * @inheritDoc * phpcs:disable Magento2.CodeAnalysis.EmptyBlock */ protected function processVariableInString(File $phpcsFile, $stackPtr) diff --git a/Magento2/Sniffs/Less/AvoidIdSniff.php b/Magento2/Sniffs/Less/AvoidIdSniff.php index d4e1b213..62346274 100644 --- a/Magento2/Sniffs/Less/AvoidIdSniff.php +++ b/Magento2/Sniffs/Less/AvoidIdSniff.php @@ -25,8 +25,6 @@ class AvoidIdSniff implements Sniff public $supportedTokenizers = [TokenizerSymbolsInterface::TOKENIZER_CSS]; /** - * Tokens that can appear in a selector - * * @var array */ private $selectorTokens = [ diff --git a/Magento2/Sniffs/Less/IndentationSniff.php b/Magento2/Sniffs/Less/IndentationSniff.php index 76a6fbcc..57947a07 100644 --- a/Magento2/Sniffs/Less/IndentationSniff.php +++ b/Magento2/Sniffs/Less/IndentationSniff.php @@ -39,8 +39,7 @@ class IndentationSniff implements Sniff */ public $maxIndentLevel = 3; - /** Skip codes that can be detected by sniffer incorrectly - * + /** * @var array */ private $styleCodesToSkip = [T_ASPERAND, T_COLON, T_OPEN_PARENTHESIS, T_CLOSE_PARENTHESIS]; diff --git a/Magento2/Sniffs/Less/PropertiesSortingSniff.php b/Magento2/Sniffs/Less/PropertiesSortingSniff.php index d1c7c387..2c5fe9e5 100644 --- a/Magento2/Sniffs/Less/PropertiesSortingSniff.php +++ b/Magento2/Sniffs/Less/PropertiesSortingSniff.php @@ -25,15 +25,11 @@ class PropertiesSortingSniff implements Sniff public $supportedTokenizers = [TokenizerSymbolsInterface::TOKENIZER_CSS]; /** - * List of properties that belong to class - * * @var array */ private $properties = []; /** - * Skip symbols that can be detected by sniffer incorrectly - * * @var array */ private $styleSymbolsToSkip = [ diff --git a/Magento2/Sniffs/Less/SemicolonSpacingSniff.php b/Magento2/Sniffs/Less/SemicolonSpacingSniff.php index cad13290..8db8d216 100644 --- a/Magento2/Sniffs/Less/SemicolonSpacingSniff.php +++ b/Magento2/Sniffs/Less/SemicolonSpacingSniff.php @@ -25,8 +25,6 @@ class SemicolonSpacingSniff implements Sniff public $supportedTokenizers = [TokenizerSymbolsInterface::TOKENIZER_CSS]; /** - * Skip symbols that can be detected by sniffer incorrectly - * * @var array */ private $styleSymbolsToSkip = [ @@ -36,8 +34,7 @@ class SemicolonSpacingSniff implements Sniff TokenizerSymbolsInterface::CLOSE_PARENTHESIS, ]; - /** Skip codes that can be detected by sniffer incorrectly - * + /** * @var array */ private $styleCodesToSkip = [T_ASPERAND, T_COLON, T_OPEN_PARENTHESIS, T_CLOSE_PARENTHESIS]; diff --git a/Magento2/Sniffs/Less/TypeSelectorsSniff.php b/Magento2/Sniffs/Less/TypeSelectorsSniff.php index 00c8b35c..4735c761 100644 --- a/Magento2/Sniffs/Less/TypeSelectorsSniff.php +++ b/Magento2/Sniffs/Less/TypeSelectorsSniff.php @@ -23,8 +23,6 @@ class TypeSelectorsSniff implements Sniff { /** - * Tags that are not allowed as type selector - * * @var array */ private $tags = [ diff --git a/Magento2/Sniffs/Less/ZeroUnitsSniff.php b/Magento2/Sniffs/Less/ZeroUnitsSniff.php index 51971c78..c62eaa52 100644 --- a/Magento2/Sniffs/Less/ZeroUnitsSniff.php +++ b/Magento2/Sniffs/Less/ZeroUnitsSniff.php @@ -24,8 +24,6 @@ class ZeroUnitsSniff implements Sniff const CSS_PROPERTY_UNIT_REM = 'rem'; /** - * List of available CSS Property units - * * @var array */ private $units = [