mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-26 00:32:41 +01:00
Warn before using strpos($a, $b) === 0
Summary: I'm not sure if we are interested in this kind of linters ("Don't use slow function if there is a fast alternative"). I didn't find a case where `strpos() === 0` could be useful except [[ http://www.php.net/manual/en/mbstring.overload.php | mbstring.func_overload ]] craziness. Test Plan: New unit test. Linted Arcanist, libphutil, Phabricator repositories, found no false positive and one real positive. Reviewers: btrahan, epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3296
This commit is contained in:
parent
390bb280e1
commit
34efe49e12
2 changed files with 58 additions and 0 deletions
|
@ -59,6 +59,7 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
|
|||
const LINT_PHT_WITH_DYNAMIC_STRING = 33;
|
||||
const LINT_COMMENT_SPACING = 34;
|
||||
const LINT_PHP_54_FEATURES = 35;
|
||||
const LINT_SLOWNESS = 36;
|
||||
|
||||
|
||||
public function getLintNameMap() {
|
||||
|
@ -97,6 +98,7 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
|
|||
self::LINT_REUSED_AS_ITERATOR => 'Variable Reused As Iterator',
|
||||
self::LINT_PHT_WITH_DYNAMIC_STRING => 'Use of pht() on Dynamic String',
|
||||
self::LINT_COMMENT_SPACING => 'Comment Spaces',
|
||||
self::LINT_SLOWNESS => 'Slow Construct',
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -127,6 +129,8 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
|
|||
=> ArcanistLintSeverity::SEVERITY_WARNING,
|
||||
self::LINT_PHT_WITH_DYNAMIC_STRING
|
||||
=> ArcanistLintSeverity::SEVERITY_WARNING,
|
||||
self::LINT_SLOWNESS
|
||||
=> ArcanistLintSeverity::SEVERITY_WARNING,
|
||||
|
||||
self::LINT_COMMENT_SPACING
|
||||
=> ArcanistLintSeverity::SEVERITY_ADVICE,
|
||||
|
@ -216,6 +220,50 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
|
|||
$this->lintPHP53Features($root);
|
||||
$this->lintPHP54Features($root);
|
||||
$this->lintPHT($root);
|
||||
$this->lintStrposUsedForStart($root);
|
||||
}
|
||||
|
||||
public function lintStrposUsedForStart($root) {
|
||||
$expressions = $root->selectDescendantsOfType('n_BINARY_EXPRESSION');
|
||||
foreach ($expressions as $expression) {
|
||||
$operator = $expression->getChildOfType(1, 'n_OPERATOR');
|
||||
$operator = $operator->getConcreteString();
|
||||
|
||||
if ($operator != '===' && $operator != '!==') {
|
||||
continue;
|
||||
}
|
||||
|
||||
$zero = $expression->getChildByIndex(0);
|
||||
if ($zero->getTypeName() == 'n_NUMERIC_SCALAR' &&
|
||||
$zero->getConcreteString() == '0') {
|
||||
$strpos = $expression->getChildByIndex(2);
|
||||
} else {
|
||||
$strpos = $zero;
|
||||
$zero = $expression->getChildByIndex(2);
|
||||
if ($zero->getTypeName() != 'n_NUMERIC_SCALAR' ||
|
||||
$zero->getConcreteString() != '0') {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
if ($strpos->getTypeName() != 'n_FUNCTION_CALL') {
|
||||
continue;
|
||||
}
|
||||
|
||||
$name = strtolower($strpos->getChildByIndex(0)->getConcreteString());
|
||||
if ($name == 'strpos') {
|
||||
$this->raiseLintAtNode(
|
||||
$strpos,
|
||||
self::LINT_SLOWNESS,
|
||||
"Use strncmp() for checking if the string starts with something.");
|
||||
} else if ($name == 'stripos') {
|
||||
$this->raiseLintAtNode(
|
||||
$strpos,
|
||||
self::LINT_SLOWNESS,
|
||||
"Use strncasecmp() for checking if the string starts with ".
|
||||
"something.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public function lintPHT($root) {
|
||||
|
|
10
src/lint/linter/__tests__/xhpast/slowness.lint-test
Normal file
10
src/lint/linter/__tests__/xhpast/slowness.lint-test
Normal file
|
@ -0,0 +1,10 @@
|
|||
<?php
|
||||
|
||||
(strpos('a', 'b') === 0);
|
||||
(strpos('a', 'b') !== 0);
|
||||
(0 === strpos('a', 'b'));
|
||||
(1 + 0 === strpos('a', 'b'));
|
||||
~~~~~~~~~~
|
||||
warning:3:2
|
||||
warning:4:2
|
||||
warning:5:8
|
Loading…
Reference in a new issue