Skip to content

Commit 8bc679b

Browse files
m2-github-servicesrmsundar1svera
authored
[Imported] Fix DirectThrowSniff false positive (#137)
* Fix DirectThrowSniff false positive * fix custom exception gives false positive with DirectThrowSniff * fix coding standard * Apply suggestions from code review Co-authored-by: Sergio Vera <[email protected]> * Fix error Co-authored-by: MeenakshiSundaram <[email protected]> Co-authored-by: Sergio Vera <[email protected]>
1 parent 3230fbb commit 8bc679b

File tree

4 files changed

+97
-7
lines changed

4 files changed

+97
-7
lines changed

Magento2/Sniffs/Exceptions/DirectThrowSniff.php

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,30 @@ public function process(File $phpcsFile, $stackPtr)
4444
$tokens = $phpcsFile->getTokens();
4545
$endOfStatement = $phpcsFile->findEndOfStatement($stackPtr);
4646
$posOfException = $phpcsFile->findNext(T_STRING, $stackPtr, $endOfStatement);
47-
if ($tokens[$posOfException]['content'] === 'Exception') {
47+
48+
$fullExceptionString = $this->getFullClassNameAndAlias($tokens, $stackPtr, $endOfStatement);
49+
$exceptionString = 'Exception';
50+
$customExceptionFound = false;
51+
foreach ($tokens as $key => $token) {
52+
if ($token['code'] !== T_USE) {
53+
continue;
54+
}
55+
$endOfUse = $phpcsFile->findEndOfStatement($key);
56+
$useStatementValue = $this->getFullClassNameAndAlias($tokens, $key, $endOfUse);
57+
//we safely consider use statement has alias will not be a direct exception class
58+
if (empty($useStatementValue['alias'])) {
59+
if (substr($useStatementValue['name'], 0, strlen($exceptionString)) !== $exceptionString
60+
&& substr($useStatementValue['name'], -strlen($exceptionString)) === $exceptionString
61+
&& $useStatementValue['name'] !== $exceptionString
62+
) {
63+
$customExceptionFound = true;
64+
break;
65+
}
66+
}
67+
}
68+
if (($tokens[$posOfException]['content'] === 'Exception' && !$customExceptionFound)
69+
|| $fullExceptionString['name'] === '\Exception'
70+
) {
4871
$phpcsFile->addWarning(
4972
$this->warningMessage,
5073
$stackPtr,
@@ -53,4 +76,33 @@ public function process(File $phpcsFile, $stackPtr)
5376
);
5477
}
5578
}
79+
80+
/**
81+
* Get full class name and alias
82+
*
83+
* @param array $tokens
84+
* @param int $start
85+
* @param int $end
86+
* @return array
87+
*/
88+
private function getFullClassNameAndAlias($tokens, $start, $end): array
89+
{
90+
$fullName = $alias = '';
91+
$foundAlias = false;
92+
for ($i = $start; $i <= $end; $i++) {
93+
$type = $tokens[$i]['code'];
94+
if ($type === T_AS) {
95+
$foundAlias = true;
96+
continue;
97+
}
98+
if ($type === T_STRING || $type === T_NS_SEPARATOR) {
99+
if (!$foundAlias) {
100+
$fullName .= $tokens[$i]['content'];
101+
} else {
102+
$alias = $tokens[$i]['content'];
103+
}
104+
}
105+
}
106+
return ['name' => $fullName, 'alias' => $alias];
107+
}
56108
}
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: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,26 @@ 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+
return [];
2936
}
3037
}

0 commit comments

Comments
 (0)