Skip to content

Commit fe32712

Browse files
sivaschenkosivaschenko
committed
magento#315: Fixed undefined index in Magento2.Commenting.ClassPropertyPHPDocFormatting
1 parent 6451e34 commit fe32712

File tree

3 files changed

+105
-61
lines changed

3 files changed

+105
-61
lines changed

‎Magento2/Sniffs/Commenting/ClassPropertyPHPDocFormattingSniff.php

+94-61
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,32 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
declare(strict_types=1);
7+
68
namespace Magento2\Sniffs\Commenting;
79

810
use PHP_CodeSniffer\Files\File;
911
use PHP_CodeSniffer\Sniffs\AbstractVariableSniff;
10-
use PHP_CodeSniffer\Util\Tokens;
1112
use Magento2\Helpers\Commenting\PHPDocFormattingValidator;
1213

1314
/**
1415
* Class ClassPropertyPHPDocFormattingSniff
1516
*/
1617
class ClassPropertyPHPDocFormattingSniff extends AbstractVariableSniff
1718
{
18-
1919
/**
2020
* @var array
2121
*/
22-
private $ignoreTokens = [
22+
private const TOKENS_ALLOWED_BETWEEN_PHPDOC_AND_PROPERTY_NAME = [
2323
T_PUBLIC,
2424
T_PRIVATE,
2525
T_PROTECTED,
2626
T_VAR,
2727
T_STATIC,
2828
T_WHITESPACE,
29+
T_NS_SEPARATOR,
30+
T_STRING,
31+
T_COMMENT
2932
];
3033

3134
/**
@@ -38,15 +41,9 @@ class ClassPropertyPHPDocFormattingSniff extends AbstractVariableSniff
3841
*/
3942
public function __construct()
4043
{
41-
$scopes = Tokens::$ooScopeTokens;
4244
$this->PHPDocFormattingValidator = new PHPDocFormattingValidator();
43-
$listen = [
44-
T_VARIABLE,
45-
T_DOUBLE_QUOTED_STRING,
46-
T_HEREDOC,
47-
];
4845

49-
parent::__construct($scopes, $listen, true);
46+
parent::__construct();
5047
}
5148

5249
/**
@@ -56,111 +53,147 @@ public function processMemberVar(File $phpcsFile, $stackPtr)
5653
{
5754
$tokens = $phpcsFile->getTokens();
5855

59-
$commentEnd = $phpcsFile->findPrevious($this->ignoreTokens, ($stackPtr - 1), null, true);
56+
$commentEnd = $phpcsFile->findPrevious(
57+
self::TOKENS_ALLOWED_BETWEEN_PHPDOC_AND_PROPERTY_NAME,
58+
$stackPtr - 1,
59+
null,
60+
true
61+
);
6062

61-
if ($commentEnd !== false && $tokens[$commentEnd]['code'] === T_STRING) {
62-
$commentEnd = $phpcsFile->findPrevious($this->ignoreTokens, ($commentEnd - 1), null, true);
63-
} elseif ($commentEnd === false
64-
|| ($tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG
65-
&& $tokens[$commentEnd]['code'] !== T_COMMENT)
66-
) {
63+
if ($commentEnd === false || $tokens[$commentEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG) {
6764
$phpcsFile->addWarning('Missing PHP DocBlock for class property.', $stackPtr, 'Missing');
6865
return;
6966
}
7067

7168
$commentStart = $tokens[$commentEnd]['comment_opener'];
72-
$foundVar = null;
69+
$varAnnotationPosition = null;
7370
foreach ($tokens[$commentStart]['comment_tags'] as $tag) {
7471
if ($tokens[$tag]['content'] === '@var') {
75-
if ($foundVar !== null) {
76-
$error = 'Only one @var tag is allowed for class property declaration.';
77-
$phpcsFile->addWarning($error, $tag, 'DuplicateVar');
72+
if ($varAnnotationPosition !== null) {
73+
$phpcsFile->addWarning(
74+
'Only one @var tag is allowed for class property declaration.',
75+
$tag,
76+
'DuplicateVar'
77+
);
7878
} else {
79-
$foundVar = $tag;
79+
$varAnnotationPosition = $tag;
8080
}
8181
}
8282
}
8383

84-
if ($foundVar === null) {
85-
$error = 'Class properties must have type declaration using @var tag.';
86-
$phpcsFile->addWarning($error, $stackPtr, 'MissingVar');
84+
if ($varAnnotationPosition === null) {
85+
$phpcsFile->addWarning(
86+
'Class properties must have type declaration using @var tag.',
87+
$stackPtr,
88+
'MissingVar'
89+
);
8790
return;
8891
}
8992

90-
$string = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $foundVar, $commentEnd);
91-
if ($string === false || $tokens[$string]['line'] !== $tokens[$foundVar]['line']) {
92-
$error = 'Content missing for @var tag in class property declaration.';
93-
$phpcsFile->addWarning($error, $foundVar, 'EmptyVar');
93+
$string = $phpcsFile->findNext(T_DOC_COMMENT_STRING, $varAnnotationPosition, $commentEnd);
94+
if ($string === false || $tokens[$string]['line'] !== $tokens[$varAnnotationPosition]['line']) {
95+
$phpcsFile->addWarning(
96+
'Content missing for @var tag in class property declaration.',
97+
$varAnnotationPosition,
98+
'EmptyVar'
99+
);
94100
return;
95101
}
96102

97103
// Check if class has already have meaningful description after @var tag
98-
$isShortDescriptionAfterVar = $phpcsFile->findNext(
104+
$shortDescriptionAfterVarPosition = $phpcsFile->findNext(
99105
T_DOC_COMMENT_STRING,
100-
$foundVar + 4,
106+
$varAnnotationPosition + 4,
101107
$commentEnd,
102108
false,
103109
null,
104110
false
105111
);
112+
106113
if ($this->PHPDocFormattingValidator->providesMeaning(
107-
$isShortDescriptionAfterVar,
114+
$shortDescriptionAfterVarPosition,
108115
$commentStart,
109116
$tokens
110117
) !== true) {
111118
preg_match(
112119
'`^((?:\|?(?:array\([^\)]*\)|[\\\\\[\]]+))*)( .*)?`i',
113-
$tokens[($foundVar + 2)]['content'],
120+
$tokens[($varAnnotationPosition + 2)]['content'],
114121
$varParts
115122
);
116123
if ($varParts[1]) {
117124
return;
118125
}
119-
$error = 'Short description must be before @var tag.';
120-
$phpcsFile->addWarning($error, $isShortDescriptionAfterVar, 'ShortDescriptionAfterVar');
121-
return;
126+
$phpcsFile->addWarning(
127+
'Short description must be before @var tag.',
128+
$shortDescriptionAfterVarPosition,
129+
'ShortDescriptionAfterVar'
130+
);
122131
}
123132

124-
// Check if class has already have meaningful description before @var tag
125-
$isShortDescriptionPreviousVar = $phpcsFile->findPrevious(
133+
$this->processPropertyShortDescription($phpcsFile, $stackPtr, $varAnnotationPosition, $commentStart);
134+
}
135+
136+
/**
137+
* Check if class has already have meaningful description before var tag
138+
*
139+
* @param File $phpcsFile
140+
* @param int $propertyNamePosition
141+
* @param int $varAnnotationPosition
142+
* @param int $commentStart
143+
*/
144+
private function processPropertyShortDescription(
145+
File $phpcsFile,
146+
int $propertyNamePosition,
147+
int $varAnnotationPosition,
148+
int $commentStart
149+
) {
150+
$propertyShortDescriptionPosition = $phpcsFile->findPrevious(
126151
T_DOC_COMMENT_STRING,
127-
$foundVar,
152+
$varAnnotationPosition,
128153
$commentStart,
129154
false,
130155
null,
131156
false
132157
);
133158

134-
if ($isShortDescriptionPreviousVar === false) {
159+
if ($propertyShortDescriptionPosition === false) {
135160
return;
136161
}
137162

138-
$propertyNamePosition = $phpcsFile->findNext(
139-
T_VARIABLE,
140-
$foundVar,
141-
null,
142-
false,
143-
null,
144-
false
145-
);
146-
if ($propertyNamePosition === false) {
147-
return;
148-
};
163+
$tokens = $phpcsFile->getTokens();
149164
$propertyName = trim($tokens[$propertyNamePosition]['content'], '$');
150-
$shortDescription = strtolower($tokens[$isShortDescriptionPreviousVar]['content']);
165+
$shortDescription = strtolower($tokens[$propertyShortDescriptionPosition]['content']);
151166

152-
if ($shortDescription === strtolower($propertyName)) {
153-
$error = 'Short description duplicates class property name.';
154-
$phpcsFile->addWarning($error, $isShortDescriptionPreviousVar, 'AlreadyHaveMeaningfulNameVar');
155-
return;
167+
if ($this->isDuplicate($propertyName, $shortDescription)) {
168+
$phpcsFile->addWarning(
169+
'Short description duplicates class property name.',
170+
$propertyShortDescriptionPosition,
171+
'AlreadyHaveMeaningfulNameVar'
172+
);
156173
}
174+
}
157175

158-
$propertyNameParts = array_filter(preg_split('/(?=[A-Z])/', $propertyName));
176+
/**
177+
* Does short description duplicate the property name
178+
*
179+
* @param string $propertyName
180+
* @param string $shortDescription
181+
* @return bool
182+
*/
183+
private function isDuplicate(string $propertyName, string $shortDescription): bool
184+
{
185+
return $this->clean($propertyName) === $this->clean($shortDescription);
186+
}
159187

160-
if ($shortDescription === strtolower(implode(' ', $propertyNameParts))) {
161-
$error = 'Short description duplicates class property name.';
162-
$phpcsFile->addWarning($error, $isShortDescriptionPreviousVar, 'AlreadyHaveMeaningfulNameVar');
163-
}
188+
/**
189+
* Return only A-Za-z characters converted to lowercase from the string
190+
*
191+
* @param string $string
192+
* @return string
193+
*/
194+
private function clean(string $string): string
195+
{
196+
return strtolower(preg_replace('/[^A-Za-z]/', '', $string));
164197
}
165198

166199
/**

‎Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.inc

+10
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,16 @@ class correctlyFormattedClassMemberDocBlock
114114
*/
115115
protected $rulePool;
116116

117+
/**
118+
* @var \Magento\Store\Model\StoreManager
119+
*/
120+
private \Magento\Store\Model\StoreManager $decoratedStoreManager;
121+
122+
/*
123+
* @var \Magento\Eav\Model\Entity\Attribute\SetFactory
124+
*/
125+
private $attributeSetFactory;
126+
117127
/**
118128
* A description that includes test which is the same name as the variable is allowed
119129
*

‎Magento2/Tests/Commenting/ClassPropertyPHPDocFormattingUnitTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public function getWarningList()
3434
63 => 1,
3535
68 => 1,
3636
75 => 1,
37+
125 => 1
3738
];
3839
}
3940
}

0 commit comments

Comments
 (0)