Allow app:check-code to check for deprecated methods

core/command/app/checkcode.php

@@ -23,9 +23,12 @@
 
 namespace OC\Core\Command\App;
 
+use OC\App\CodeChecker;
+use OC\App\DeprecationCodeChecker;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class CheckCode extends Command {
@@ -37,12 +40,22 @@ class CheckCode extends Command {
 				'app-id',
 				InputArgument::REQUIRED,
 				'check the specified app'
+			)
+			->addOption(
+				'deprecated',
+				'd',
+				InputOption::VALUE_NONE,
+				'check the specified app'
 			);
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
 		$appId = $input->getArgument('app-id');
-		$codeChecker = new \OC\App\CodeChecker();
+		if ($input->getOption('deprecated')) {
+			$codeChecker = new DeprecationCodeChecker();
+		} else {
+			$codeChecker = new CodeChecker();
+		}
 		$codeChecker->listen('CodeChecker', 'analyseFileBegin', function($params) use ($output) {
 			if(OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) {
 				$output->writeln("<info>Analysing {$params}</info>");
@@ -72,6 +85,8 @@ class CheckCode extends Command {
 		$errors = $codeChecker->analyse($appId);
 		if (empty($errors)) {
 			$output->writeln('<info>App is compliant - awesome job!</info>');
+		} elseif ($input->getOption('deprecated')) {
+			$output->writeln('<comment>App uses deprecated functionality</comment>');
 		} else {
 			$output->writeln('<error>App is not compliant</error>');
 			return 1;

lib/private/app/codechecker.php

@@ -28,7 +28,6 @@ use PhpParser\Lexer;
 use PhpParser\Node;
 use PhpParser\Node\Name;
 use PhpParser\NodeTraverser;
-use PhpParser\NodeVisitorAbstract;
 use PhpParser\Parser;
 use RecursiveCallbackFilterIterator;
 use RecursiveDirectoryIterator;
@@ -48,37 +47,43 @@ class CodeChecker extends BasicEmitter {
 	/** @var Parser */
 	private $parser;
 
+	/** @var string */
+	protected $blackListDescription = 'private';
+
 	/** @var string[] */
-	private $blackListedClassNames;
+	protected $blackListedClassNames = [
+		// 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',
+	];
+
+	/** @var bool */
+	protected $checkEqualOperators = false;
+
 
 	public function __construct() {
 		$this->parser = new Parser(new Lexer);
-		$this->blackListedClassNames = [
-			// 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',
-		];
 	}
 
 	/**
@@ -138,7 +143,7 @@ class CodeChecker extends BasicEmitter {
 		$code = file_get_contents($file);
 		$statements = $this->parser->parse($code);
 
-		$visitor = new CodeCheckVisitor($this->blackListedClassNames);
+		$visitor = new CodeCheckVisitor($this->blackListDescription, $this->blackListedClassNames, $this->checkEqualOperators);
 		$traverser = new NodeTraverser;
 		$traverser->addVisitor($visitor);
 

lib/private/app/codecheckvisitor.php

@@ -27,15 +27,41 @@ use PhpParser\Node\Name;
 use PhpParser\NodeVisitorAbstract;
 
 class CodeCheckVisitor extends NodeVisitorAbstract {
+	/** @var string */
+	protected $blackListDescription;
+	/** @var string[] */
+	protected $blackListedClassNames;
+	/** @var bool */
+	protected $checkEqualOperatorUsage;
+	/** @var string[] */
+	protected $errorMessages;
 
-	public function __construct($blackListedClassNames) {
+	/**
+	 * @param string $blackListDescription
+	 * @param string[] $blackListedClassNames
+	 * @param bool $checkEqualOperatorUsage
+	 */
+	public function __construct($blackListDescription, $blackListedClassNames, $checkEqualOperatorUsage) {
+		$this->blackListDescription = $blackListDescription;
 		$this->blackListedClassNames = array_map('strtolower', $blackListedClassNames);
+		$this->checkEqualOperatorUsage = $checkEqualOperatorUsage;
+
+		$this->errorMessages = [
+			CodeChecker::CLASS_EXTENDS_NOT_ALLOWED => "{$this->blackListDescription} class must not be extended",
+			CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED => "{$this->blackListDescription} interface must not be implemented",
+			CodeChecker::STATIC_CALL_NOT_ALLOWED => "Static method of {$this->blackListDescription} class must not be called",
+			CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED => "Constant of {$this->blackListDescription} class must not not be fetched",
+			CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED => "{$this->blackListDescription} class must not be instanciated",
+
+			CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED => "is discouraged",
+		];
 	}
 
+	/** @var array */
 	public $errors = [];
 
 	public function enterNode(Node $node) {
-		if ($node instanceof Node\Expr\BinaryOp\Equal) {
+		if ($this->checkEqualOperatorUsage && $node instanceof Node\Expr\BinaryOp\Equal) {
 			$this->errors[]= [
 				'disallowedToken' => '==',
 				'errorCode' => CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED,
@@ -43,7 +69,7 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
 				'reason' => $this->buildReason('==', CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED)
 			];
 		}
-		if ($node instanceof Node\Expr\BinaryOp\NotEqual) {
+		if ($this->checkEqualOperatorUsage && $node instanceof Node\Expr\BinaryOp\NotEqual) {
 			$this->errors[]= [
 				'disallowedToken' => '!=',
 				'errorCode' => CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED,
@@ -115,17 +141,8 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
 	}
 
 	private function buildReason($name, $errorCode) {
-		static $errorMessages= [
-			CodeChecker::CLASS_EXTENDS_NOT_ALLOWED => "used as base class",
-			CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED => "used as interface",
-			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])) {
-			return $errorMessages[$errorCode];
+		if (isset($this->errorMessages[$errorCode])) {
+			return $this->errorMessages[$errorCode];
 		}
 
 		return "$name usage not allowed - error: $errorCode";

lib/private/app/deprecationcodechecker.php

@@ -0,0 +1,44 @@
+<?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;
+
+use PhpParser\Lexer;
+use PhpParser\Node;
+use PhpParser\Node\Name;
+
+class DeprecationCodeChecker extends CodeChecker {
+	protected $checkEqualOperators = true;
+
+	/** @var string */
+	protected $blackListDescription = 'deprecated';
+
+	protected $blackListedClassNames = [
+		// Deprecated classes
+		'OCP\IConfig',
+		'OCP\Contacts',
+		'OCP\DB',
+		'OCP\IHelper',
+		'OCP\JSON',
+		'OCP\Response',
+		'OCP\AppFramework\IApi',
+	];
+}

tests/lib/app/codechecker.php

@@ -35,8 +35,6 @@ class CodeChecker extends 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'],
 		];
 	}
 

tests/lib/app/deprecationcodechecker.php

@@ -0,0 +1,59 @@
+<?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;
+
+use OC;
+use Test\TestCase;
+
+class DeprecationCodeChecker extends TestCase {
+
+	/**
+	 * @dataProvider providesFilesToCheck
+	 * @param $expectedErrorToken
+	 * @param $expectedErrorCode
+	 * @param $fileToVerify
+	 */
+	public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
+		$checker = new \OC\App\DeprecationCodeChecker();
+		$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 $fileToVerify
+	 */
+	public function testPassValidUsage($fileToVerify) {
+		$checker = new \OC\App\DeprecationCodeChecker();
+		$errors = $checker->analyseFile(OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
+
+		$this->assertEquals(0, count($errors));
+	}
+
+	public function validFilesData() {
+		return [
+			['test-extends.php'],
+			['test-implements.php'],
+			['test-static-call.php'],
+			['test-const.php'],
+			['test-new.php'],
+			['test-identical-operator.php'],
+		];
+	}
+}