mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-25 16:22:42 +01:00
Warn about strstr() misuse
Summary: See D3296#1. Test Plan: New test. Linted Arcanist, libphutil, Phabricator repositories, found no false positive and one real positive. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3297
This commit is contained in:
parent
34efe49e12
commit
cae7631dff
2 changed files with 47 additions and 0 deletions
|
@ -221,6 +221,49 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
|
||||||
$this->lintPHP54Features($root);
|
$this->lintPHP54Features($root);
|
||||||
$this->lintPHT($root);
|
$this->lintPHT($root);
|
||||||
$this->lintStrposUsedForStart($root);
|
$this->lintStrposUsedForStart($root);
|
||||||
|
$this->lintStrstrUsedForCheck($root);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function lintStrstrUsedForCheck($root) {
|
||||||
|
$expressions = $root->selectDescendantsOfType('n_BINARY_EXPRESSION');
|
||||||
|
foreach ($expressions as $expression) {
|
||||||
|
$operator = $expression->getChildOfType(1, 'n_OPERATOR');
|
||||||
|
$operator = $operator->getConcreteString();
|
||||||
|
|
||||||
|
if ($operator != '===' && $operator != '!==') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$false = $expression->getChildByIndex(0);
|
||||||
|
if ($false->getTypeName() == 'n_SYMBOL_NAME' &&
|
||||||
|
$false->getConcreteString() == 'false') {
|
||||||
|
$strstr = $expression->getChildByIndex(2);
|
||||||
|
} else {
|
||||||
|
$strstr = $false;
|
||||||
|
$false = $expression->getChildByIndex(2);
|
||||||
|
if ($false->getTypeName() != 'n_SYMBOL_NAME' ||
|
||||||
|
$false->getConcreteString() != 'false') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($strstr->getTypeName() != 'n_FUNCTION_CALL') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$name = strtolower($strstr->getChildByIndex(0)->getConcreteString());
|
||||||
|
if ($name == 'strstr' || $name == 'strchr') {
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$strstr,
|
||||||
|
self::LINT_SLOWNESS,
|
||||||
|
"Use strpos() for checking if the string contains something.");
|
||||||
|
} else if ($name == 'stristr') {
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$strstr,
|
||||||
|
self::LINT_SLOWNESS,
|
||||||
|
"Use stripos() for checking if the string contains something.");
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public function lintStrposUsedForStart($root) {
|
public function lintStrposUsedForStart($root) {
|
||||||
|
|
|
@ -4,7 +4,11 @@
|
||||||
(strpos('a', 'b') !== 0);
|
(strpos('a', 'b') !== 0);
|
||||||
(0 === strpos('a', 'b'));
|
(0 === strpos('a', 'b'));
|
||||||
(1 + 0 === strpos('a', 'b'));
|
(1 + 0 === strpos('a', 'b'));
|
||||||
|
|
||||||
|
(strstr('a', 'b') === false);
|
||||||
|
(strstr('a', 'b') === 'b');
|
||||||
~~~~~~~~~~
|
~~~~~~~~~~
|
||||||
warning:3:2
|
warning:3:2
|
||||||
warning:4:2
|
warning:4:2
|
||||||
warning:5:8
|
warning:5:8
|
||||||
|
warning:8:2
|
||||||
|
|
Loading…
Reference in a new issue