From aae098c24a2c588ff195427f3404b9d679fd9eba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20M=C3=BCller?= <thomas.mueller@tmit.eu>
Date: Tue, 5 May 2015 12:59:33 +0200
Subject: [PATCH 1/2] Check usage of != and == - refs #16054

---
 lib/private/app/codechecker.php                |  1 +
 lib/private/app/codecheckvisitor.php           | 17 +++++++++++++++++
 tests/data/app/code-checker/test-equal.php     | 11 +++++++++++
 tests/data/app/code-checker/test-not-equal.php | 11 +++++++++++
 tests/lib/app/codechecker.php                  |  8 ++++++--
 5 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/app/code-checker/test-equal.php
 create mode 100644 tests/data/app/code-checker/test-not-equal.php

diff --git a/lib/private/app/codechecker.php b/lib/private/app/codechecker.php
index 75db9ab356..918d04a0bd 100644
--- a/lib/private/app/codechecker.php
+++ b/lib/private/app/codechecker.php
@@ -42,6 +42,7 @@ class CodeChecker extends BasicEmitter {
 	const STATIC_CALL_NOT_ALLOWED = 1002;
 	const CLASS_CONST_FETCH_NOT_ALLOWED = 1003;
 	const CLASS_NEW_FETCH_NOT_ALLOWED =  1004;
+	const OP_OPERATOR_USAGE_DISCOURAGED =  1005;
 
 	/** @var Parser */
 	private $parser;
diff --git a/lib/private/app/codecheckvisitor.php b/lib/private/app/codecheckvisitor.php
index c8f383c049..03b238096e 100644
--- a/lib/private/app/codecheckvisitor.php
+++ b/lib/private/app/codecheckvisitor.php
@@ -44,6 +44,22 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
 	public $errors = [];
 
 	public function enterNode(Node $node) {
+		if ($node instanceof Node\Expr\BinaryOp\Equal) {
+			$this->errors[]= [
+				'disallowedToken' => '==',
+				'errorCode' => CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED,
+				'line' => $node->getLine(),
+				'reason' => $this->buildReason('==', CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED)
+			];
+		}
+		if ($node instanceof Node\Expr\BinaryOp\NotEqual) {
+			$this->errors[]= [
+				'disallowedToken' => '!=',
+				'errorCode' => CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED,
+				'line' => $node->getLine(),
+				'reason' => $this->buildReason('!=', CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED)
+			];
+		}
 		if ($node instanceof Node\Stmt\Class_) {
 			if (!is_null($node->extends)) {
 				$this->checkBlackList($node->extends->toString(), CodeChecker::CLASS_EXTENDS_NOT_ALLOWED, $node);
@@ -114,6 +130,7 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
 			CodeChecker::STATIC_CALL_NOT_ALLOWED => "static method call on private class",
 			CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED => "used to fetch a const from",
 			CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED => "is instanciated",
+			CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED => "is discouraged"
 		];
 
 		if (isset($errorMessages[$errorCode])) {
diff --git a/tests/data/app/code-checker/test-equal.php b/tests/data/app/code-checker/test-equal.php
new file mode 100644
index 0000000000..90543ba725
--- /dev/null
+++ b/tests/data/app/code-checker/test-equal.php
@@ -0,0 +1,11 @@
+<?php
+
+/**
+ * Class BadClass - uses equal instead of identical operator
+ */
+class BadClass {
+	public function foo() {
+		if (true == false) {
+		}
+	}
+}
diff --git a/tests/data/app/code-checker/test-not-equal.php b/tests/data/app/code-checker/test-not-equal.php
new file mode 100644
index 0000000000..d9a8d1c25c
--- /dev/null
+++ b/tests/data/app/code-checker/test-not-equal.php
@@ -0,0 +1,11 @@
+<?php
+
+/**
+ * Class BadClass - uses equal instead of identical operator
+ */
+class BadClass {
+	public function foo() {
+		if (true != false) {
+		}
+	}
+}
diff --git a/tests/lib/app/codechecker.php b/tests/lib/app/codechecker.php
index 64403fd0f2..5f4fe162c1 100644
--- a/tests/lib/app/codechecker.php
+++ b/tests/lib/app/codechecker.php
@@ -9,12 +9,14 @@
 namespace Test\App;
 
 use OC;
+use Test\TestCase;
 
-class CodeChecker extends \Test\TestCase {
+class CodeChecker extends TestCase {
 
 	/**
 	 * @dataProvider providesFilesToCheck
-	 * @param $expectedErrors
+	 * @param $expectedErrorToken
+	 * @param $expectedErrorCode
 	 * @param $fileToVerify
 	 */
 	public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
@@ -33,6 +35,8 @@ class CodeChecker extends \Test\TestCase {
 			['OC_App', 1002, 'test-static-call.php'],
 			['OC_API', 1003, 'test-const.php'],
 			['OC_AppConfig', 1004, 'test-new.php'],
+			['==', 1005, 'test-equal.php'],
+			['!=', 1005, 'test-not-equal.php'],
 		];
 	}
 }

From 493844eda4ec955b0d12e5c84412b42ae1df3fe2 Mon Sep 17 00:00:00 2001
From: Morris Jobke <hey@morrisjobke.de>
Date: Tue, 5 May 2015 13:09:12 +0200
Subject: [PATCH 2/2] add positive tests for operator in code checker

---
 .../code-checker/test-identical-operator.php    | 13 +++++++++++++
 tests/lib/app/codechecker.php                   | 17 +++++++++++++++++
 2 files changed, 30 insertions(+)
 create mode 100644 tests/data/app/code-checker/test-identical-operator.php

diff --git a/tests/data/app/code-checker/test-identical-operator.php b/tests/data/app/code-checker/test-identical-operator.php
new file mode 100644
index 0000000000..4c7641ede8
--- /dev/null
+++ b/tests/data/app/code-checker/test-identical-operator.php
@@ -0,0 +1,13 @@
+<?php
+
+/**
+ * Class GoodClass - uses identical operator
+ */
+class GoodClass {
+	public function foo() {
+		if (true === false) {
+		}
+		if (true !== false) {
+		}
+	}
+}
diff --git a/tests/lib/app/codechecker.php b/tests/lib/app/codechecker.php
index 5f4fe162c1..f45ee02d18 100644
--- a/tests/lib/app/codechecker.php
+++ b/tests/lib/app/codechecker.php
@@ -39,4 +39,21 @@ class CodeChecker extends TestCase {
 			['!=', 1005, 'test-not-equal.php'],
 		];
 	}
+
+	/**
+	 * @dataProvider validFilesData
+	 * @param $fileToVerify
+	 */
+	public function testPassValidUsage($fileToVerify) {
+		$checker = new OC\App\CodeChecker();
+		$errors = $checker->analyseFile(OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+
+		$this->assertEquals(0, count($errors));
+	}
+
+	public function validFilesData() {
+		return [
+			['test-identical-operator.php'],
+		];
+	}
 }