diff --git a/core/command/app/checkcode.php b/core/command/app/checkcode.php index ecec51e576..8bd079eaad 100644 --- a/core/command/app/checkcode.php +++ b/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("Analysing {$params}"); @@ -72,6 +85,8 @@ class CheckCode extends Command { $errors = $codeChecker->analyse($appId); if (empty($errors)) { $output->writeln('App is compliant - awesome job!'); + } elseif ($input->getOption('deprecated')) { + $output->writeln('App uses deprecated functionality'); } else { $output->writeln('App is not compliant'); return 1; diff --git a/lib/private/app/codechecker.php b/lib/private/app/codechecker.php index 3f6cd19d6b..38d98255cf 100644 --- a/lib/private/app/codechecker.php +++ b/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); diff --git a/lib/private/app/codecheckvisitor.php b/lib/private/app/codecheckvisitor.php index e983bd8630..d4b5bedbb1 100644 --- a/lib/private/app/codecheckvisitor.php +++ b/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"; diff --git a/lib/private/app/deprecationcodechecker.php b/lib/private/app/deprecationcodechecker.php new file mode 100644 index 0000000000..c89428927c --- /dev/null +++ b/lib/private/app/deprecationcodechecker.php @@ -0,0 +1,44 @@ + + * + * @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 + * + */ + +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', + ]; +} diff --git a/tests/lib/app/codechecker.php b/tests/lib/app/codechecker.php index f45ee02d18..36c1251b8c 100644 --- a/tests/lib/app/codechecker.php +++ b/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'], ]; } diff --git a/tests/lib/app/deprecationcodechecker.php b/tests/lib/app/deprecationcodechecker.php new file mode 100644 index 0000000000..3f3baa7c31 --- /dev/null +++ b/tests/lib/app/deprecationcodechecker.php @@ -0,0 +1,59 @@ + + * 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'], + ]; + } +}