mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-26 00:32:41 +01:00
Add a linter rule for invalid default parameters
Summary: See http://www.phpwtf.org/default-arguments-and-type-hinting. Test Plan: Added test case. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D12855
This commit is contained in:
parent
d0d2cf903c
commit
60a5a24033
2 changed files with 92 additions and 1 deletions
|
@ -69,6 +69,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
const LINT_TOSTRING_EXCEPTION = 67;
|
const LINT_TOSTRING_EXCEPTION = 67;
|
||||||
const LINT_LAMBDA_FUNC_FUNCTION = 68;
|
const LINT_LAMBDA_FUNC_FUNCTION = 68;
|
||||||
const LINT_INSTANCEOF_OPERATOR = 69;
|
const LINT_INSTANCEOF_OPERATOR = 69;
|
||||||
|
const LINT_INVALID_DEFAULT_PARAMETER = 70;
|
||||||
|
|
||||||
private $blacklistedFunctions = array();
|
private $blacklistedFunctions = array();
|
||||||
private $naminghook;
|
private $naminghook;
|
||||||
|
@ -215,6 +216,8 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
=> pht('%s Function', '__lambda_func'),
|
=> pht('%s Function', '__lambda_func'),
|
||||||
self::LINT_INSTANCEOF_OPERATOR
|
self::LINT_INSTANCEOF_OPERATOR
|
||||||
=> pht('%s Operator', 'instanceof'),
|
=> pht('%s Operator', 'instanceof'),
|
||||||
|
self::LINT_INVALID_DEFAULT_PARAMETER
|
||||||
|
=> pht('Invalid Default Parameter'),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -335,7 +338,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
|
|
||||||
public function getVersion() {
|
public function getVersion() {
|
||||||
// The version number should be incremented whenever a new rule is added.
|
// The version number should be incremented whenever a new rule is added.
|
||||||
return '31';
|
return '32';
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function resolveFuture($path, Future $future) {
|
protected function resolveFuture($path, Future $future) {
|
||||||
|
@ -431,6 +434,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
'lintThrowExceptionInToStringMethod' => self::LINT_TOSTRING_EXCEPTION,
|
'lintThrowExceptionInToStringMethod' => self::LINT_TOSTRING_EXCEPTION,
|
||||||
'lintLambdaFuncFunction' => self::LINT_LAMBDA_FUNC_FUNCTION,
|
'lintLambdaFuncFunction' => self::LINT_LAMBDA_FUNC_FUNCTION,
|
||||||
'lintInstanceOfOperator' => self::LINT_INSTANCEOF_OPERATOR,
|
'lintInstanceOfOperator' => self::LINT_INSTANCEOF_OPERATOR,
|
||||||
|
'lintInvalidDefaultParameters' => self::LINT_INVALID_DEFAULT_PARAMETER,
|
||||||
);
|
);
|
||||||
|
|
||||||
foreach ($method_codes as $method => $codes) {
|
foreach ($method_codes as $method => $codes) {
|
||||||
|
@ -4224,6 +4228,76 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function lintInvalidDefaultParameters(XHPASTNode $root) {
|
||||||
|
$parameters = $root->selectDescendantsOfType('n_DECLARATION_PARAMETER');
|
||||||
|
|
||||||
|
foreach ($parameters as $parameter) {
|
||||||
|
$type = $parameter->getChildByIndex(0);
|
||||||
|
$default = $parameter->getChildByIndex(2);
|
||||||
|
|
||||||
|
if ($type->getTypeName() == 'n_EMPTY') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($default->getTypeName() == 'n_EMPTY') {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$default_is_null = $default->getTypeName() == 'n_SYMBOL_NAME' &&
|
||||||
|
strtolower($default->getConcreteString()) == 'null';
|
||||||
|
|
||||||
|
switch (strtolower($type->getConcreteString())) {
|
||||||
|
case 'array':
|
||||||
|
if ($default->getTypeName() == 'n_ARRAY_LITERAL') {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if ($default_is_null) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$default,
|
||||||
|
self::LINT_INVALID_DEFAULT_PARAMETER,
|
||||||
|
pht(
|
||||||
|
'Default value for parameters with %s type hint '.
|
||||||
|
'can only be an %s or %s.',
|
||||||
|
'array',
|
||||||
|
'array',
|
||||||
|
'null'));
|
||||||
|
break;
|
||||||
|
|
||||||
|
case 'callable':
|
||||||
|
if ($default_is_null) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$default,
|
||||||
|
self::LINT_INVALID_DEFAULT_PARAMETER,
|
||||||
|
pht(
|
||||||
|
'Default value for parameters with %s type hint can only be %s.',
|
||||||
|
'callable',
|
||||||
|
'null'));
|
||||||
|
break;
|
||||||
|
|
||||||
|
default:
|
||||||
|
// Class/interface parameter.
|
||||||
|
if ($default_is_null) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$default,
|
||||||
|
self::LINT_INVALID_DEFAULT_PARAMETER,
|
||||||
|
pht(
|
||||||
|
'Default value for parameters with a class type hint '.
|
||||||
|
'can only be %s.',
|
||||||
|
'null'));
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Retrieve all calls to some specified function(s).
|
* Retrieve all calls to some specified function(s).
|
||||||
|
|
|
@ -0,0 +1,17 @@
|
||||||
|
<?php
|
||||||
|
function func_one(array $x) {}
|
||||||
|
function func_two(array $x = null) {}
|
||||||
|
function func_three(array $x = array()) {}
|
||||||
|
function func_four(array $x = false) {}
|
||||||
|
|
||||||
|
function func_five(callable $x) {}
|
||||||
|
function func_six(callable $x = null) {}
|
||||||
|
function func_seven(callable $x = false) {}
|
||||||
|
|
||||||
|
function func_eight(stdClass $x) {}
|
||||||
|
function func_nine(stdClass $x = null) {}
|
||||||
|
function func_ten(stdClass $x = array()) {}
|
||||||
|
~~~~~~~~~~
|
||||||
|
error:5:31
|
||||||
|
error:9:35
|
||||||
|
error:13:33
|
Loading…
Reference in a new issue