mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-25 08:12:40 +01:00
Add PHP lint check for expressions which can be statically evaluated
Summary: This is pretty coarse and could be refined, but I often do this when testing: lang=diff - if (some_complicated_condition()) + if (true || some_complicated_condition()) aran has caught me not only doing it but sending out diffs with it like 30 times. Catch it in lint instead. Test Plan: Unit test, added a "true || $junk" to the code and linted it. Reviewed By: aran Reviewers: aran, jungejason, tuomaspelkonen, pad CC: aran Differential Revision: 447
This commit is contained in:
parent
42c6f00315
commit
65c0d07935
2 changed files with 62 additions and 10 deletions
|
@ -172,12 +172,14 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
|
|||
'>' => true,
|
||||
);
|
||||
|
||||
static $logical = array(
|
||||
'||' => true,
|
||||
'&&' => true,
|
||||
);
|
||||
|
||||
foreach ($expressions as $expr) {
|
||||
$operator = $expr->getChildByIndex(1)->getConcreteString();
|
||||
if (empty($operators[$operator])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!empty($operators[$operator])) {
|
||||
$left = $expr->getChildByIndex(0)->getSemanticString();
|
||||
$right = $expr->getChildByIndex(2)->getSemanticString();
|
||||
|
||||
|
@ -189,7 +191,50 @@ class ArcanistXHPASTLinter extends ArcanistLinter {
|
|||
'evaluates to a constant.');
|
||||
}
|
||||
}
|
||||
|
||||
if (!empty($logical[$operator])) {
|
||||
$left = $expr->getChildByIndex(0)->getSemanticString();
|
||||
$right = $expr->getChildByIndex(2)->getSemanticString();
|
||||
|
||||
// NOTE: These will be null to indicate "could not evaluate".
|
||||
$left = $this->evaluateStaticBoolean($left);
|
||||
$right = $this->evaluateStaticBoolean($right);
|
||||
|
||||
if (($operator == '||' && ($left === true || $right === true)) ||
|
||||
($operator == '&&' && ($left === false || $right === false))) {
|
||||
$this->raiseLintAtNode(
|
||||
$expr,
|
||||
self::LINT_TAUTOLOGICAL_EXPRESSION,
|
||||
'The logical value of this expression is static. Did you forget '.
|
||||
'to remove some debugging code?');
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Statically evaluate a boolean value from an XHP tree.
|
||||
*
|
||||
* TODO: Improve this and move it to XHPAST proper?
|
||||
*
|
||||
* @param string The "semantic string" of a single value.
|
||||
* @return mixed ##true## or ##false## if the value could be evaluated
|
||||
* statically; ##null## if static evaluation was not possible.
|
||||
*/
|
||||
private function evaluateStaticBoolean($string) {
|
||||
switch (strtolower($string)) {
|
||||
case '0':
|
||||
case 'null':
|
||||
case 'false':
|
||||
return false;
|
||||
case '1':
|
||||
case 'true':
|
||||
return true;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
protected function lintHashComments($root) {
|
||||
$tokens = $root->getTokens();
|
||||
|
|
|
@ -15,7 +15,14 @@ if ($x == $y) {
|
|||
// See xhpast 0.54 -> 0.55.
|
||||
return $a->sub->value < $b->sub->value;
|
||||
|
||||
$skip_cache = true || $some_complicated_expression;
|
||||
$skip_cache = $a || $b;
|
||||
$skip_cache = false && something();
|
||||
$skip_cache = f();
|
||||
|
||||
~~~~~~~~~~
|
||||
error:3:5
|
||||
error:6:5
|
||||
error:9:5
|
||||
error:18:15
|
||||
error:20:15
|
||||
|
|
Loading…
Reference in a new issue