mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-29 10:12:41 +01:00
Add a linter rule to prevent exceptions from being thrown in a toString method
Summary: Fixes T8207. It is not possible to throw an exception from within the `__toString()` method. Add a linter rule to prevent this from happening. Test Plan: Added test cases. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Maniphest Tasks: T8207 Differential Revision: https://secure.phabricator.com/D12854
This commit is contained in:
parent
2e66d03c68
commit
1e3073a4f8
2 changed files with 51 additions and 1 deletions
|
@ -66,6 +66,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
const LINT_NO_PARENT_SCOPE = 64;
|
||||
const LINT_ALIAS_FUNCTION = 65;
|
||||
const LINT_CAST_SPACING = 66;
|
||||
const LINT_TOSTRING_EXCEPTION = 67;
|
||||
|
||||
private $blacklistedFunctions = array();
|
||||
private $naminghook;
|
||||
|
@ -206,6 +207,8 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
=> pht('Alias Functions'),
|
||||
self::LINT_CAST_SPACING
|
||||
=> pht('Cast Spacing'),
|
||||
self::LINT_TOSTRING_EXCEPTION
|
||||
=> pht('Throwing Exception in %s Method', '__toString'),
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -326,7 +329,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
|
||||
public function getVersion() {
|
||||
// The version number should be incremented whenever a new rule is added.
|
||||
return '28';
|
||||
return '29';
|
||||
}
|
||||
|
||||
protected function resolveFuture($path, Future $future) {
|
||||
|
@ -419,6 +422,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
'lintNoParentScope' => self::LINT_NO_PARENT_SCOPE,
|
||||
'lintAliasFunctions' => self::LINT_ALIAS_FUNCTION,
|
||||
'lintCastSpacing' => self::LINT_CAST_SPACING,
|
||||
'lintThrowExceptionInToStringMethod' => self::LINT_TOSTRING_EXCEPTION,
|
||||
);
|
||||
|
||||
foreach ($method_codes as $method => $codes) {
|
||||
|
@ -4124,6 +4128,34 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
|||
}
|
||||
}
|
||||
|
||||
private function lintThrowExceptionInToStringMethod(XHPASTNode $root) {
|
||||
$methods = $root->selectDescendantsOfType('n_METHOD_DECLARATION');
|
||||
|
||||
foreach ($methods as $method) {
|
||||
$name = $method
|
||||
->getChildOfType(2, 'n_STRING')
|
||||
->getConcreteString();
|
||||
|
||||
if ($name != '__toString') {
|
||||
continue;
|
||||
}
|
||||
|
||||
$throws = $method
|
||||
->getChildOfType(5, 'n_STATEMENT_LIST')
|
||||
->selectDescendantsOfType('n_THROW');
|
||||
|
||||
foreach ($throws as $throw) {
|
||||
$this->raiseLintAtNode(
|
||||
$throw,
|
||||
self::LINT_TOSTRING_EXCEPTION,
|
||||
pht(
|
||||
'It is not possible to throw an %s from within the %s method.',
|
||||
'Exception',
|
||||
'__toString'));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Retrieve all calls to some specified function(s).
|
||||
|
|
|
@ -0,0 +1,18 @@
|
|||
<?php
|
||||
class MyClass {
|
||||
public function __toString() {
|
||||
if (some_function()) {
|
||||
throw new Exception('Oops');
|
||||
}
|
||||
|
||||
return __CLASS__;
|
||||
}
|
||||
}
|
||||
|
||||
class MyOtherClass {
|
||||
public function __toString() {
|
||||
return 'Success';
|
||||
}
|
||||
}
|
||||
~~~~~~~~~~
|
||||
error:5:7
|
Loading…
Reference in a new issue