From 34efe49e1252d64103b222a61332b24ad22cac67 Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Wed, 15 Aug 2012 12:56:36 -0700
Subject: [PATCH] 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
---
 src/lint/linter/ArcanistXHPASTLinter.php      | 48 +++++++++++++++++++
 .../__tests__/xhpast/slowness.lint-test       | 10 ++++
 2 files changed, 58 insertions(+)
 create mode 100644 src/lint/linter/__tests__/xhpast/slowness.lint-test

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index 372b7ddb..40569074 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -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) {
diff --git a/src/lint/linter/__tests__/xhpast/slowness.lint-test b/src/lint/linter/__tests__/xhpast/slowness.lint-test
new file mode 100644
index 00000000..60da4d8f
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/slowness.lint-test
@@ -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