Merge pull request #16065 from owncloud/check-binaryops-in-code-checker
Check usage of != and ==
This commit is contained in:
commit
767cf2774a
|
@ -42,6 +42,7 @@ class CodeChecker extends BasicEmitter {
|
||||||
const STATIC_CALL_NOT_ALLOWED = 1002;
|
const STATIC_CALL_NOT_ALLOWED = 1002;
|
||||||
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;
|
||||||
|
|
||||||
/** @var Parser */
|
/** @var Parser */
|
||||||
private $parser;
|
private $parser;
|
||||||
|
|
|
@ -44,6 +44,22 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
|
||||||
public $errors = [];
|
public $errors = [];
|
||||||
|
|
||||||
public function enterNode(Node $node) {
|
public function enterNode(Node $node) {
|
||||||
|
if ($node instanceof Node\Expr\BinaryOp\Equal) {
|
||||||
|
$this->errors[]= [
|
||||||
|
'disallowedToken' => '==',
|
||||||
|
'errorCode' => CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED,
|
||||||
|
'line' => $node->getLine(),
|
||||||
|
'reason' => $this->buildReason('==', CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED)
|
||||||
|
];
|
||||||
|
}
|
||||||
|
if ($node instanceof Node\Expr\BinaryOp\NotEqual) {
|
||||||
|
$this->errors[]= [
|
||||||
|
'disallowedToken' => '!=',
|
||||||
|
'errorCode' => CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED,
|
||||||
|
'line' => $node->getLine(),
|
||||||
|
'reason' => $this->buildReason('!=', CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED)
|
||||||
|
];
|
||||||
|
}
|
||||||
if ($node instanceof Node\Stmt\Class_) {
|
if ($node instanceof Node\Stmt\Class_) {
|
||||||
if (!is_null($node->extends)) {
|
if (!is_null($node->extends)) {
|
||||||
$this->checkBlackList($node->extends->toString(), CodeChecker::CLASS_EXTENDS_NOT_ALLOWED, $node);
|
$this->checkBlackList($node->extends->toString(), CodeChecker::CLASS_EXTENDS_NOT_ALLOWED, $node);
|
||||||
|
@ -114,6 +130,7 @@ class CodeCheckVisitor extends NodeVisitorAbstract {
|
||||||
CodeChecker::STATIC_CALL_NOT_ALLOWED => "static method call on private class",
|
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_CONST_FETCH_NOT_ALLOWED => "used to fetch a const from",
|
||||||
CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED => "is instanciated",
|
CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED => "is instanciated",
|
||||||
|
CodeChecker::OP_OPERATOR_USAGE_DISCOURAGED => "is discouraged"
|
||||||
];
|
];
|
||||||
|
|
||||||
if (isset($errorMessages[$errorCode])) {
|
if (isset($errorMessages[$errorCode])) {
|
||||||
|
|
|
@ -0,0 +1,11 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Class BadClass - uses equal instead of identical operator
|
||||||
|
*/
|
||||||
|
class BadClass {
|
||||||
|
public function foo() {
|
||||||
|
if (true == false) {
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,13 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Class GoodClass - uses identical operator
|
||||||
|
*/
|
||||||
|
class GoodClass {
|
||||||
|
public function foo() {
|
||||||
|
if (true === false) {
|
||||||
|
}
|
||||||
|
if (true !== false) {
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,11 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Class BadClass - uses equal instead of identical operator
|
||||||
|
*/
|
||||||
|
class BadClass {
|
||||||
|
public function foo() {
|
||||||
|
if (true != false) {
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
|
@ -9,12 +9,14 @@
|
||||||
namespace Test\App;
|
namespace Test\App;
|
||||||
|
|
||||||
use OC;
|
use OC;
|
||||||
|
use Test\TestCase;
|
||||||
|
|
||||||
class CodeChecker extends \Test\TestCase {
|
class CodeChecker extends TestCase {
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @dataProvider providesFilesToCheck
|
* @dataProvider providesFilesToCheck
|
||||||
* @param $expectedErrors
|
* @param $expectedErrorToken
|
||||||
|
* @param $expectedErrorCode
|
||||||
* @param $fileToVerify
|
* @param $fileToVerify
|
||||||
*/
|
*/
|
||||||
public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
|
public function testFindInvalidUsage($expectedErrorToken, $expectedErrorCode, $fileToVerify) {
|
||||||
|
@ -33,6 +35,25 @@ class CodeChecker extends \Test\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'],
|
||||||
|
['==', 1005, 'test-equal.php'],
|
||||||
|
['!=', 1005, 'test-not-equal.php'],
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider validFilesData
|
||||||
|
* @param $fileToVerify
|
||||||
|
*/
|
||||||
|
public function testPassValidUsage($fileToVerify) {
|
||||||
|
$checker = new OC\App\CodeChecker();
|
||||||
|
$errors = $checker->analyseFile(OC::$SERVERROOT . "/tests/data/app/code-checker/$fileToVerify");
|
||||||
|
|
||||||
|
$this->assertEquals(0, count($errors));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function validFilesData() {
|
||||||
|
return [
|
||||||
|
['test-identical-operator.php'],
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue