Skip to content

Commit 483de85

Browse files
committed
fix custom exception gives false positive with DirectThrowSniff
1 parent 54ca471 commit 483de85

File tree

4 files changed

+92
-13
lines changed

4 files changed

+92
-13
lines changed

Magento2/Sniffs/Exceptions/DirectThrowSniff.php

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,19 +44,29 @@ public function process(File $phpcsFile, $stackPtr)
4444
$tokens = $phpcsFile->getTokens();
4545
$endOfStatement = $phpcsFile->findEndOfStatement($stackPtr);
4646
$posOfException = $phpcsFile->findNext(T_STRING, $stackPtr, $endOfStatement);
47-
$content = $tokens[$posOfException]['content'];
48-
$exceptionClassInUseStatement = false;
47+
48+
$fullExceptionString = $this->getFullClassNameAndAlias($tokens, $stackPtr, $endOfStatement);
49+
$exceptionString = 'Exception';
50+
$customExceptionFound = false;
4951
foreach ($tokens as $key => $token) {
5052
if ($token['code'] === T_USE) {
5153
$endOfUse = $phpcsFile->findEndOfStatement($key);
52-
$posOfException = $phpcsFile->findNext(T_STRING, $key, $key + 3, false, 'Exception');
53-
if ($posOfException && $phpcsFile->findNext(T_SEMICOLON, $posOfException+1, $endOfUse + 1)) {
54-
$exceptionClassInUseStatement = true;
55-
break;
54+
$useStatementValue = $this->getFullClassNameAndAlias($tokens, $key, $endOfUse);
55+
//we safely consider use statement has alias will not be a direct exception class
56+
if (empty($useStatementValue['alias'])) {
57+
if (substr($useStatementValue['name'], 0, strlen($exceptionString)) !== $exceptionString
58+
&& substr($useStatementValue['name'], -strlen($exceptionString)) === $exceptionString
59+
&& $useStatementValue['name'] !== $exceptionString
60+
) {
61+
$customExceptionFound = true;
62+
break;
63+
}
5664
}
5765
}
5866
}
59-
if ($content === '\Exception' || ($content === 'Exception' && $exceptionClassInUseStatement)) {
67+
if (($tokens[$posOfException]['content'] === 'Exception' && !$customExceptionFound)
68+
|| $fullExceptionString['name'] === '\Exception'
69+
) {
6070
$phpcsFile->addWarning(
6171
$this->warningMessage,
6272
$stackPtr,
@@ -65,4 +75,33 @@ public function process(File $phpcsFile, $stackPtr)
6575
);
6676
}
6777
}
78+
79+
/**
80+
* Get full class name and alias
81+
*
82+
* @param array $tokens
83+
* @param int $start
84+
* @param int $end
85+
* @return array
86+
*/
87+
private function getFullClassNameAndAlias($tokens, $start, $end)
88+
{
89+
$fullName = $alias = '';
90+
$foundAlias = false;
91+
for ($i = $start; $i <= $end; $i++) {
92+
$type = $tokens[$i]['code'];
93+
if ($type === T_AS) {
94+
$foundAlias = true;
95+
continue;
96+
}
97+
if ($type === T_STRING || $type === T_NS_SEPARATOR) {
98+
if (!$foundAlias) {
99+
$fullName .= $tokens[$i]['content'];
100+
} else {
101+
$alias = $tokens[$i]['content'];
102+
}
103+
}
104+
}
105+
return ['name' => $fullName, 'alias' => $alias];
106+
}
68107
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
namespace Vendor\Module\Anyname;
3+
4+
use Vendor\Module\Anyname\Exception;
5+
6+
class Foo
7+
{
8+
protected $isEnabled = true;
9+
10+
public function go()
11+
{
12+
if (!$this->isEnabled) {
13+
throw new Exception('Action disabled.');
14+
}
15+
}
16+
17+
public function exceptionTest()
18+
{
19+
if (!$this->isEnabled) {
20+
throw new \Exception('Action disabled.');
21+
}
22+
}
23+
24+
public function localizedExceptionTest()
25+
{
26+
27+
if (!$this->isEnabled) {
28+
throw new LocalizedException('Localized exception.');
29+
}
30+
}
31+
}

Magento2/Tests/Exceptions/DirectThrowUnitTest.php

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,28 @@ class DirectThrowUnitTest extends AbstractSniffUnitTest
1212
/**
1313
* @inheritdoc
1414
*/
15-
public function getErrorList()
15+
protected function getErrorList()
1616
{
1717
return [];
1818
}
1919

2020
/**
2121
* @inheritdoc
2222
*/
23-
public function getWarningList()
23+
protected function getWarningList($testFile = '')
2424
{
25-
return [
26-
10 => 1,
27-
17 => 1,
28-
];
25+
if ($testFile === 'DirectThrowUnitTest.1.inc') {
26+
return [
27+
10 => 1,
28+
17 => 1,
29+
];
30+
} elseif ($testFile === 'DirectThrowUnitTest.2.inc') {
31+
return [
32+
20 => 1
33+
];
34+
35+
}
36+
return [];
37+
2938
}
3039
}

0 commit comments

Comments
 (0)