Skip to content

Commit f3a98ac

Browse files
author
roettigl
committed
#54: Add new Rule for PHPDoc formatting
1 parent 90ab7cd commit f3a98ac

5 files changed

+126
-45
lines changed

‎Magento2/Sniffs/Commenting/FunctionPHPDocBlock.php renamed to ‎Magento2/Sniffs/Commenting/FunctionPHPDocBlockParser.php

+21-21
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace Magento2\Sniffs\Commenting;
88

9-
class FunctionPHPDocBlock
9+
class FunctionPHPDocBlockParser
1010
{
1111

1212
/**
@@ -33,10 +33,10 @@ public function execute(array $tokens, $docStart, $docEnd)
3333
}
3434

3535
$functionDeclarations = [
36-
'warning' => false,
3736
'description' => $description,
37+
'tags' => [],
3838
'return' => '',
39-
'parameter' => [],
39+
'parameters' => [],
4040
'throws' => [],
4141
];
4242

@@ -48,14 +48,13 @@ public function execute(array $tokens, $docStart, $docEnd)
4848
$line = $token['line'];
4949
$content = $token['content'];
5050

51-
preg_match('/@inheritdoc*/', $content, $inheritdoc);
52-
if (isset($inheritdoc[0])) {
53-
$functionDeclarations['warning'] = ['The @inheritdoc tag SHOULD NOT be used.', $i];
54-
55-
return $functionDeclarations;
56-
}
5751

5852
if ($code === T_DOC_COMMENT_TAG) {
53+
// add php tokens also without comment to tag list
54+
if (preg_match('/@[a-z]++/', $content, $output_array)) {
55+
$functionDeclarations['tags'][] = $content;
56+
}
57+
5958
$lastDocLine = $line;
6059
$docType = $token['content'];
6160
continue;
@@ -73,17 +72,22 @@ public function execute(array $tokens, $docStart, $docEnd)
7372
preg_match_all('/[A-Za-z0-9$]*/', $content, $docTokens);
7473
$docTokens = array_values(array_filter($docTokens[0]));
7574

75+
if (count($docTokens) === 0) {
76+
// ignore empty parameter declaration
77+
continue;
78+
}
79+
7680
switch ($docType) {
7781
case '@param':
78-
$functionDeclarations = self::addParamTagValue($docTokens, $functionDeclarations);
82+
$functionDeclarations = $this->addParamTagValue($docTokens, $functionDeclarations);
7983
break;
8084

8185
case '@return':
82-
$functionDeclarations = self::addReturnTagValue($docTokens, $functionDeclarations);
86+
$functionDeclarations = $this->addReturnTagValue($docTokens, $functionDeclarations);
8387
break;
8488

8589
case '@throws':
86-
$functionDeclarations = self::addThrowsTagValue($docTokens, $functionDeclarations);
90+
$functionDeclarations = $this->addThrowsTagValue($docTokens, $functionDeclarations);
8791
break;
8892
}
8993
}
@@ -98,10 +102,6 @@ public function execute(array $tokens, $docStart, $docEnd)
98102
*/
99103
private function addParamTagValue(array $tokens, array $functionDeclarations)
100104
{
101-
if (count($tokens) === 0) {
102-
return $functionDeclarations; // empty parameter declaration
103-
}
104-
105105
$type = false;
106106
$content = false;
107107

@@ -113,7 +113,7 @@ private function addParamTagValue(array $tokens, array $functionDeclarations)
113113
$type = $token;
114114
}
115115

116-
$functionDeclarations['parameter'][] = ['content' => $content, 'type' => $type,];
116+
$functionDeclarations['parameters'][] = ['content' => $content, 'type' => $type,];
117117

118118
return $functionDeclarations;
119119
}
@@ -123,9 +123,9 @@ private function addParamTagValue(array $tokens, array $functionDeclarations)
123123
* @param array $functionDeclarations
124124
* @return array
125125
*/
126-
private function addReturnTagValue(array $docTokens, array $functionDeclarations)
126+
private function addReturnTagValue(array $tokens, array $functionDeclarations)
127127
{
128-
// @todo imepelement me
128+
$functionDeclarations['return'] = $tokens[0];
129129
return $functionDeclarations;
130130
}
131131

@@ -134,9 +134,9 @@ private function addReturnTagValue(array $docTokens, array $functionDeclarations
134134
* @param array $functionDeclarations
135135
* @return array
136136
*/
137-
private function addThrowsTagValue(array $docTokens, array $functionDeclarations)
137+
private function addThrowsTagValue(array $tokens, array $functionDeclarations)
138138
{
139-
// @todo imepelement me
139+
$functionDeclarations['throws'][] = $tokens[0];
140140
return $functionDeclarations;
141141
}
142142
}

