From b11757fa23a5ddb5220f990e88bdb790d7242032 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Mon, 13 Apr 2015 07:34:17 +1000
Subject: [PATCH] Add a linter rule for logical operators

Summary:
Ref T7409. Adds `ArcanistXHPASTLinter::LINT_LOGICAL_OPERATORS` for advising:

  - `&&` in favor of `and`.
  - `||` in favor of `or`.

Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/Operators/ValidLogicalOperatorsSniff.php | Squiz_Sniffs_Operators_ValidLogicalOperatorsSniff]].

Test Plan: Added test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12376
---
 src/lint/linter/ArcanistXHPASTLinter.php      | 27 ++++++++++++++++++-
 .../xhpast/logical-operators.lint-test        | 22 +++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 src/lint/linter/__tests__/xhpast/logical-operators.lint-test

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index 7485cdae..18257e51 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -57,6 +57,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
   const LINT_UNNECESSARY_FINAL_MODIFIER = 55;
   const LINT_UNNECESSARY_SEMICOLON      = 56;
   const LINT_SELF_MEMBER_REFERENCE      = 57;
+  const LINT_LOGICAL_OPERATORS          = 58;
 
   private $blacklistedFunctions = array();
   private $naminghook;
@@ -127,6 +128,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       self::LINT_UNNECESSARY_FINAL_MODIFIER => 'Unnecessary Final Modifier',
       self::LINT_UNNECESSARY_SEMICOLON      => 'Unnecessary Semicolon',
       self::LINT_SELF_MEMBER_REFERENCE      => 'Self Member Reference',
+      self::LINT_LOGICAL_OPERATORS          => 'Logical Operators',
     );
   }
 
@@ -172,6 +174,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       self::LINT_UNNECESSARY_FINAL_MODIFIER => $advice,
       self::LINT_UNNECESSARY_SEMICOLON      => $advice,
       self::LINT_SELF_MEMBER_REFERENCE      => $advice,
+      self::LINT_LOGICAL_OPERATORS          => $advice,
     );
   }
 
@@ -239,7 +242,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
 
   public function getVersion() {
     // The version number should be incremented whenever a new rule is added.
-    return '20';
+    return '21';
   }
 
   protected function resolveFuture($path, Future $future) {
@@ -321,6 +324,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
       'lintUnnecessarySemicolons' => self::LINT_UNNECESSARY_SEMICOLON,
       'lintConstantDefinitions' => self::LINT_NAMING_CONVENTIONS,
       'lintSelfMemberReference' => self::LINT_SELF_MEMBER_REFERENCE,
+      'lintLogicalOperators' => self::LINT_LOGICAL_OPERATORS,
     );
 
     foreach ($method_codes as $method => $codes) {
@@ -3456,6 +3460,27 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
     }
   }
 
+  private function lintLogicalOperators(XHPASTNode $root) {
+    $logical_ands = $root->selectTokensOfType('T_LOGICAL_AND');
+    $logical_ors  = $root->selectTokensOfType('T_LOGICAL_OR');
+
+    foreach ($logical_ands as $logical_and) {
+      $this->raiseLintAtToken(
+        $logical_and,
+        self::LINT_LOGICAL_OPERATORS,
+        pht('Use `%s` instead of `%s`.', '&&', 'and'),
+        '&&');
+    }
+
+    foreach ($logical_ors as $logical_or) {
+      $this->raiseLintAtToken(
+        $logical_or,
+        self::LINT_LOGICAL_OPERATORS,
+        pht('Use `%s` instead of `%s`.', '||', 'or'),
+        '||');
+    }
+  }
+
   /**
    * Retrieve all calls to some specified function(s).
    *
diff --git a/src/lint/linter/__tests__/xhpast/logical-operators.lint-test b/src/lint/linter/__tests__/xhpast/logical-operators.lint-test
new file mode 100644
index 00000000..5b9dac62
--- /dev/null
+++ b/src/lint/linter/__tests__/xhpast/logical-operators.lint-test
@@ -0,0 +1,22 @@
+<?php
+
+$x = false;
+$y = true;
+
+$x and $y;
+$x && $y;
+$x or $y;
+$x || $y;
+~~~~~~~~~~
+advice:6:4
+advice:8:4
+~~~~~~~~~~
+<?php
+
+$x = false;
+$y = true;
+
+$x && $y;
+$x && $y;
+$x || $y;
+$x || $y;