Skip to content

Commit f145bb4

Browse files
jrfnljrfnl
authored
Merge pull request #653 from PHPCSStandards/feature/generic-duplicateclassname-fix-bugs-performance
Generic/DuplicateClassName: 4 bug fixes + performance improvement
2 parents a22a1cb + 060338f commit f145bb4

10 files changed

+135
-44
lines changed

‎src/Standards/Generic/Sniffs/Classes/DuplicateClassNameSniff.php‎

Lines changed: 52 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use PHP_CodeSniffer\Files\File;
1313
use PHP_CodeSniffer\Sniffs\Sniff;
14+
use PHP_CodeSniffer\Util\Tokens;
1415

1516
class DuplicateClassNameSniff implements Sniff
1617
{
@@ -55,63 +56,70 @@ public function process(File $phpcsFile, $stackPtr)
5556
T_TRAIT,
5657
T_ENUM,
5758
T_NAMESPACE,
58-
T_CLOSE_TAG,
5959
];
6060

6161
$stackPtr = $phpcsFile->findNext($findTokens, ($stackPtr + 1));
6262
while ($stackPtr !== false) {
63-
if ($tokens[$stackPtr]['code'] === T_CLOSE_TAG) {
64-
// We can stop here. The sniff will continue from the next open
65-
// tag when PHPCS reaches that token, if there is one.
66-
return;
67-
}
68-
6963
// Keep track of what namespace we are in.
7064
if ($tokens[$stackPtr]['code'] === T_NAMESPACE) {
71-
$nsEnd = $phpcsFile->findNext(
72-
[
73-
T_NS_SEPARATOR,
74-
T_STRING,
75-
T_WHITESPACE,
76-
],
77-
($stackPtr + 1),
78-
null,
79-
true
80-
);
81-
82-
$namespace = trim($phpcsFile->getTokensAsString(($stackPtr + 1), ($nsEnd - $stackPtr - 1)));
83-
$stackPtr = $nsEnd;
84-
} else {
85-
$nameToken = $phpcsFile->findNext(T_STRING, $stackPtr);
86-
$name = $tokens[$nameToken]['content'];
87-
if ($namespace !== '') {
88-
$name = $namespace.'\\'.$name;
65+
$nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true);
66+
if ($nextNonEmpty !== false
67+
// Ignore namespace keyword used as operator.
68+
&& $tokens[$nextNonEmpty]['code'] !== T_NS_SEPARATOR
69+
) {
70+
$namespace = '';
71+
for ($i = $nextNonEmpty; $i < $phpcsFile->numTokens; $i++) {
72+
if (isset(Tokens::$emptyTokens[$tokens[$i]['code']]) === true) {
73+
continue;
74+
}
75+
76+
if ($tokens[$i]['code'] !== T_STRING && $tokens[$i]['code'] !== T_NS_SEPARATOR) {
77+
break;
78+
}
79+
80+
$namespace .= $tokens[$i]['content'];
81+
}
82+
83+
$stackPtr = $i;
8984
}
90-
91-
$compareName = strtolower($name);
92-
if (isset($this->foundClasses[$compareName]) === true) {
93-
$type = strtolower($tokens[$stackPtr]['content']);
94-
$file = $this->foundClasses[$compareName]['file'];
95-
$line = $this->foundClasses[$compareName]['line'];
96-
$error = 'Duplicate %s name "%s" found; first defined in %s on line %s';
97-
$data = [
98-
$type,
99-
$name,
100-
$file,
101-
$line,
102-
];
103-
$phpcsFile->addWarning($error, $stackPtr, 'Found', $data);
104-
} else {
105-
$this->foundClasses[$compareName] = [
106-
'file' => $phpcsFile->getFilename(),
107-
'line' => $tokens[$stackPtr]['line'],
108-
];
85+
} else {
86+
$name = $phpcsFile->getDeclarationName($stackPtr);
87+
if (empty($name) === false) {
88+
if ($namespace !== '') {
89+
$name = $namespace.'\\'.$name;
90+
}
91+
92+
$compareName = strtolower($name);
93+
if (isset($this->foundClasses[$compareName]) === true) {
94+
$type = strtolower($tokens[$stackPtr]['content']);
95+
$file = $this->foundClasses[$compareName]['file'];
96+
$line = $this->foundClasses[$compareName]['line'];
97+
$error = 'Duplicate %s name "%s" found; first defined in %s on line %s';
98+
$data = [
99+
$type,
100+
$name,
101+
$file,
102+
$line,
103+
];
104+
$phpcsFile->addWarning($error, $stackPtr, 'Found', $data);
105+
} else {
106+
$this->foundClasses[$compareName] = [
107+
'file' => $phpcsFile->getFilename(),
108+
'line' => $tokens[$stackPtr]['line'],
109+
];
110+
}
111+
}//end if
112+
113+
if (isset($tokens[$stackPtr]['scope_closer']) === true) {
114+
$stackPtr = $tokens[$stackPtr]['scope_closer'];
109115
}
110116
}//end if
111117

112118
$stackPtr = $phpcsFile->findNext($findTokens, ($stackPtr + 1));
113119
}//end while
114120

121+
return $phpcsFile->numTokens;
122+
115123
}//end process()
116124