‎Magento2/Sniffs/Commenting/FunctionsPHPDocFormattingSniff.php

+75-16
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ class FunctionsPHPDocFormattingSniff implements Sniff
2828

2929
public function __construct()
3030
{
31-
$this->functionPHPDocBlock = new FunctionPHPDocBlock();
31+
$this->functionPHPDocBlock = new FunctionPHPDocBlockParser();
3232
}
3333

3434
/**
@@ -44,37 +44,44 @@ public function register()
4444
*/
4545
public function process(File $phpcsFile, $stackPtr)
4646
{
47-
$funcReturnType = $this->anaylsePhp7ReturnDeclaration($phpcsFile, $stackPtr);
47+
$funcReturnType = $this->analysePhp7ReturnDeclaration($phpcsFile, $stackPtr);
4848
$funcParamTypeList = $this->analysePhp7ParamDeclarations($phpcsFile, $stackPtr);
4949
$phpDocTokens = $this->getPhpDocTokens($phpcsFile, $stackPtr);
50+
$hasPhp7TypeDeclarations = $funcReturnType !== false || count($funcParamTypeList['missing_type']) === 0;
5051

51-
if ($phpDocTokens === false && $funcReturnType === false && count($funcParamTypeList['missing_type'] !== 0)) {
52-
$phpcsFile->addWarning('Use php 7 type declarations or an php doc block', $stackPtr, $this->warningCode);
53-
54-
// @fixme find also __constuct that dont have return type
52+
if ($phpDocTokens === false && $hasPhp7TypeDeclarations === true) {
53+
// NO check it use all php 7 type declarations and no php doc docblock
5554
return;
5655
}
5756

58-
if ($phpDocTokens === false) {
59-
// @todo impelement checks
57+
if ($phpDocTokens === false && $hasPhp7TypeDeclarations === false) {
58+
$phpcsFile->addWarning('Use php 7 type declarations or an php doc block', $stackPtr, $this->warningCode);
59+
6060
return;
6161
}
6262

63-
$parsePhpDocTokens = $this->functionPHPDocBlock->execute(
63+
$phpDocTokensList = $this->functionPHPDocBlock->execute(
6464
$phpcsFile->getTokens(),
6565
$phpDocTokens[0],
6666
$phpDocTokens[1]
6767
);
6868

69-
$warning = $parsePhpDocTokens['warning'];
69+
if (array_key_exists('@inheritdoc', array_flip($phpDocTokensList['tags']))) {
70+
$phpcsFile->addWarning('The @inheritdoc tag SHOULD NOT be used', $stackPtr, $this->warningCode);
7071

71-
if ($warning !== false) {
72-
$phpcsFile->addWarning($warning[0], $warning[1], $this->warningCode);
7372
return;
7473
}
7574

75+
if (count($phpDocTokensList['parameters']) > 0) {
76+
$phpcsFile = $this->comparePhp7WithDocBlock(
77+
$funcParamTypeList,
78+
$phpDocTokensList['parameters'],
79+
$phpcsFile,
80+
$stackPtr
81+
);
82+
}
83+
7684

77-
// @todo impelement checks
7885
}
7986

8087
/**
@@ -83,12 +90,17 @@ public function process(File $phpcsFile, $stackPtr)
8390
* @param $stackPtr
8491
* @return bool|string
8592
*/
86-
private function anaylsePhp7ReturnDeclaration(File $phpcsFile, $stackPtr)
93+
private function analysePhp7ReturnDeclaration(File $phpcsFile, $stackPtr)
8794
{
8895
$tokens = $phpcsFile->getTokens();
96+
$functionNameToken = $phpcsFile->findNext(T_STRING, $stackPtr, $tokens[$stackPtr]['parenthesis_opener']);
97+
if (strpos($tokens[$functionNameToken]['content'], '__construct') === 0) {
98+
// magic functions start with __construct dont have php7 return type
99+
return 'void';
100+
}
101+
89102
$funcParamCloser = $tokens[$stackPtr]['parenthesis_closer'];
90103
$funcReturnTypePos = $phpcsFile->findNext([T_STRING], $funcParamCloser, $tokens[$stackPtr]['scope_opener']);
91-
92104
$funcReturnType = false;
93105
if ($funcReturnTypePos !== false) {
94106
$funcReturnType = $tokens[$funcReturnTypePos]['content'];
@@ -130,6 +142,7 @@ private function parseFunctionTokens(array $tokens)
130142
{
131143
$paramType = null;
132144
$functionParameterList = [
145+
'count' => 0,
133146
'missing_type' => [],
134147
'has_type' => [],
135148
];
@@ -151,8 +164,9 @@ private function parseFunctionTokens(array $tokens)
151164
continue;
152165
}
153166

167+
$functionParameterList['count']++;
154168
$key = $paramType !== null ? 'has_type' : 'missing_type';
155-
$functionParameterList[$key][] =
169+
$functionParameterList[$key][$content] =
156170
[
157171
'content' => $content,
158172
'type' => $paramType,
@@ -192,4 +206,49 @@ private function getPhpDocTokens(File $phpcsFile, $stackPtr)
192206

193207
return [$commentStart + 1, $commentEnd - 1];
194208
}
209+
210+
private function comparePhp7WithDocBlock(array $php7Tokens, array $docBlockTokens, File $phpcsFile, $stackPtr)
211+
{
212+
213+
$parsedDocToken = [];
214+
foreach ($docBlockTokens as $token) {
215+
$parameterName = $token['content'];
216+
if (isset($parsedDocToken[$parameterName])) {
217+
$phpcsFile->addWarning(
218+
sprintf('Parameter %s is definitely multiple', $parameterName),
219+
$stackPtr,
220+
$this->warningCode
221+
);
222+
return $phpcsFile;
223+
}
224+
225+
$parsedDocToken[$parameterName] = $token['type'];
226+
}
227+
228+
if (count($parsedDocToken) > $php7Tokens['count']) {
229+
$phpcsFile->addWarning(
230+
'More documented parameter than real function parameter',
231+
$stackPtr,
232+
$this->warningCode
233+
);
234+
return $phpcsFile;
235+
}
236+
237+
$parsedDocTokenKeys = array_keys($parsedDocToken);
238+
$hasMissingTypes = count($php7Tokens['missing_type']) > 0;
239+
if ($hasMissingTypes === true && array_keys($php7Tokens['missing_type']) !== $parsedDocTokenKeys) {
240+
$phpcsFile->addWarning(
241+
'Documented parameter and real function parameter dont match',
242+
$stackPtr,
243+
$this->warningCode
244+
);
245+
246+
return $phpcsFile;
247+
}
248+
249+
$t = 12;
250+
251+
252+
return $phpcsFile;
253+
}
195254
}

‎Magento2/Tests/Commenting/FunctionsPHPDocFormattingUnitTest.1.inc

+4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
class EverythingIsGoodHere
44
{
55

6+
public function __construct(string $arg1)
7+
{
8+
}
9+
610
private function executeSimpleFunctionNoDoc(): void
711
{
812
}

‎Magento2/Tests/Commenting/FunctionsPHPDocFormattingUnitTest.2.inc

+10-8
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,30 @@
11
<?php
22

3+
34
class EverythingIsBadHere
45
{
6+
7+
8+
/**
9+
* @param string $arg1
10+
*/
11+
private function removeParam ()
12+
{
13+
}
14+
515
/**
616
* @param bool $arg2
717
* @param string $arg1
818
*/
919
private function wrongParamOrder($arg1, $arg2): bool
1020
{
11-
1221
}
1322

1423
/**
1524
* @param bool $arg2
1625
*/
1726
private function missingParamAnnotation($arg1, $arg2): bool
1827
{
19-
2028
}
2129

2230
private function missingArgumentAnnotations($arg1, $arg2): bool
@@ -31,15 +39,13 @@ class EverythingIsBadHere
3139
*/
3240
private function paramDuplication($arg1, $arg2): bool
3341
{
34-
3542
}
3643

3744
/**
3845
* @inheritdoc
3946
*/
4047
private function inheritDocShouldNotBeUsed(string $arg1, bool $arg2)
4148
{
42-
4349
}
4450

4551
/**
@@ -48,23 +54,20 @@ class EverythingIsBadHere
4854
*/
4955
private function missingParamType($arg1, $arg2): bool
5056
{
51-
5257
}
5358

5459
/**
5560
* Useless description
5661
*/
5762
private function uselessDescription(string $arg1, bool $arg2): bool
5863
{
59-
6064
}
6165

6266
/**
6367
*
6468
*/
6569
private function dockBlockIsEmpty(string $arg1, bool $arg2): bool
6670
{
67-
6871
}
6972

7073
/**
@@ -76,6 +79,5 @@ class EverythingIsBadHere
7679
*/
7780
private function redundantLines($arg1, $arg2): bool
7881
{
79-
8082
}
8183
}

‎Magento2/Tests/Commenting/FunctionsPHPDocFormattingUnitTest.php

+16
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,22 @@ public function getErrorList()
2626
public function getWarningList($testFile = '')
2727
{
2828

29+
if($testFile === 'FunctionsPHPDocFormattingUnitTest.2.inc')
30+
{
31+
return [
32+
11,
33+
19,
34+
26,
35+
30,
36+
40,
37+
47,
38+
55,
39+
62,
40+
69,
41+
80
42+
];
43+
}
44+
2945
return [];
3046
}
3147
}

0 commit comments

Comments
 (0)