Merge pull request #16065 from owncloud/check-binaryops-in-code-checker

Check usage of != and == 

6 files changed, 76 insertions, 2 deletions

diff --git a/lib/private/app/codechecker.php b/lib/private/app/codechecker.php
index 75db9ab3560..918d04a0bd6 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 c8f383c0493..03b238096e7 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 00000000000..90543ba7258
--- /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-identical-operator.php b/tests/data/app/code-checker/test-identical-operator.php
new file mode 100644
index 00000000000..4c7641ede89
--- /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/data/app/code-checker/test-not-equal.php b/tests/data/app/code-checker/test-not-equal.php
new file mode 100644
index 00000000000..d9a8d1c25c6
--- /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 64403fd0f23..f45ee02d185 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,25 @@ 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'],
+		];
+	}
+
+	/**
+	 * @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'],
 		];
 	}
 }