Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit cb944f9

Browse files
author
Lars Roettig
committedMar 4, 2019
Merge remote-tracking branch 'origin/develop' into 22-api-abstarct
2 parents 4f2fd32 + c287499 commit cb944f9

File tree

7 files changed

+355
-5
lines changed

7 files changed

+355
-5
lines changed
 

‎Magento/Sniffs/Exceptions/ThrowCatchSniff.php

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
<?php
2+
/**
3+
* Copyright © Magento. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Sniffs\Exceptions;
8+
9+
use function array_slice;
10+
use PHP_CodeSniffer\Sniffs\Sniff;
11+
use PHP_CodeSniffer\Files\File;
12+
13+
/**
14+
* Detects exceptions must not be handled in same function
15+
*/
16+
class ThrowCatchSniff implements Sniff
17+
{
18+
/**
19+
* String representation of warning.
20+
*
21+
* @var string
22+
*/
23+
protected $warningMessage = 'Exceptions must not be handled in the same function where they are thrown.';
24+
25+
/**
26+
* Warning violation code.
27+
*
28+
* @var string
29+
*/
30+
protected $warningCode = 'ThrowCatch';
31+
32+
/**
33+
* @inheritDoc
34+
*/
35+
public function register()
36+
{
37+
return [T_FUNCTION, T_CLOSURE];
38+
}
39+
40+
/**
41+
* @inheritDoc
42+
*/
43+
public function process(File $phpcsFile, $stackPtr)
44+
{
45+
$tokens = $phpcsFile->getTokens();
46+
if (!isset($tokens[$stackPtr]['scope_closer'])) {
47+
// Probably an interface method no check
48+
return;
49+
}
50+
51+
$closeBrace = $tokens[$stackPtr]['scope_closer'];
52+
$throwTags = [];
53+
$catchTags = [];
54+
55+
for ($i = $stackPtr; $i < $closeBrace; $i++) {
56+
$token = $tokens[$i];
57+
if ($token['code'] === T_CATCH) {
58+
$catchTags[] = $token;
59+
}
60+
if ($token['code'] === T_THROW) {
61+
$throwTags[] = $i;
62+
}
63+
}
64+
65+
if (count($catchTags) === 0 || count($throwTags) === 0) {
66+
// No catch or throw found no check
67+
return;
68+
}
69+
70+
$catchClassNames = [];
71+
$throwClassNames = [];
72+
73+
// find all relevant classes in catch
74+
foreach ($catchTags as $catchTag) {
75+
$start = $catchTag['parenthesis_opener'];
76+
$end = $catchTag['parenthesis_closer'];
77+
78+
$match = $phpcsFile->findNext(T_STRING, $start, $end);
79+
$catchClassNames[$match] = $tokens[$match]['content'];
80+
}
81+
82+
// find all relevant classes in throws
83+
foreach ($throwTags as $throwTag) {
84+
$match = $phpcsFile->findNext(T_STRING, $throwTag);
85+
$throwClassNames[] = $tokens[$match]['content'];
86+
}
87+
88+
$throwClassNames = array_flip($throwClassNames);
89+
foreach ($catchClassNames as $match => $catchClassName) {
90+
if (array_key_exists($catchClassName, $throwClassNames)) {
91+
$phpcsFile->addWarning($this->warningMessage, $match, $this->warningCode);
92+
}
93+
}
94+
}
95+
}

‎Magento/Sniffs/Files/LineLengthSniff.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@ class LineLengthSniff extends FilesLineLengthSniff
1919
*/
2020
protected $previousLineContent = '';
2121

22+
/**
23+
* @inheritdoc
24+
*/
25+
public $lineLimit = 120;
26+
27+
/**
28+
* @inheritdoc
29+
*/
30+
public $absoluteLineLimit = 120;
31+
2232
/**
2333
* @inheritdoc
2434
*/

