From 2422202ba4298b3b53433954ae98cc22f5eb36de Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Mon, 18 May 2015 08:23:48 +1000 Subject: [PATCH] Add a linter rule to prevent the __lambda_func function from being redeclared Summary: See http://phpsadness.com/sad/39. Declaring a function named `__lambda_func` prevents the `create_function` function from working. This is because `create_function` eval-declares the function `__lambda_func`, then modifies the symbol table so that the function is instead named `"\0lambda_".(++$i)`, and returns that name. NOTE: Personally, I don't think that anyone should use `create_function`. However, despite this, I think it is reasonable that no function is named `__lambda_func` in case some external library relies on `create_function`. Test Plan: Added test case. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D12870 --- src/lint/linter/ArcanistXHPASTLinter.php | 38 ++++++++++++++++++- .../xhpast/lamba-func-function.lint-test | 4 ++ 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 src/lint/linter/__tests__/xhpast/lamba-func-function.lint-test diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php index d08364fd..cad5bffa 100644 --- a/src/lint/linter/ArcanistXHPASTLinter.php +++ b/src/lint/linter/ArcanistXHPASTLinter.php @@ -67,6 +67,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { const LINT_ALIAS_FUNCTION = 65; const LINT_CAST_SPACING = 66; const LINT_TOSTRING_EXCEPTION = 67; + const LINT_LAMBDA_FUNC_FUNCTION = 68; private $blacklistedFunctions = array(); private $naminghook; @@ -209,6 +210,8 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { => pht('Cast Spacing'), self::LINT_TOSTRING_EXCEPTION => pht('Throwing Exception in %s Method', '__toString'), + self::LINT_LAMBDA_FUNC_FUNCTION + => pht('%s Function', '__lambda_func'), ); } @@ -329,7 +332,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { public function getVersion() { // The version number should be incremented whenever a new rule is added. - return '29'; + return '30'; } protected function resolveFuture($path, Future $future) { @@ -423,6 +426,7 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { 'lintAliasFunctions' => self::LINT_ALIAS_FUNCTION, 'lintCastSpacing' => self::LINT_CAST_SPACING, 'lintThrowExceptionInToStringMethod' => self::LINT_TOSTRING_EXCEPTION, + 'lintLambdaFuncFunction' => self::LINT_LAMBDA_FUNC_FUNCTION, ); foreach ($method_codes as $method => $codes) { @@ -4156,6 +4160,38 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { } } + private function lintLambdaFuncFunction(XHPASTNode $root) { + $function_declarations = $root + ->selectDescendantsOfType('n_FUNCTION_DECLARATION'); + + foreach ($function_declarations as $function_declaration) { + $function_name = $function_declaration->getChildByIndex(2); + + if ($function_name->getTypeName() == 'n_EMPTY') { + // Anonymous closure. + continue; + } + + if ($function_name->getConcreteString() != '__lambda_func') { + continue; + } + + $this->raiseLintAtNode( + $function_declaration, + self::LINT_LAMBDA_FUNC_FUNCTION, + pht( + 'Declaring a function named %s causes any call to %s to fail. '. + 'This is because %s eval-declares the function %s, then '. + 'modifies the symbol table so that the function is instead '. + 'named %s, and returns that name.', + '__lambda_func', + 'create_function', + 'create_function', + '__lambda_func', + '"\0lambda_".(++$i)')); + } + } + /** * Retrieve all calls to some specified function(s). diff --git a/src/lint/linter/__tests__/xhpast/lamba-func-function.lint-test b/src/lint/linter/__tests__/xhpast/lamba-func-function.lint-test new file mode 100644 index 00000000..a401b41f --- /dev/null +++ b/src/lint/linter/__tests__/xhpast/lamba-func-function.lint-test @@ -0,0 +1,4 @@ +