Use the decorator pattern

core/command/app/checkcode.php

@@ -24,8 +24,10 @@
 namespace OC\Core\Command\App;
 
 use OC\App\CodeChecker\CodeChecker;
-use OC\App\CodeChecker\DeprecationList;
-use OC\App\CodeChecker\PrivateList;
+use OC\App\CodeChecker\DeprecationCheck;
+use OC\App\CodeChecker\EmptyCheck;
+use OC\App\CodeChecker\PrivateCheck;
+use OC\App\CodeChecker\StrongComparisonCheck;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
@@ -52,12 +54,14 @@ class CheckCode extends Command {
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
 		$appId = $input->getArgument('app-id');
+		$checkList = new EmptyCheck();
 		if ($input->getOption('deprecated')) {
-			$list = new DeprecationList();
+			$checkList = new DeprecationCheck($checkList);
+			$checkList = new StrongComparisonCheck($checkList);
 		} else {
-			$list = new PrivateList();
+			$checkList = new PrivateCheck($checkList);
 		}
-		$codeChecker = new CodeChecker($list);
+		$codeChecker = new CodeChecker($checkList);
 
 		$codeChecker->listen('CodeChecker', 'analyseFileBegin', function($params) use ($output) {
 			if(OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) {

lib/private/app/codechecker/codechecker.php

@@ -49,11 +49,11 @@ class CodeChecker extends BasicEmitter {
 	/** @var Parser */
 	private $parser;
 
-	/** @var ICheckList */
-	protected $list;
+	/** @var ICheck */
+	protected $checkList;
 
-	public function __construct(ICheckList $list) {
-		$this->list = $list;
+	public function __construct(ICheck $checkList) {
+		$this->checkList = $checkList;
 		$this->parser = new Parser(new Lexer);
 	}
 
@@ -114,7 +114,7 @@ class CodeChecker extends BasicEmitter {
 		$code = file_get_contents($file);
 		$statements = $this->parser->parse($code);
 
-		$visitor = new NodeVisitor($this->list);
+		$visitor = new NodeVisitor($this->checkList);
 		$traverser = new NodeTraverser;
 		$traverser->addVisitor($visitor);
 

lib/private/app/codechecker/deprecationlist.php → lib/private/app/codechecker/deprecationcheck.php

@@ -21,19 +21,30 @@
 
 namespace OC\App\CodeChecker;
 
-class DeprecationList implements ICheckList {
+class DeprecationCheck implements ICheck {
+	/** @var ICheck */
+	protected $check;
+
+	/**
+	 * @param ICheck $check
+	 */
+	public function __construct(ICheck $check) {
+		$this->check = $check;
+	}
+
 	/**
 	 * @return string
 	 */
 	public function getDescription() {
-		return 'deprecated';
+		$innerDescription = $this->check->getDescription();
+		return 'deprecated' . (($innerDescription === '') ? '' : ' ' . $innerDescription);
 	}
 
 	/**
 	 * @return array E.g.: `'ClassName' => 'oc version',`
 	 */
 	public function getClasses() {
-		return [
+		return array_merge([
 			'OCP\Config' => '8.0.0',
 			'OCP\Contacts' => '8.1.0',
 			'OCP\DB' => '8.1.0',
@@ -41,14 +52,14 @@ class DeprecationList implements ICheckList {
 			'OCP\JSON' => '8.1.0',
 			'OCP\Response' => '8.1.0',
 			'OCP\AppFramework\IApi' => '8.0.0',
-		];
+		], $this->check->getClasses());
 	}
 
 	/**
 	 * @return array E.g.: `'ClassName::CONSTANT_NAME' => 'oc version',`
 	 */
 	public function getConstants() {
-		return [
+		return array_merge([
 			'OCP::PERMISSION_CREATE' => '8.0.0',
 			'OCP::PERMISSION_READ' => '8.0.0',
 			'OCP::PERMISSION_UPDATE' => '8.0.0',
@@ -56,14 +67,14 @@ class DeprecationList implements ICheckList {
 			'OCP::PERMISSION_SHARE' => '8.0.0',
 			'OCP::PERMISSION_ALL' => '8.0.0',
 			'OCP::FILENAME_INVALID_CHARS' => '8.0.0',
-		];
+		], $this->check->getConstants());
 	}
 
 	/**
 	 * @return array E.g.: `'functionName' => 'oc version',`
 	 */
 	public function getFunctions() {
-		return [
+		return array_merge([
 			'OCP::image_path' => '8.0.0',
 			'OCP::mimetype_icon' => '8.0.0',
 			'OCP::preview_icon' => '8.0.0',
@@ -72,14 +83,14 @@ class DeprecationList implements ICheckList {
 			'OCP::relative_modified_date' => '8.0.0',
 			'OCP::simple_file_size' => '8.0.0',
 			'OCP::html_select_options' => '8.0.0',
-		];
+		], $this->check->getFunctions());
 	}
 
 	/**
 	 * @return array E.g.: `'ClassName::methodName' => 'oc version',`
 	 */
 	public function getMethods() {
-		return [
+		return array_merge([
 			'OCP\App::register' => '8.1.0',
 			'OCP\App::addNavigationEntry' => '8.1.0',
 			'OCP\App::setActiveNavigationEntry' => '8.1.0',
@@ -140,13 +151,13 @@ class DeprecationList implements ICheckList {
 			'OCP\Util::imagePath' => '8.1.0',
 			'OCP\Util::isValidFileName' => '8.1.0',
 			'OCP\Util::generateRandomBytes' => '8.1.0',
-		];
+		], $this->check->getMethods());
 	}
 
 	/**
 	 * @return bool
 	 */
 	public function checkStrongComparisons() {
-		return true;
+		return $this->check->checkStrongComparisons();
 	}
 }

lib/private/app/codechecker/emptycheck.php

@@ -0,0 +1,65 @@
+<?php
+/**
+ * @author Joas Schilling <nickvergessen@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+namespace OC\App\CodeChecker;
+
+class EmptyCheck implements ICheck {
+	/**
+	 * @return string
+	 */
+	public function getDescription() {
+		return '';
+	}
+
+	/**
+	 * @return array E.g.: `'ClassName' => 'oc version',`
+	 */
+	public function getClasses() {
+		return [];
+	}
+
+	/**
+	 * @return array E.g.: `'ClassName::CONSTANT_NAME' => 'oc version',`
+	 */
+	public function getConstants() {
+		return [];
+	}
+
+	/**
+	 * @return array E.g.: `'functionName' => 'oc version',`
+	 */
+	public function getFunctions() {
+		return [];
+	}
+
+	/**
+	 * @return array E.g.: `'ClassName::methodName' => 'oc version',`
+	 */
+	public function getMethods() {
+		return [];
+	}
+
+	/**
+	 * @return bool
+	 */
+	public function checkStrongComparisons() {
+		return false;
+	}
+}

lib/private/app/codechecker/ichecklist.php → lib/private/app/codechecker/icheck.php

@@ -20,7 +20,7 @@
  */
 namespace OC\App\CodeChecker;
 
-interface ICheckList {
+interface ICheck {
 	/**
 	 * @return string
 	 */

lib/private/app/codechecker/nodevisitor.php

@@ -43,9 +43,9 @@ class NodeVisitor extends NodeVisitorAbstract {
 	protected $errorMessages;
 
 	/**
-	 * @param ICheckList $list
+	 * @param ICheck $list
 	 */
-	public function __construct(ICheckList $list) {
+	public function __construct(ICheck $list) {
 		$this->blackListDescription = $list->getDescription();
 
 		$this->blackListedClassNames = [];

lib/private/app/codechecker/privatecheck.php

@@ -0,0 +1,104 @@
+<?php
+/**
+ * @author Joas Schilling <nickvergessen@owncloud.com>
+ * @author Morris Jobke <hey@morrisjobke.de>
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\App\CodeChecker;
+
+class PrivateCheck implements ICheck {
+	/** @var ICheck */
+	protected $check;
+
+	/**
+	 * @param ICheck $check
+	 */
+	public function __construct(ICheck $check) {
+		$this->check = $check;
+	}
+
+	/**
+	 * @return string
+	 */
+	public function getDescription() {
+		$innerDescription = $this->check->getDescription();
+		return 'private' . (($innerDescription === '') ? '' : ' ' . $innerDescription);
+	}
+
+	/**
+	 * @return array
+	 */
+	public function getClasses() {
+		return array_merge([
+			// classes replaced by the public api
+			'OC_API' => '6.0.0',
+			'OC_App' => '6.0.0',
+			'OC_AppConfig' => '6.0.0',
+			'OC_Avatar' => '6.0.0',
+			'OC_BackgroundJob' => '6.0.0',
+			'OC_Config' => '6.0.0',
+			'OC_DB' => '6.0.0',
+			'OC_Files' => '6.0.0',
+			'OC_Helper' => '6.0.0',
+			'OC_Hook' => '6.0.0',
+			'OC_Image' => '6.0.0',
+			'OC_JSON' => '6.0.0',
+			'OC_L10N' => '6.0.0',
+			'OC_Log' => '6.0.0',
+			'OC_Mail' => '6.0.0',
+			'OC_Preferences' => '6.0.0',
+			'OC_Search_Provider' => '6.0.0',
+			'OC_Search_Result' => '6.0.0',
+			'OC_Request' => '6.0.0',
+			'OC_Response' => '6.0.0',
+			'OC_Template' => '6.0.0',
+			'OC_User' => '6.0.0',
+			'OC_Util' => '6.0.0',
+		], $this->check->getClasses());
+	}
+
+	/**
+	 * @return array
+	 */
+	public function getConstants() {
+		return $this->check->getConstants();
+	}
+
+	/**
+	 * @return array
+	 */
+	public function getFunctions() {
+		return $this->check->getFunctions();
+	}
+
+	/**
+	 * @return array
+	 */
+	public function getMethods() {
+		return $this->check->getMethods();
+	}
+
+	/**
+	 * @return bool
+	 */
+	public function checkStrongComparisons() {
+		return $this->check->checkStrongComparisons();
+	}
+}

lib/private/app/codechecker/privatelist.php → lib/private/app/codechecker/strongcomparisoncheck.php

@@ -23,83 +23,56 @@
 
 namespace OC\App\CodeChecker;
 
-use OC\Hooks\BasicEmitter;
-use PhpParser\Lexer;
-use PhpParser\Node;
-use PhpParser\Node\Name;
-use PhpParser\NodeTraverser;
-use PhpParser\Parser;
-use RecursiveCallbackFilterIterator;
-use RecursiveDirectoryIterator;
-use RecursiveIteratorIterator;
-use RegexIterator;
-use SplFileInfo;
+class StrongComparisonCheck implements ICheck {
+	/** @var ICheck */
+	protected $check;
+
+	/**
+	 * @param ICheck $check
+	 */
+	public function __construct(ICheck $check) {
+		$this->check = $check;
+	}
 
-class PrivateList implements ICheckList {
 	/**
 	 * @return string
 	 */
 	public function getDescription() {
-		return 'private';
+		return $this->check->getDescription();
 	}
 
 	/**
 	 * @return array
 	 */
 	public function getClasses() {
-		return [
-			// classes replaced by the public api
-			'OC_API',
-			'OC_App',
-			'OC_AppConfig',
-			'OC_Avatar',
-			'OC_BackgroundJob',
-			'OC_Config',
-			'OC_DB',
-			'OC_Files',
-			'OC_Helper',
-			'OC_Hook',
-			'OC_Image',
-			'OC_JSON',
-			'OC_L10N',
-			'OC_Log',
-			'OC_Mail',
-			'OC_Preferences',
-			'OC_Search_Provider',
-			'OC_Search_Result',
-			'OC_Request',
-			'OC_Response',
-			'OC_Template',
-			'OC_User',
-			'OC_Util',
-		];
+		return $this->check->getClasses();
 	}
 
 	/**
 	 * @return array
 	 */
 	public function getConstants() {
-		return [];
+		return $this->check->getConstants();
 	}
 
 	/**
 	 * @return array
 	 */
 	public function getFunctions() {
-		return [];
+		return $this->check->getFunctions();
 	}
 
 	/**
 	 * @return array
 	 */
 	public function getMethods() {
-		return [];
+		return $this->check->getMethods();
 	}
 
 	/**
 	 * @return bool
 	 */
 	public function checkStrongComparisons() {
-		return false;
+		return true;
 	}
 }

tests/lib/app/codechecker/codecheckertest.php

@@ -8,7 +8,9 @@
 
 namespace Test\App\CodeChecker;
 
-use OC;
+use OC\App\CodeChecker\CodeChecker;
+use OC\App\CodeChecker\EmptyCheck;
+use OC\App\CodeChecker\PrivateCheck;
 use Test\TestCase;
 
 class CodeCheckerTest extends TestCase {
@@ -20,10 +22,10 @@ class CodeCheckerTest extends TestCase {
 	 * @param string $fileToVerify
 	 */
 	public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
-		$checker = new OC\App\CodeChecker\CodeChecker(
-			new OC\App\CodeChecker\PrivateList()
+		$checker = new CodeChecker(
+			new PrivateCheck(new EmptyCheck())
 		);
-		$errors = $checker->analyseFile(OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+		$errors = $checker->analyseFile(\OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
 
 		$this->assertEquals(1, count($errors));
 		$this->assertEquals($expectedErrorCode, $errors[0]['errorCode']);
@@ -46,10 +48,10 @@ class CodeCheckerTest extends TestCase {
 	 * @param string $fileToVerify
 	 */
 	public function testPassValidUsage($fileToVerify) {
-		$checker = new OC\App\CodeChecker\CodeChecker(
-			new OC\App\CodeChecker\PrivateList()
+		$checker = new CodeChecker(
+			new PrivateCheck(new EmptyCheck())
 		);
-		$errors = $checker->analyseFile(OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+		$errors = $checker->analyseFile(\OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
 
 		$this->assertEquals(0, count($errors));
 	}

tests/lib/app/codechecker/deprecationlisttest.php → tests/lib/app/codechecker/deprecationchecktest.php

@@ -6,12 +6,14 @@
  * See the COPYING-README file.
  */
 
-namespace Test\App;
+namespace Test\App\CodeChecker;
 
-use OC;
+use OC\App\CodeChecker\CodeChecker;
+use OC\App\CodeChecker\DeprecationCheck;
+use OC\App\CodeChecker\EmptyCheck;
 use Test\TestCase;
 
-class DeprecationListTest extends TestCase {
+class DeprecationCheckTest extends TestCase {
 
 	/**
 	 * @dataProvider providesFilesToCheck
@@ -20,10 +22,10 @@ class DeprecationListTest extends TestCase {
 	 * @param string $fileToVerify
 	 */
 	public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
-		$checker = new OC\App\CodeChecker\CodeChecker(
-			new OC\App\CodeChecker\DeprecationList()
+		$checker = new CodeChecker(
+			new DeprecationCheck(new EmptyCheck())
 		);
-		$errors = $checker->analyseFile(OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+		$errors = $checker->analyseFile(\OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
 
 		$this->assertEquals(1, count($errors));
 		$this->assertEquals($expectedErrorCode, $errors[0]['errorCode']);
@@ -32,8 +34,6 @@ class DeprecationListTest extends TestCase {
 
 	public function providesFilesToCheck() {
 		return [
-			['==', 1005, 'test-equal.php'],
-			['!=', 1005, 'test-not-equal.php'],
 			['OCP\AppFramework\IApi', 1006, 'test-deprecated-use.php'],
 			['OCP\AppFramework\IApi', 1006, 'test-deprecated-use-alias.php'],
 			['AppFramework\IApi', 1001, 'test-deprecated-use-sub.php'],
@@ -46,16 +46,18 @@ class DeprecationListTest extends TestCase {
 	 * @param string $fileToVerify
 	 */
 	public function testPassValidUsage($fileToVerify) {
-		$checker = new OC\App\CodeChecker\CodeChecker(
-			new OC\App\CodeChecker\DeprecationList()
+		$checker = new CodeChecker(
+			new DeprecationCheck(new EmptyCheck())
 		);
-		$errors = $checker->analyseFile(OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+		$errors = $checker->analyseFile(\OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
 
 		$this->assertEquals(0, count($errors));
 	}
 
 	public function validFilesData() {
 		return [
+			['test-equal.php'],
+			['test-not-equal.php'],
 			['test-extends.php'],
 			['test-implements.php'],
 			['test-static-call.php'],

tests/lib/app/codechecker/mock/testlist.php

@@ -21,15 +21,24 @@
 
 namespace Test\App\CodeChecker\Mock;
 
-use OC\App\CodeChecker\ICheckList;
+use OC\App\CodeChecker\ICheck;
 
-class TestList implements ICheckList {
+class TestList implements ICheck {
+	/** @var ICheck */
+	protected $check;
+
+	/**
+	 * @param ICheck $check
+	 */
+	public function __construct(ICheck $check) {
+		$this->check = $check;
+	}
 
 	/**
 	 * @return string
 	 */
 	public function getDescription() {
-		return 'deprecated';
+		return 'testing';
 	}
 
 	/**

tests/lib/app/codechecker/nodevisitortest.php

@@ -8,7 +8,9 @@
 
 namespace Test\AppCodeChecker;
 
-use OC;
+use OC\App\CodeChecker\CodeChecker;
+use OC\App\CodeChecker\EmptyCheck;
+use Test\App\CodeChecker\Mock\TestList;
 use Test\TestCase;
 
 class NodeVisitorTest extends TestCase {
@@ -56,10 +58,10 @@ class NodeVisitorTest extends TestCase {
 	 * @param string $fileToVerify
 	 */
 	public function testMethodsToCheck($expectedErrors, $fileToVerify) {
-		$checker = new OC\App\CodeChecker\CodeChecker(
-			new \Test\App\CodeChecker\Mock\TestList()
+		$checker = new CodeChecker(
+			new TestList(new EmptyCheck())
 		);
-		$errors = $checker->analyseFile(OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+		$errors = $checker->analyseFile(\OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
 
 		$this->assertCount(sizeof($expectedErrors), $errors);
 

tests/lib/app/codechecker/strongcomparisonchecktest.php

@@ -0,0 +1,70 @@
+<?php
+/**
+ * Copyright (c) 2015 Joas Schilling <nickvergessen@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace Test\App\CodeChecker;
+
+use OC\App\CodeChecker\CodeChecker;
+use OC\App\CodeChecker\EmptyCheck;
+use OC\App\CodeChecker\StrongComparisonCheck;
+use Test\TestCase;
+
+class StrongComparisonCheckTest extends TestCase {
+
+	/**
+	 * @dataProvider providesFilesToCheck
+	 * @param string $expectedErrorToken
+	 * @param int $expectedErrorCode
+	 * @param string $fileToVerify
+	 */
+	public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
+		$checker = new CodeChecker(
+			new StrongComparisonCheck(new EmptyCheck())
+		);
+		$errors = $checker->analyseFile(\OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+
+		$this->assertEquals(1, count($errors));
+		$this->assertEquals($expectedErrorCode, $errors[0]['errorCode']);
+		$this->assertEquals($expectedErrorToken, $errors[0]['disallowedToken']);
+	}
+
+	public function providesFilesToCheck() {
+		return [
+			['==', 1005, 'test-equal.php'],
+			['!=', 1005, 'test-not-equal.php'],
+		];
+	}
+
+	/**
+	 * @dataProvider validFilesData
+	 * @param string $fileToVerify
+	 */
+	public function testPassValidUsage($fileToVerify) {
+		$checker = new CodeChecker(
+			new StrongComparisonCheck(new EmptyCheck())
+		);
+		$errors = $checker->analyseFile(\OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+
+		$this->assertEquals(0, count($errors));
+	}
+
+	public function validFilesData() {
+		return [
+			['test-deprecated-use.php'],
+			['test-deprecated-use-alias.php'],
+			['test-deprecated-use-sub.php'],
+			['test-deprecated-use-sub-alias.php'],
+			['test-extends.php'],
+			['test-implements.php'],
+			['test-static-call.php'],
+			['test-const.php'],
+			['test-new.php'],
+			['test-use.php'],
+			['test-identical-operator.php'],
+		];
+	}
+}