mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-24 15:52:40 +01:00
Add a lint check for catching "Exception" without catching "Throwable"
Summary: Ref T13588. For consistency of behavior between versions on either side of PHP7, any "catch (Exception)" block should generally have a "catch (Throwable)" block in the same catch list. Raise a lint warning when "Exception" is caught without also catching "Throwable", since this is almost certainly not desired. Test Plan: Added tests. Maniphest Tasks: T13588 Differential Revision: https://secure.phabricator.com/D21542
This commit is contained in:
parent
c1afa91f9f
commit
32fe933f3a
4 changed files with 76 additions and 0 deletions
|
@ -402,6 +402,8 @@ phutil_register_library_map(array(
|
||||||
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParenthesesSpacingXHPASTLinterRuleTestCase.php',
|
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParenthesesSpacingXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php',
|
'ArcanistParseStrUseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistParseStrUseXHPASTLinterRule.php',
|
||||||
'ArcanistParseStrUseXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParseStrUseXHPASTLinterRuleTestCase.php',
|
'ArcanistParseStrUseXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistParseStrUseXHPASTLinterRuleTestCase.php',
|
||||||
|
'ArcanistPartialCatchXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPartialCatchXHPASTLinterRule.php',
|
||||||
|
'ArcanistPartialCatchXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPartialCatchXHPASTLinterRuleTestCase.php',
|
||||||
'ArcanistPasteRef' => 'ref/paste/ArcanistPasteRef.php',
|
'ArcanistPasteRef' => 'ref/paste/ArcanistPasteRef.php',
|
||||||
'ArcanistPasteSymbolRef' => 'ref/paste/ArcanistPasteSymbolRef.php',
|
'ArcanistPasteSymbolRef' => 'ref/paste/ArcanistPasteSymbolRef.php',
|
||||||
'ArcanistPasteWorkflow' => 'workflow/ArcanistPasteWorkflow.php',
|
'ArcanistPasteWorkflow' => 'workflow/ArcanistPasteWorkflow.php',
|
||||||
|
@ -1451,6 +1453,8 @@ phutil_register_library_map(array(
|
||||||
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistParenthesesSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
'ArcanistParseStrUseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
'ArcanistParseStrUseXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
'ArcanistParseStrUseXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
|
'ArcanistPartialCatchXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
|
||||||
|
'ArcanistPartialCatchXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
|
||||||
'ArcanistPasteRef' => 'ArcanistRef',
|
'ArcanistPasteRef' => 'ArcanistRef',
|
||||||
'ArcanistPasteSymbolRef' => 'ArcanistSimpleSymbolRef',
|
'ArcanistPasteSymbolRef' => 'ArcanistSimpleSymbolRef',
|
||||||
'ArcanistPasteWorkflow' => 'ArcanistArcWorkflow',
|
'ArcanistPasteWorkflow' => 'ArcanistArcWorkflow',
|
||||||
|
|
|
@ -0,0 +1,45 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistPartialCatchXHPASTLinterRule
|
||||||
|
extends ArcanistXHPASTLinterRule {
|
||||||
|
|
||||||
|
const ID = 132;
|
||||||
|
|
||||||
|
public function getLintName() {
|
||||||
|
return pht('Partial Catch');
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getLintSeverity() {
|
||||||
|
return ArcanistLintSeverity::SEVERITY_WARNING;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function process(XHPASTNode $root) {
|
||||||
|
$catch_lists = $root->selectDescendantsOfType('n_CATCH_LIST');
|
||||||
|
|
||||||
|
foreach ($catch_lists as $catch_list) {
|
||||||
|
$catches = $catch_list->getChildrenOfType('n_CATCH');
|
||||||
|
|
||||||
|
$classes = array();
|
||||||
|
foreach ($catches as $catch) {
|
||||||
|
$class_node = $catch->getChildOfType(0, 'n_CLASS_NAME');
|
||||||
|
$class_name = $class_node->getConcreteString();
|
||||||
|
$class_name = phutil_utf8_strtolower($class_name);
|
||||||
|
|
||||||
|
$classes[$class_name] = $class_node;
|
||||||
|
}
|
||||||
|
|
||||||
|
$catches_exception = idx($classes, 'exception');
|
||||||
|
$catches_throwable = idx($classes, 'throwable');
|
||||||
|
|
||||||
|
if ($catches_exception && !$catches_throwable) {
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$catches_exception,
|
||||||
|
pht(
|
||||||
|
'Try/catch block catches "Exception", but does not catch '.
|
||||||
|
'"Throwable". In PHP7 and newer, some runtime exceptions '.
|
||||||
|
'will escape this block.'));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -0,0 +1,10 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
final class ArcanistPartialCatchXHPASTLinterRuleTestCase
|
||||||
|
extends ArcanistXHPASTLinterRuleTestCase {
|
||||||
|
|
||||||
|
public function testLinter() {
|
||||||
|
$this->executeTestsInDirectory(dirname(__FILE__).'/partial-catch/');
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
|
@ -0,0 +1,17 @@
|
||||||
|
<?php
|
||||||
|
try {
|
||||||
|
f();
|
||||||
|
} catch (Exception $ex) {
|
||||||
|
g();
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
f();
|
||||||
|
} catch (Exception $ex) {
|
||||||
|
g();
|
||||||
|
} catch (Throwable $ex) {
|
||||||
|
h();
|
||||||
|
}
|
||||||
|
|
||||||
|
~~~~~~~~~~
|
||||||
|
warning:4:10:XHP132
|
Loading…
Reference in a new issue