Correctly handle use statements

This commit is contained in:
Joas Schilling 2015-06-15 15:24:45 +02:00
parent 4e95031ec4
commit d2fc1b2302
9 changed files with 104 additions and 11 deletions

View File

@ -43,6 +43,7 @@ class CodeChecker extends BasicEmitter {
const CLASS_CONST_FETCH_NOT_ALLOWED = 1003; const CLASS_CONST_FETCH_NOT_ALLOWED = 1003;
const CLASS_NEW_FETCH_NOT_ALLOWED = 1004; const CLASS_NEW_FETCH_NOT_ALLOWED = 1004;
const OP_OPERATOR_USAGE_DISCOURAGED = 1005; const OP_OPERATOR_USAGE_DISCOURAGED = 1005;
const CLASS_USE_NOT_ALLOWED = 1006;
/** @var Parser */ /** @var Parser */
private $parser; private $parser;

View File

@ -43,7 +43,12 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
*/ */
public function __construct($blackListDescription, $blackListedClassNames, $checkEqualOperatorUsage) { public function __construct($blackListDescription, $blackListedClassNames, $checkEqualOperatorUsage) {
$this->blackListDescription = $blackListDescription; $this->blackListDescription = $blackListDescription;
$this->blackListedClassNames = array_map('strtolower', $blackListedClassNames);
$this->blackListedClassNames = [];
foreach ($blackListedClassNames as $class) {
$class = strtolower($class);
$this->blackListedClassNames[$class] = $class;
}
$this->checkEqualOperatorUsage = $checkEqualOperatorUsage; $this->checkEqualOperatorUsage = $checkEqualOperatorUsage;
$this->errorMessages = [ $this->errorMessages = [
@ -52,6 +57,7 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
CodeChecker::STATIC_CALL_NOT_ALLOWED => "Static method of {$this->blackListDescription} class must not be called", 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_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::CLASS_NEW_FETCH_NOT_ALLOWED => "{$this->blackListDescription} class must not be instanciated",
CodeChecker::CLASS_USE_NOT_ALLOWED => "{$this->blackListDescription} class must not be imported with a use statement",
CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED => "is discouraged", CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED => "is discouraged",
]; ];
@ -127,15 +133,47 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
} }
} }
} }
if ($node instanceof Node\Stmt\UseUse) {
$this->checkBlackList($node->name->toString(), CodeChecker::CLASS_USE_NOT_ALLOWED, $node);
if ($node->alias) {
$this->addUseNameToBlackList($node->name->toString(), $node->alias);
} else {
$this->addUseNameToBlackList($node->name->toString(), $node->name->getLast());
}
}
}
/**
* Check whether an alias was introduced for a namespace of a blacklisted class
*
* Example:
* - Blacklist entry: OCP\AppFramework\IApi
* - Name: OCP\AppFramework
* - Alias: OAF
* => new blacklist entry: OAF\IApi
*
* @param string $name
* @param string $alias
*/
private function addUseNameToBlackList($name, $alias) {
$name = strtolower($name);
$alias = strtolower($alias);
foreach ($this->blackListedClassNames as $blackListedAlias => $blackListedClassName) {
if (strpos($blackListedClassName, $name . '\\') === 0) {
$aliasedClassName = str_replace($name, $alias, $blackListedClassName);
$this->blackListedClassNames[$aliasedClassName] = $blackListedClassName;
}
}
} }
private function checkBlackList($name, $errorCode, Node $node) { private function checkBlackList($name, $errorCode, Node $node) {
if (in_array(strtolower($name), $this->blackListedClassNames)) { if (isset($this->blackListedClassNames[strtolower($name)])) {
$this->errors[]= [ $this->errors[]= [
'disallowedToken' => $name, 'disallowedToken' => $name,
'errorCode' => $errorCode, 'errorCode' => $errorCode,
'line' => $node->getLine(), 'line' => $node->getLine(),
'reason' => $this->buildReason($name, $errorCode) 'reason' => $this->buildReason($this->blackListedClassNames[strtolower($name)], $errorCode)
]; ];
} }
} }

