From 4e95031ec4e74bb99dc2dc819ddb77893564e01c Mon Sep 17 00:00:00 2001
From: Joas Schilling <nickvergessen@owncloud.com>
Date: Mon, 15 Jun 2015 12:51:22 +0200
Subject: [PATCH] Allow app:check-code to check for deprecated methods
---
core/command/app/checkcode.php | 17 +++++-
lib/private/app/codechecker.php | 63 ++++++++++++----------
lib/private/app/codecheckvisitor.php | 45 +++++++++++-----
lib/private/app/deprecationcodechecker.php | 44 +++++++++++++++
tests/lib/app/codechecker.php | 2 -
tests/lib/app/deprecationcodechecker.php | 59 ++++++++++++++++++++
6 files changed, 184 insertions(+), 46 deletions(-)
create mode 100644 lib/private/app/deprecationcodechecker.php
create mode 100644 tests/lib/app/deprecationcodechecker.php
diff --git a/core/command/app/checkcode.php b/core/command/app/checkcode.php
index ecec51e5768..8bd079eaad1 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("<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;
diff --git a/lib/private/app/codechecker.php b/lib/private/app/codechecker.php
index 3f6cd19d6b4..38d98255cf1 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 e983bd8630b..d4b5bedbb1f 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 00000000000..c89428927c1
--- /dev/null
+++ b/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',
+ ];
+}
diff --git a/tests/lib/app/codechecker.php b/tests/lib/app/codechecker.php
index f45ee02d185..36c1251b8c5 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 00000000000..3f3baa7c314
--- /dev/null
+++ b/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'],
+ ];
+ }
+}
--
GitLab