From cae7631dff44769ecddc224d163ab8b3a186dfda Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Wed, 15 Aug 2012 13:30:40 -0700
Subject: [PATCH] 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
---
 src/lint/linter/ArcanistXHPASTLinter.php      | 43 +++++++++++++++++++
 .../__tests__/xhpast/slowness.lint-test       |  4 ++
 2 files changed, 47 insertions(+)

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index 40569074..6779f839 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -221,6 +221,49 @@ final class ArcanistXHPASTLinter extends ArcanistLinter {
     $this->lintPHP54Features($root);
     $this->lintPHT($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) {
diff --git a/src/lint/linter/__tests__/xhpast/slowness.lint-test b/src/lint/linter/__tests__/xhpast/slowness.lint-test
index 60da4d8f..6549eca3 100644
--- a/src/lint/linter/__tests__/xhpast/slowness.lint-test
+++ b/src/lint/linter/__tests__/xhpast/slowness.lint-test
@@ -4,7 +4,11 @@
 (strpos('a', 'b') !== 0);
 (0 === strpos('a', 'b'));
 (1 + 0 === strpos('a', 'b'));
+
+(strstr('a', 'b') === false);
+(strstr('a', 'b') === 'b');
 ~~~~~~~~~~
 warning:3:2
 warning:4:2
 warning:5:8
+warning:8:2