View File

@ -0,0 +1,9 @@
<?php
use OCP\AppFramework\IApi as OAFIA;
/**
* Class BadClass - creating an instance of a blacklisted class is not allowed
*/
class BadClass implements OAFIA {
}

View File

@ -0,0 +1,9 @@
<?php
use OCP\AppFramework as OAF;
/**
* Class BadClass - creating an instance of a blacklisted class is not allowed
*/
class BadClass implements OAF\IApi {
}

View File

@ -0,0 +1,9 @@
<?php
use OCP\AppFramework;
/**
* Class BadClass - creating an instance of a blacklisted class is not allowed
*/
class BadClass implements AppFramework\IApi {
}

View File

@ -0,0 +1,9 @@
<?php
use OCP\AppFramework\IApi;
/**
* Class BadClass - creating an instance of a blacklisted class is not allowed
*/
class BadClass implements IApi {
}

View File

@ -0,0 +1,12 @@
<?php
use OC_AppConfig as UseConfig;
/**
* Class BadClass - creating an instance of a blacklisted class is not allowed
*/
class BadClass {
public function foo() {
$bar = new UseConfig();
}
}

View File

@ -15,9 +15,9 @@ class CodeChecker extends TestCase {
/** /**
* @dataProvider providesFilesToCheck * @dataProvider providesFilesToCheck
* @param $expectedErrorToken * @param string $expectedErrorToken
* @param $expectedErrorCode * @param int $expectedErrorCode
* @param $fileToVerify * @param string $fileToVerify
*/ */
public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) { public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
$checker = new OC\App\CodeChecker(); $checker = new OC\App\CodeChecker();
@ -35,12 +35,13 @@ class CodeChecker extends TestCase {
['OC_App', 1002, 'test-static-call.php'], ['OC_App', 1002, 'test-static-call.php'],
['OC_API', 1003, 'test-const.php'], ['OC_API', 1003, 'test-const.php'],
['OC_AppConfig', 1004, 'test-new.php'], ['OC_AppConfig', 1004, 'test-new.php'],
['OC_AppConfig', 1006, 'test-use.php'],
]; ];
} }
/** /**
* @dataProvider validFilesData * @dataProvider validFilesData
* @param $fileToVerify * @param string $fileToVerify
*/ */
public function testPassValidUsage($fileToVerify) { public function testPassValidUsage($fileToVerify) {
$checker = new OC\App\CodeChecker(); $checker = new OC\App\CodeChecker();

View File

@ -15,9 +15,9 @@ class DeprecationCodeChecker extends TestCase {
/** /**
* @dataProvider providesFilesToCheck * @dataProvider providesFilesToCheck
* @param $expectedErrorToken * @param string $expectedErrorToken
* @param $expectedErrorCode * @param int $expectedErrorCode
* @param $fileToVerify * @param string $fileToVerify
*/ */
public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) { public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
$checker = new \OC\App\DeprecationCodeChecker(); $checker = new \OC\App\DeprecationCodeChecker();
@ -32,12 +32,16 @@ class DeprecationCodeChecker extends TestCase {
return [ return [
['==', 1005, 'test-equal.php'], ['==', 1005, 'test-equal.php'],
['!=', 1005, 'test-not-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'],
['OAF\IApi', 1001, 'test-deprecated-use-sub-alias.php'],
]; ];
} }
/** /**
* @dataProvider validFilesData * @dataProvider validFilesData
* @param $fileToVerify * @param string $fileToVerify
*/ */
public function testPassValidUsage($fileToVerify) { public function testPassValidUsage($fileToVerify) {
$checker = new \OC\App\DeprecationCodeChecker(); $checker = new \OC\App\DeprecationCodeChecker();
@ -53,6 +57,7 @@ class DeprecationCodeChecker extends TestCase {
['test-static-call.php'], ['test-static-call.php'],
['test-const.php'], ['test-const.php'],
['test-new.php'], ['test-new.php'],
['test-use.php'],
['test-identical-operator.php'], ['test-identical-operator.php'],
]; ];
} }