From 2dd54f064ced73708e54607889874f5d14d09f2c Mon Sep 17 00:00:00 2001 From: Daimona Eaytoy Date: Wed, 19 Apr 2023 14:25:20 +0200 Subject: [PATCH 1/4] Make InnerFunctionsSniff detect functions inside closures --- src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php | 7 +++++-- src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc | 7 +++++++ src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php index 4e3ee2152c..c8afb57dfa 100644 --- a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php +++ b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php @@ -43,8 +43,11 @@ public function process(File $phpcsFile, $stackPtr) $function = $phpcsFile->getCondition($stackPtr, T_FUNCTION); if ($function === false) { - // Not a nested function. - return; + $function = $phpcsFile->getCondition($stackPtr, T_CLOSURE); + if ($function === false) { + // Not a nested function. + return; + } } $class = $phpcsFile->getCondition($stackPtr, T_ANON_CLASS, false); diff --git a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc index dd851461b9..0c69e06794 100644 --- a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc +++ b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc @@ -48,3 +48,10 @@ new class { } } }; + +$outerClosure = function () +{ + // Functions inside closures are not allowed. + function innerFunction() { + } +}; diff --git a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php index 3c9ad07bd3..606d23563c 100644 --- a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php +++ b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php @@ -28,6 +28,7 @@ public function getErrorList() return [ 5 => 1, 46 => 1, + 55 => 1, ]; }//end getErrorList() From 07c94c7e43d5d3b454fa95406d1dd3dc6316e9e9 Mon Sep 17 00:00:00 2001 From: Daimona Eaytoy Date: Wed, 19 Apr 2023 18:30:09 +0200 Subject: [PATCH 2/4] Allow methods inside OOP structures within functions --- .../Squiz/Sniffs/PHP/InnerFunctionsSniff.php | 42 +++++++++++++------ .../Tests/PHP/InnerFunctionsUnitTest.inc | 21 ++++++++++ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php index c8afb57dfa..efcf06c65a 100644 --- a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php +++ b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php @@ -41,25 +41,41 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - $function = $phpcsFile->getCondition($stackPtr, T_FUNCTION); - if ($function === false) { - $function = $phpcsFile->getCondition($stackPtr, T_CLOSURE); - if ($function === false) { - // Not a nested function. - return; + if (isset($tokens[$stackPtr]['conditions']) === false) { + return; + } + + $conditions = $tokens[$stackPtr]['conditions']; + + $outerFuncToken = null; + foreach ($conditions as $condToken => $condition) { + if ($condition === T_FUNCTION || $condition === T_CLOSURE) { + $outerFuncToken = $condToken; + break; } } - $class = $phpcsFile->getCondition($stackPtr, T_ANON_CLASS, false); - if ($class !== false && $class > $function) { - // Ignore methods in anon classes. + if ($outerFuncToken === null) { + // Not a nested function. return; } - $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); - if ($tokens[$prev]['code'] === T_EQUAL) { - // Ignore closures. - return; + $reversedConditions = array_reverse($conditions, true); + $allowedOOPConditions = [ + T_ANON_CLASS => true, + T_CLASS => true, + T_TRAIT => true, + T_INTERFACE => true, + ]; + foreach ($reversedConditions as $condToken => $condition) { + if ($condToken <= $outerFuncToken) { + break; + } + + if (\array_key_exists($condition, $allowedOOPConditions) === true) { + // Ignore methods in OOP structures defined within functions. + return; + } } $error = 'The use of inner functions is forbidden'; diff --git a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc index 0c69e06794..6f78608809 100644 --- a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc +++ b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc @@ -55,3 +55,24 @@ $outerClosure = function () function innerFunction() { } }; + +// Allow methods in classes/traits/interfaces defined inside functions +function foo() { + if (class_exists('MyClass') === false) { + class MyClass { + function foo() {} + } + } + + if (trait_exists('MyTrait') === false) { + trait MyTrait { + function foo() {} + } + } + + if (interface_exists('MyInterface') === false) { + interface MyInterface { + function foo(); + } + } +} From d027abb2a56fdfd6f59f11f61b810115465a47fd Mon Sep 17 00:00:00 2001 From: Daimona Eaytoy Date: Wed, 19 Apr 2023 23:26:50 +0200 Subject: [PATCH 3/4] Simplify and improve OOP scope check for nested functions --- .../Squiz/Sniffs/PHP/InnerFunctionsSniff.php | 28 ++++++------------- .../Tests/PHP/InnerFunctionsUnitTest.inc | 9 ++++++ .../Tests/PHP/InnerFunctionsUnitTest.php | 1 + 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php index efcf06c65a..721c0aeda8 100644 --- a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php +++ b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php @@ -11,6 +11,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Util\Tokens; class InnerFunctionsSniff implements Sniff { @@ -45,14 +46,19 @@ public function process(File $phpcsFile, $stackPtr) return; } - $conditions = $tokens[$stackPtr]['conditions']; + $reversedConditions = array_reverse($tokens[$stackPtr]['conditions'], true); $outerFuncToken = null; - foreach ($conditions as $condToken => $condition) { + foreach ($reversedConditions as $condToken => $condition) { if ($condition === T_FUNCTION || $condition === T_CLOSURE) { $outerFuncToken = $condToken; break; } + + if (\array_key_exists($condition, Tokens::$ooScopeTokens) === true) { + // Ignore methods in OOP structures defined within functions. + return; + } } if ($outerFuncToken === null) { @@ -60,24 +66,6 @@ public function process(File $phpcsFile, $stackPtr) return; } - $reversedConditions = array_reverse($conditions, true); - $allowedOOPConditions = [ - T_ANON_CLASS => true, - T_CLASS => true, - T_TRAIT => true, - T_INTERFACE => true, - ]; - foreach ($reversedConditions as $condToken => $condition) { - if ($condToken <= $outerFuncToken) { - break; - } - - if (\array_key_exists($condition, $allowedOOPConditions) === true) { - // Ignore methods in OOP structures defined within functions. - return; - } - } - $error = 'The use of inner functions is forbidden'; $phpcsFile->addError($error, $stackPtr, 'NotAllowed'); diff --git a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc index 6f78608809..d16c7f2eb6 100644 --- a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc +++ b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc @@ -75,4 +75,13 @@ function foo() { function foo(); } } + + // But disallow functions nested inside those methods + if (class_exists('NestedFunctionInMethod') === false) { + class NestedFunctionInMethod { + function foo() { + function innerFunction() {} + } + } + } } diff --git a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php index 606d23563c..b0b13b6147 100644 --- a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php +++ b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php @@ -29,6 +29,7 @@ public function getErrorList() 5 => 1, 46 => 1, 55 => 1, + 83 => 1, ]; }//end getErrorList() From 47b9cbb75585db5c1c8917f649806f5f7294952e Mon Sep 17 00:00:00 2001 From: Daimona Eaytoy Date: Wed, 19 Apr 2023 23:43:02 +0200 Subject: [PATCH 4/4] Micro-optimize array_reverse call --- src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php index 721c0aeda8..e42ef5a16a 100644 --- a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php +++ b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php @@ -46,7 +46,8 @@ public function process(File $phpcsFile, $stackPtr) return; } - $reversedConditions = array_reverse($tokens[$stackPtr]['conditions'], true); + $conditions = $tokens[$stackPtr]['conditions']; + $reversedConditions = array_reverse($conditions, true); $outerFuncToken = null; foreach ($reversedConditions as $condToken => $condition) {