117125

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.10.inc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<?php
2+
namespace Code\IsNamespaced ?>
3+
4+
<?php
5+
class MyClass {}
6+
interface MyInterface {}

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.11.inc‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
namespace Code\IsNamespaced;
3+
class FooBar {}
4+
5+
namespace Code\AnotherNamespace;
6+
7+
namespace Code\IsNamespaced ?>
8+
9+
<?php
10+
echo '123';
11+
?>
12+
<?php
13+
class FooBar {}

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.7.inc‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
namespace E;
3+
namespace\functionCall();
4+
class MyClass {}
5+
interface YourInterface {}
6+
7+
namespace F;
8+
namespace\functionCall();
9+
class MyClass {}
10+
11+
namespace /*comment*/ G;
12+
namespace\functionCall();
13+
class MyClass {}

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.8.inc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
namespace Foo\Bar;
3+
class MyClass {}
4+
interface YourInterface {}
5+
6+
namespace Foo /*comment*/ \ /*comment*/ Bar;
7+
class MyClass {}
8+
interface YourInterface {}

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.9.inc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?php
2+
namespace {
3+
class MyClass {}
4+
trait YourTrait {}
5+
}

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.97.inc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// Intentional parse error/live coding.
4+
// This should be the only test in this file.
5+
// This is a two-file test: test case file 97 and 98 belong together.
6+
// Testing against false positives during live coding and invalid class names being stored in the cache.
7+
8+
class

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.98.inc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// Intentional parse error/live coding.
4+
// This should be the only test in this file.
5+
// This is a two-file test: test case file 97 and 98 belong together.
6+
// Testing against false positives during live coding and invalid class names being stored in the cache.
7+
8+
class

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.99.inc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
// Intentional parse error/live coding.
4+
// This should be the only test in this file.
5+
// Testing that the sniff is *not* triggered.
6+
7+
namespace

‎src/Standards/Generic/Tests/Classes/DuplicateClassNameUnitTest.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,21 @@ public function getWarningList($testFile='')
7373
case 'DuplicateClassNameUnitTest.6.inc':
7474
return [10 => 1];
7575

76+
case 'DuplicateClassNameUnitTest.8.inc':
77+
return [
78+
7 => 1,
79+
8 => 1,
80+
];
81+
82+
case 'DuplicateClassNameUnitTest.9.inc':
83+
return [
84+
3 => 1,
85+
4 => 1,
86+
];
87+
88+
case 'DuplicateClassNameUnitTest.11.inc':
89+
return [13 => 1];
90+
7691
default:
7792
return [];
7893
}//end switch

0 commit comments

Comments
 (0)