Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow app check code for deprecated classes #16935

Merged
merged 12 commits into from
Jul 20, 2015
33 changes: 31 additions & 2 deletions core/command/app/checkcode.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* @author Joas Schilling <nickvergessen@owncloud.com>
* @author Morris Jobke <hey@morrisjobke.de>
* @author Thomas Müller <thomas.mueller@tmit.eu>
* @author Robin McCorkell <rmccorkell@karoshi.org.uk>
*
* @copyright Copyright (c) 2015, ownCloud, Inc.
* @license AGPL-3.0
Expand All @@ -23,12 +24,21 @@

namespace OC\Core\Command\App;

use OC\App\CodeChecker\CodeChecker;
use OC\App\CodeChecker\EmptyCheck;
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 {
protected $checkers = [
'private' => '\OC\App\CodeChecker\PrivateCheck',
'deprecation' => '\OC\App\CodeChecker\DeprecationCheck',
'strong-comparison' => '\OC\App\CodeChecker\StrongComparisonCheck',
];

protected function configure() {
$this
->setName('app:check-code')
Expand All @@ -37,12 +47,30 @@ protected function configure() {
'app-id',
InputArgument::REQUIRED,
'check the specified app'
)
->addOption(
'checker',
'c',
InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY,
'enable the specified checker(s)',
[ 'private', 'deprecation', 'strong-comparison' ]
);
}

protected function execute(InputInterface $input, OutputInterface $output) {
$appId = $input->getArgument('app-id');
$codeChecker = new \OC\App\CodeChecker();

$checkList = new EmptyCheck();
foreach ($input->getOption('checker') as $checker) {
if (!isset($this->checkers[$checker])) {
throw new \InvalidArgumentException('Invalid checker: '.$checker);
}
$checkerClass = $this->checkers[$checker];
$checkList = new $checkerClass($checkList);
}

$codeChecker = new CodeChecker($checkList);

$codeChecker->listen('CodeChecker', 'analyseFileBegin', function($params) use ($output) {
if(OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) {
$output->writeln("<info>Analysing {$params}</info>");
Expand Down Expand Up @@ -72,9 +100,10 @@ protected function execute(InputInterface $input, OutputInterface $output) {
$errors = $codeChecker->analyse($appId);
if (empty($errors)) {
$output->writeln('<info>App is compliant - awesome job!</info>');
return 0;
} else {
$output->writeln('<error>App is not compliant</error>');
return 1;
return 101;
}
}
}
139 changes: 139 additions & 0 deletions lib/private/app/codechecker/abstractcheck.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
<?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;

abstract class AbstractCheck implements ICheck {
/** @var ICheck */
protected $check;

/**
* @param ICheck $check
*/
public function __construct(ICheck $check) {
$this->check = $check;
}

/**
* @param int $errorCode
* @param string $errorObject
* @return string
*/
public function getDescription($errorCode, $errorObject) {
switch ($errorCode) {
case CodeChecker::STATIC_CALL_NOT_ALLOWED:
$functions = $this->getLocalFunctions();
$functions = array_change_key_case($functions, CASE_LOWER);
if (isset($functions[$errorObject])) {
return $this->getLocalDescription();
}
// no break;
case CodeChecker::CLASS_EXTENDS_NOT_ALLOWED:
case CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED:
case CodeChecker::CLASS_NEW_NOT_ALLOWED:
case CodeChecker::CLASS_USE_NOT_ALLOWED:
$classes = $this->getLocalClasses();
$classes = array_change_key_case($classes, CASE_LOWER);
if (isset($classes[$errorObject])) {
return $this->getLocalDescription();
}
break;

case CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED:
$constants = $this->getLocalConstants();
$constants = array_change_key_case($constants, CASE_LOWER);
if (isset($constants[$errorObject])) {
return $this->getLocalDescription();
}
break;

case CodeChecker::CLASS_METHOD_CALL_NOT_ALLOWED:
$methods = $this->getLocalMethods();
$methods = array_change_key_case($methods, CASE_LOWER);
if (isset($methods[$errorObject])) {
return $this->getLocalDescription();
}
break;
}

return $this->check->getDescription($errorCode, $errorObject);
}

/**
* @return string
*/
abstract protected function getLocalDescription();

/**
* @return array
*/
abstract protected function getLocalClasses();

/**
* @return array
*/
abstract protected function getLocalConstants();

/**
* @return array
*/
abstract protected function getLocalFunctions();

/**
* @return array
*/
abstract protected function getLocalMethods();

/**
* @return array E.g.: `'ClassName' => 'oc version',`
*/
public function getClasses() {
return array_merge($this->getLocalClasses(), $this->check->getClasses());
}

/**
* @return array E.g.: `'ClassName::CONSTANT_NAME' => 'oc version',`
*/
public function getConstants() {
return array_merge($this->getLocalConstants(), $this->check->getConstants());
}

/**
* @return array E.g.: `'functionName' => 'oc version',`
*/
public function getFunctions() {
return array_merge($this->getLocalFunctions(), $this->check->getFunctions());
}

/**
* @return array E.g.: `'ClassName::methodName' => 'oc version',`
*/
public function getMethods() {
return array_merge($this->getLocalMethods(), $this->check->getMethods());
}

/**
* @return bool
*/
public function checkStrongComparisons() {
return $this->check->checkStrongComparisons();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@
*
*/

namespace OC\App;
namespace OC\App\CodeChecker;

use OC\Hooks\BasicEmitter;
use PhpParser\Lexer;
use PhpParser\Node;
use PhpParser\Node\Name;
use PhpParser\NodeTraverser;
use PhpParser\NodeVisitorAbstract;
use PhpParser\Parser;
use RecursiveCallbackFilterIterator;
use RecursiveDirectoryIterator;
Expand All @@ -42,43 +41,20 @@ class CodeChecker extends BasicEmitter {
const CLASS_IMPLEMENTS_NOT_ALLOWED = 1001;
const STATIC_CALL_NOT_ALLOWED = 1002;
const CLASS_CONST_FETCH_NOT_ALLOWED = 1003;
const CLASS_NEW_FETCH_NOT_ALLOWED = 1004;
const CLASS_NEW_NOT_ALLOWED = 1004;
const OP_OPERATOR_USAGE_DISCOURAGED = 1005;
const CLASS_USE_NOT_ALLOWED = 1006;
const CLASS_METHOD_CALL_NOT_ALLOWED = 1007;

/** @var Parser */
private $parser;

/** @var string[] */
private $blackListedClassNames;
/** @var ICheck */
protected $checkList;

public function __construct() {
public function __construct(ICheck $checkList) {
$this->checkList = $checkList;
$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',
];
}

/**
Expand Down Expand Up @@ -138,7 +114,7 @@ public function analyseFile($file) {
$code = file_get_contents($file);
$statements = $this->parser->parse($code);

$visitor = new CodeCheckVisitor($this->blackListedClassNames);
$visitor = new NodeVisitor($this->checkList);
$traverser = new NodeTraverser;
$traverser->addVisitor($visitor);

Expand Down
Loading