mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-11-25 16:22:42 +01:00
Add a linter rule for declaring nested functions
Summary: Ref T7409. Adds `ArcanistXHPASTLinter::LINT_INNER_FUNCTION` to advise against declaring PHP functions within an outer function. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php | Squiz_Sniffs_PHP_InnerFunctionsSniff]]. Test Plan: Added unit tests. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7409 Differential Revision: https://secure.phabricator.com/D12389
This commit is contained in:
parent
7bba30f66c
commit
e233b158f2
2 changed files with 40 additions and 1 deletions
|
@ -58,6 +58,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
const LINT_UNNECESSARY_SEMICOLON = 56;
|
const LINT_UNNECESSARY_SEMICOLON = 56;
|
||||||
const LINT_SELF_MEMBER_REFERENCE = 57;
|
const LINT_SELF_MEMBER_REFERENCE = 57;
|
||||||
const LINT_LOGICAL_OPERATORS = 58;
|
const LINT_LOGICAL_OPERATORS = 58;
|
||||||
|
const LINT_INNER_FUNCTION = 59;
|
||||||
|
|
||||||
private $blacklistedFunctions = array();
|
private $blacklistedFunctions = array();
|
||||||
private $naminghook;
|
private $naminghook;
|
||||||
|
@ -129,6 +130,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
self::LINT_UNNECESSARY_SEMICOLON => 'Unnecessary Semicolon',
|
self::LINT_UNNECESSARY_SEMICOLON => 'Unnecessary Semicolon',
|
||||||
self::LINT_SELF_MEMBER_REFERENCE => 'Self Member Reference',
|
self::LINT_SELF_MEMBER_REFERENCE => 'Self Member Reference',
|
||||||
self::LINT_LOGICAL_OPERATORS => 'Logical Operators',
|
self::LINT_LOGICAL_OPERATORS => 'Logical Operators',
|
||||||
|
self::LINT_INNER_FUNCTION => 'Inner Functions',
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -175,6 +177,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
self::LINT_UNNECESSARY_SEMICOLON => $advice,
|
self::LINT_UNNECESSARY_SEMICOLON => $advice,
|
||||||
self::LINT_SELF_MEMBER_REFERENCE => $advice,
|
self::LINT_SELF_MEMBER_REFERENCE => $advice,
|
||||||
self::LINT_LOGICAL_OPERATORS => $advice,
|
self::LINT_LOGICAL_OPERATORS => $advice,
|
||||||
|
self::LINT_INNER_FUNCTION => $warning,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -242,7 +245,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 '21';
|
return '22';
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function resolveFuture($path, Future $future) {
|
protected function resolveFuture($path, Future $future) {
|
||||||
|
@ -325,6 +328,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
'lintConstantDefinitions' => self::LINT_NAMING_CONVENTIONS,
|
'lintConstantDefinitions' => self::LINT_NAMING_CONVENTIONS,
|
||||||
'lintSelfMemberReference' => self::LINT_SELF_MEMBER_REFERENCE,
|
'lintSelfMemberReference' => self::LINT_SELF_MEMBER_REFERENCE,
|
||||||
'lintLogicalOperators' => self::LINT_LOGICAL_OPERATORS,
|
'lintLogicalOperators' => self::LINT_LOGICAL_OPERATORS,
|
||||||
|
'lintInnerFunctions' => self::LINT_INNER_FUNCTION,
|
||||||
);
|
);
|
||||||
|
|
||||||
foreach ($method_codes as $method => $codes) {
|
foreach ($method_codes as $method => $codes) {
|
||||||
|
@ -3486,6 +3490,27 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function lintInnerFunctions(XHPASTNode $root) {
|
||||||
|
$function_decls = $root->selectDescendantsOfType('n_FUNCTION_DECLARATION');
|
||||||
|
|
||||||
|
foreach ($function_decls as $function_declaration) {
|
||||||
|
$inner_functions = $function_declaration
|
||||||
|
->selectDescendantsOfType('n_FUNCTION_DECLARATION');
|
||||||
|
|
||||||
|
foreach ($inner_functions as $inner_function) {
|
||||||
|
if ($inner_function->getChildByIndex(2)->getTypeName() == 'n_EMPTY') {
|
||||||
|
// Anonymous closure.
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->raiseLintAtNode(
|
||||||
|
$inner_function,
|
||||||
|
self::LINT_INNER_FUNCTION,
|
||||||
|
pht('Avoid the use of inner functions.'));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Retrieve all calls to some specified function(s).
|
* Retrieve all calls to some specified function(s).
|
||||||
*
|
*
|
||||||
|
|
14
src/lint/linter/__tests__/xhpast/inner-function.lint-test
Normal file
14
src/lint/linter/__tests__/xhpast/inner-function.lint-test
Normal file
|
@ -0,0 +1,14 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
function outer() {
|
||||||
|
if (!function_exists('inner')) {
|
||||||
|
function inner() {}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Closures are allowed.
|
||||||
|
function my_func($foo) {
|
||||||
|
function() {};
|
||||||
|
}
|
||||||
|
~~~~~~~~~~
|
||||||
|
warning:5:5
|
Loading…
Reference in a new issue