diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php index 593d588d..920454de 100644 --- a/src/lint/linter/ArcanistXHPASTLinter.php +++ b/src/lint/linter/ArcanistXHPASTLinter.php @@ -69,6 +69,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { const LINT_TOSTRING_EXCEPTION = 67; const LINT_LAMBDA_FUNC_FUNCTION = 68; const LINT_INSTANCEOF_OPERATOR = 69; + const LINT_INVALID_DEFAULT_PARAMETER = 70; private $blacklistedFunctions = array(); private $naminghook; @@ -215,6 +216,8 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { => pht('%s Function', '__lambda_func'), self::LINT_INSTANCEOF_OPERATOR => 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() { // The version number should be incremented whenever a new rule is added. - return '31'; + return '32'; } protected function resolveFuture($path, Future $future) { @@ -431,6 +434,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { 'lintThrowExceptionInToStringMethod' => self::LINT_TOSTRING_EXCEPTION, 'lintLambdaFuncFunction' => self::LINT_LAMBDA_FUNC_FUNCTION, 'lintInstanceOfOperator' => self::LINT_INSTANCEOF_OPERATOR, + 'lintInvalidDefaultParameters' => self::LINT_INVALID_DEFAULT_PARAMETER, ); 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). diff --git a/src/lint/linter/__tests__/xhpast/invalid-default-paramter.lint-test b/src/lint/linter/__tests__/xhpast/invalid-default-paramter.lint-test new file mode 100644 index 00000000..bac22ef8 --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/invalid-default-paramter.lint-test @@ -0,0 +1,17 @@ +