Skip to content

Commit a3a67d8

Browse files
committed
Generic/ConstructorName: bug fix - false positive on static call to other class
When determining whether or not a "parent" constructor was being called using a call to a PHP4-style constructor, the sniff would only look at the _next_ token after a double colon, disregarding completely the class the method is being called on. This would lead to false positives for method calls to other classes, which would just happen to have a method named the same as the class being extended. This commit fixes this by verifying that the previous non-empty token before the double colon is either a hierarchy keyword or the name of the class being extended and only throwing an error if that's the case. Includes tests.
1 parent 0714850 commit a3a67d8

File tree

3 files changed

+35
-4
lines changed

3 files changed

+35
-4
lines changed

src/Standards/Generic/Sniffs/NamingConventions/ConstructorNameSniff.php

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,26 @@ protected function processTokenWithinScope(File $phpcsFile, $stackPtr, $currScop
106106
$startIndex = $stackPtr;
107107
while (($doubleColonIndex = $phpcsFile->findNext(T_DOUBLE_COLON, $startIndex, $endFunctionIndex)) !== false) {
108108
$nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($doubleColonIndex + 1), null, true);
109-
if ($tokens[$nextNonEmpty]['code'] === T_STRING
110-
&& strtolower($tokens[$nextNonEmpty]['content']) === $parentClassNameLc
109+
if ($tokens[$nextNonEmpty]['code'] !== T_STRING
110+
|| strtolower($tokens[$nextNonEmpty]['content']) !== $parentClassNameLc
111+
) {
112+
$startIndex = $nextNonEmpty;
113+
continue;
114+
}
115+
116+
$prevNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($doubleColonIndex - 1), null, true);
117+
if ($tokens[$prevNonEmpty]['code'] === T_PARENT
118+
|| $tokens[$prevNonEmpty]['code'] === T_SELF
119+
|| $tokens[$prevNonEmpty]['code'] === T_STATIC
120+
|| ($tokens[$prevNonEmpty]['code'] === T_STRING
121+
&& strtolower($tokens[$prevNonEmpty]['content']) === $parentClassNameLc)
111122
) {
112123
$error = 'PHP4 style calls to parent constructors are not allowed; use "parent::__construct()" instead';
113124
$phpcsFile->addError($error, $nextNonEmpty, 'OldStyleCall');
114125
}
115126

116127
$startIndex = $nextNonEmpty;
117-
}
128+
}//end while
118129

119130
}//end processTokenWithinScope()
120131

src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.inc

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class UnconventionalSpacing extends MyParent
101101
{
102102
public function UnconventionalSpacing() {
103103
self :: MyParent();
104-
static/*comment*/::/*comment*/MyParent();
104+
MyParent/*comment*/::/*comment*/MyParent();
105105
}
106106

107107
public function __construct() {
@@ -112,3 +112,19 @@ class UnconventionalSpacing extends MyParent
112112
MyParent();
113113
}
114114
}
115+
116+
// Bug: calling a method with the same name as the class being extended on another class, should not be flagged.
117+
class HierarchyKeywordBeforeColon extends MyParent
118+
{
119+
public function HierarchyKeywordBeforeColon() {
120+
parent::MyParent();
121+
MyParent::MyParent();
122+
SomeOtherClass::MyParent();
123+
}
124+
125+
public function __construct() {
126+
self::MyParent();
127+
static::MyParent();
128+
SomeOtherClass::MyParent();
129+
}
130+
}

src/Standards/Generic/Tests/NamingConventions/ConstructorNameUnitTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ public function getErrorList()
3939
103 => 1,
4040
104 => 1,
4141
112 => 1,
42+
120 => 1,
43+
121 => 1,
44+
126 => 1,
45+
127 => 1,
4246
];
4347

4448
}//end getErrorList()

0 commit comments

Comments
 (0)