Skip to content
95 changes: 95 additions & 0 deletions Magento2/Sniffs/Annotation/MethodArgumentsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class MethodArgumentsSniff implements Sniff
'self'
];

private const MAXIMUM_COMPLEXITY_ALLOWED_FOR_NO_DOCBLOCK = 5;

/**
* @inheritdoc
*/
Expand Down Expand Up @@ -568,6 +570,9 @@ public function process(File $phpcsFile, $stackPtr)
$previousCommentClosePtr = $phpcsFile->findPrevious(T_DOC_COMMENT_CLOSE_TAG, $stackPtr - 1, 0);
if ($previousCommentClosePtr && $previousCommentOpenPtr) {
if (!$this->validateCommentBlockExists($phpcsFile, $previousCommentClosePtr, $stackPtr)) {
if (!$this->checkIfMethodNeedsDocBlock($phpcsFile, $stackPtr)) {
return;
}
$phpcsFile->addError('Comment block is missing', $stackPtr, 'NoCommentBlock');
return;
}
Expand Down Expand Up @@ -617,6 +622,96 @@ public function process(File $phpcsFile, $stackPtr)
);
}

/**
* Check method if it has defined return & parameter types, then it doesn't need DocBlock
*
* @param File $phpcsFile
* @param int $stackPtr
* @return bool
*/
private function checkIfMethodNeedsDocBlock(File $phpcsFile, $stackPtr) : bool
{
if ($this->checkIfNamespaceContainsApi($phpcsFile)) {
// Note: API classes MUST have DocBlock because it uses them instead of reflection for types
return true;
}
Comment on lines +634 to +637
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test to cover this case - where a docblock is required due to a class being an API.

$foundAllParameterTypes = true;
$methodParameters = $phpcsFile->getMethodParameters($stackPtr);
foreach ($methodParameters as $parameter) {
if (!$parameter['type_hint']) {
return true; // doesn't have type hint
}
}
$methodProperties = $phpcsFile->getMethodProperties($stackPtr);
$foundReturnType = !!$methodProperties['return_type'];
if (!$foundReturnType) {
$methodName = $phpcsFile->getDeclarationName($stackPtr);
// Note: __construct and __destruct can't have return types
if ('__construct' !== $methodName && '__destruct' !== $methodName) {
return true;
}
}
$complexity = $this->getMethodComplexity($phpcsFile, $stackPtr);
return $complexity > static::MAXIMUM_COMPLEXITY_ALLOWED_FOR_NO_DOCBLOCK;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test to cover this case - where a docblock is required due to high complexity score.

}

/**
* Check if namespace contains API
*
* @param File $phpcsFile
* @return bool
*/
private function checkIfNamespaceContainsApi(File $phpcsFile) : bool
{
$namespaceStackPtr = $phpcsFile->findNext(T_NAMESPACE, 0);
if (!is_int($namespaceStackPtr)) {
return false;
}
$tokens = $phpcsFile->getTokens();
for ($index = $namespaceStackPtr; 'T_SEMICOLON' !== ($tokens[$index]['type'] ?? 'T_SEMICOLON'); $index++) {
if ('T_STRING' === $tokens[$index]['type'] && 'Api' === $tokens[$index]['content']) {
return true;
}
}
return false;
}

/**
* Get method CyclomaticComplexity
*
* @param File $phpcsFile
* @param int $stackPtr
* @return int
*/
private function getMethodComplexity(File $phpcsFile, $stackPtr) : int
{
$tokens = $phpcsFile->getTokens();
$start = $tokens[$stackPtr]['scope_opener'];
$end = $tokens[$stackPtr]['scope_closer'];
$predicateNodes = [
T_CASE => true,
T_DEFAULT => true,
T_CATCH => true,
T_IF => true,
T_FOR => true,
T_FOREACH => true,
T_WHILE => true,
T_ELSEIF => true,
T_INLINE_THEN => true,
T_COALESCE => true,
T_COALESCE_EQUAL => true,
T_MATCH_ARROW => true,
T_NULLSAFE_OBJECT_OPERATOR => true,
];
$complexity = 1;
for ($stackPtr = $start + 1; $stackPtr < $end; $stackPtr++) {
if (isset($predicateNodes[$tokens[$stackPtr]['code']])) {
$complexity++;
}
}
return $complexity;
}

/**
* Validates function params format consistency.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ public function processMemberVar(File $phpcsFile, $stackPtr)
);

if ($commentEnd === false || $tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG) {
$propertyInfo = $phpcsFile->getMemberProperties($stackPtr);
if ($propertyInfo['type']) {
return; // If variable has a type, it is okay to not have a DocBlock
}
$phpcsFile->addWarning('Missing PHP DocBlock for class property.', $stackPtr, 'Missing');
return;
}
Expand Down
54 changes: 52 additions & 2 deletions Magento2/Tests/Annotation/MethodArgumentsUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,63 @@ public function invalidDocBlockShouldNotCauseFatalErrorInSniff(int $number): int
* @return string
*/
#[\ReturnTypeWillChange]
public function methodWithAttributeAndValidDocblock(string $text): string
public function methodWithAttributeAndValidDocblockWithTypes(string $text): string
{
return $text;
}

/**
* Short description.
*
* @param string $text
* @return string
*/
#[\ReturnTypeWillChange]
public function methodWithAttributeAndValidDocblockWithoutTypes()
Comment on lines +52 to +56
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method has no $text parameter. If this is an intentional test, please add a comment to show the same.

{
return $text;
}

#[\ReturnTypeWillChange]
public function methodWithAttributeAndWithoutDocblock(string $text): string
public function methodWithAttributeAndWithoutDocblockWithTypes(string $text): string
{
return $text;
}

#[\ReturnTypeWillChange]
public function methodWithAttributeAndWithoutDocblockWithoutTypes($text)
{
return $text;
}

public function methodWithoutDockblockWithParameterTypeWithoutReturnType(string $text)
{
return $text;
}

public function methodWithoutDockblockWithoutParameterTypeWithReturnType($text) : string
{
return $text;
}

/**
* Short description.
*
* @param string $text
* @return string
*/
public function methodWithDockblockWithParameterTypeWithoutReturnType(string $text)
{
return $text;
}

/**
* Short description.
*
* @param mixed $text
* @return string
*/
public function methodWithDockblockWithoutParameterTypeWithReturnType($text) : string
{
return $text;
}
4 changes: 3 additions & 1 deletion Magento2/Tests/Annotation/MethodArgumentsUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ public function getErrorList()
12 => 1,
21 => 1,
32 => 1,
50 => 1
68 => 1,
73 => 1,
78 => 1,
];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,20 @@ class correctlyFormattedClassMemberDocBlock
*/
protected readonly int $readOnlyInteger;
}

class classWithPropertiesWithTypesAndWithoutDocBlock
{
protected string $test1 = 'asdf';

protected ?string $test2;

private array $array1 = [];

public array $array2;

protected readonly int $readOnlyInteger;

protected readonly string $readOnlyString;

private ?\Exception $exception;
}
Loading