‎Magento/Tests/Exceptions/ThrowCatchUnitTest.inc

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
<?php
2+
3+
namespace Foo\Bar;
4+
5+
use ArithmeticError;
6+
use BadFunctionCallException;
7+
use Exception;
8+
use LogicException;
9+
10+
abstract class Calculator
11+
{
12+
abstract public function complexNotRelevantToCheck(array $data);
13+
}
14+
15+
class CustomFooException extends Exception {}
16+
17+
class Foo extends Calculator
18+
{
19+
20+
/**
21+
* @var CallulationService
22+
*/
23+
private $calculationService;
24+
25+
public function __construct(CallulationService $calculationService)
26+
{
27+
$this->calculationService = $calculationService;
28+
}
29+
30+
/**
31+
* @param array $data
32+
* @return array|mixed
33+
*/
34+
private function vaildateFunction(array $data)
35+
{
36+
try {
37+
if (!isset($data['formData'])) {
38+
throw new CustomFooException('Bar is not set.');
39+
}
40+
return $data['formData'];
41+
} catch (CustomFooException $exception) {
42+
// Rule find: Exceptions MUST NOT handled in same function
43+
// handle $exception code
44+
}
45+
46+
return [];
47+
}
48+
49+
/**
50+
* @param array $data
51+
* @return int
52+
*/
53+
public function complexNotRelevantToCheck(array $data)
54+
{
55+
$result = $this->vaildateFunction($data);
56+
$previous = 1;
57+
58+
foreach ($result as $number) {
59+
if ($number === 0) {
60+
throw new LogicException('Cant not deviated by null');
61+
}
62+
63+
$previous /= $number;
64+
65+
// more complex code
66+
67+
if ($previous === 0) {
68+
throw new ArithmeticError('Calculation error');
69+
}
70+
71+
// more complex code
72+
73+
$result = $this->calculationService->call($previous);
74+
75+
if ($result === false) {
76+
throw new BadFunctionCallException('Cant not reach calculationService');
77+
}
78+
}
79+
80+
return (int)$previous;
81+
}
82+
83+
/**
84+
* @param array $data
85+
* @return int
86+
* @throws BadFunctionCallException
87+
*/
88+
public function complexDivisionFunction(array $data)
89+
{
90+
try {
91+
try {
92+
93+
$result = $this->vaildateFunction($data);
94+
$previous = 1;
95+
96+
foreach ($result as $number) {
97+
if ($number === 0) {
98+
throw new LogicException('Cant not deviated by null');
99+
}
100+
101+
$previous /= $number;
102+
103+
// more complex code
104+
105+
if ($previous === 0) {
106+
throw new ArithmeticError('Calculation error');
107+
}
108+
109+
// more complex code
110+
111+
$result = $this->calculationService->call($previous);
112+
113+
if ($result === false) {
114+
throw new BadFunctionCallException('Cant not reach calculationService');
115+
}
116+
}
117+
118+
return (int)$previous;
119+
120+
} catch (LogicException $logicException) {
121+
// Rule find: Exceptions MUST NOT handled in same function
122+
// handle $exception code
123+
} catch (CustomFooException $exception) {
124+
// handle $exception code
125+
}
126+
} catch (ArithmeticError $arithmeticError) {
127+
// Rule find: Exceptions MUST NOT handled in same function
128+
// handle $exception code
129+
}
130+
131+
return 0;
132+
}
133+
}
134+
135+
136+
// bad logic function in .phtml
137+
138+
function formatFunction(array $data)
139+
{
140+
try {
141+
if (!isset($data['formData'])) {
142+
throw new CustomFooException('Bar is not set.');
143+
}
144+
return $data['formData'];
145+
} catch (CustomFooException $exception) {
146+
// Rule find: Exceptions MUST NOT handled in same function
147+
// handle $exception code
148+
}
149+
150+
return [];
151+
}
152+
153+
$d = function ($data) {
154+
try {
155+
throw new CustomFooException('Bar is not set.');
156+
} catch (CustomFooException $exception) {
157+
// Rule find: Exceptions MUST NOT handled in same function
158+
// handle $exception code
159+
}
160+
};

‎Magento/Tests/Exceptions/ThrowCatchUnitTest.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
/**
3+
* Copyright © Magento. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Tests\Exceptions;
8+
9+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
10+
11+
/**
12+
* Class ThrowCatchUnitTest
13+
*/
14+
class ThrowCatchUnitTest extends AbstractSniffUnitTest
15+
{
16+
/**
17+
* @inheritdoc
18+
*/
19+
protected function getErrorList()
20+
{
21+
return [];
22+
}
23+
24+
/**
25+
* @inheritdoc
26+
*/
27+
protected function getWarningList()
28+
{
29+
return [
30+
41 => 1,
31+
120 => 1,
32+
126 => 1,
33+
145 => 1,
34+
156 => 1
35+
];
36+
}
37+
}

‎Magento/Tests/Files/LineLengthUnitTest.inc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
// allowed
4+
$shortString = 'This is a short line';
5+
6+
// triggers an error
7+
$tooLongString = 'This is a pretty long line. The code sniffer will report an error if a single line exceeds the maximum character count. You have need to insert a line break to avoid this error.';
8+
9+
// allowed
10+
$longStringBrokenUp = 'This is a pretty long line. The code sniffer would report an error if this was written as a ' .
11+
'single line. You have to insert a line break to avoid this error.';
12+
13+
// allowed by Magento Standard (Generic ruleset would trigger warning / error)
14+
$test = '012344567890123445678901234456789012344567890123445678901234456789';
15+
$test1 = '01234456789012344567890123445678901234456789012344567890123445678901234456789';
16+
$test2 = '012344567890123445678901234456789012344567890123445678901234456789012344567890123445678901234456789';

‎Magento/Tests/Files/LineLengthUnitTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
/**
3+
* Copyright © Magento. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Tests\Files;
7+
8+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
9+
10+
/**
11+
* Class LineLengthUnitTest
12+
*/
13+
class LineLengthUnitTest extends AbstractSniffUnitTest
14+
{
15+
/**
16+
* @inheritdoc
17+
*/
18+
public function getErrorList()
19+
{
20+
return [
21+
7 => 1
22+
];
23+
}
24+
25+
/**
26+
* @inheritdoc
27+
*/
28+
public function getWarningList()
29+
{
30+
return [];
31+
}
32+
}

‎Magento/ruleset.xml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@
105105
</rule>
106106

107107
<!-- Severity 8 warnings: Magento specific code issues and design violations. -->
108+
<rule ref="Magento.Classes.AbstractApi">
109+
<severity>8</severity>
110+
<type>warning</type>
111+
</rule>
108112
<rule ref="Magento.Classes.ObjectInstantiation">
109113
<severity>8</severity>
110114
<type>warning</type>
@@ -118,10 +122,6 @@
118122
<type>warning</type>
119123
</rule>
120124
<rule ref="Magento.Files.LineLength">
121-
<properties>
122-
<property name="lineLimit" value="120"/>
123-
<property name="absoluteLineLimit" value="120"/>
124-
</properties>
125125
<severity>8</severity>
126126
<type>warning</type>
127127
</rule>
@@ -142,7 +142,7 @@
142142
<severity>8</severity>
143143
<type>warning</type>
144144
</rule>
145-
<rule ref="Magento.Classes.AbstractApi">
145+
<rule ref="Magento.Exceptions.ThrowCatch">
146146
<severity>8</severity>
147147
<type>warning</type>
148148
</rule>

0 commit comments

Comments
 (0)
Please sign in to comment.