From c6a5a8e3934b1522a75b38a12b59800fa3a6b064 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 16 Jul 2016 01:43:44 +0200 Subject: [PATCH] Separate out the sniffing for `extract()`. Currently only VIP checked for the usage of `extract()`, even though it is an explicit rule for core. Ref: https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#dont-extract Solved #90 for real ;-) --- WordPress-Core/ruleset.xml | 1 + .../Sniffs/Functions/DontExtractSniff.php | 48 +++++++++++++++++ .../Sniffs/VIP/RestrictedFunctionsSniff.php | 8 --- .../Tests/Functions/DontExtractUnitTest.inc | 9 ++++ .../Tests/Functions/DontExtractUnitTest.php | 53 +++++++++++++++++++ .../Tests/VIP/RestrictedFunctionsUnitTest.inc | 2 +- .../Tests/VIP/RestrictedFunctionsUnitTest.php | 1 - 7 files changed, 112 insertions(+), 10 deletions(-) create mode 100644 WordPress/Sniffs/Functions/DontExtractSniff.php create mode 100644 WordPress/Tests/Functions/DontExtractUnitTest.inc create mode 100644 WordPress/Tests/Functions/DontExtractUnitTest.php diff --git a/WordPress-Core/ruleset.xml b/WordPress-Core/ruleset.xml index cf7de837..965eff86 100644 --- a/WordPress-Core/ruleset.xml +++ b/WordPress-Core/ruleset.xml @@ -111,5 +111,6 @@ + diff --git a/WordPress/Sniffs/Functions/DontExtractSniff.php b/WordPress/Sniffs/Functions/DontExtractSniff.php new file mode 100644 index 00000000..86bdd044 --- /dev/null +++ b/WordPress/Sniffs/Functions/DontExtractSniff.php @@ -0,0 +1,48 @@ + + */ +class WordPress_Sniffs_Functions_DontExtractSniff extends WordPress_AbstractFunctionRestrictionsSniff { + + /** + * Groups of functions to restrict. + * + * Example: groups => array( + * 'lambda' => array( + * 'type' => 'error' | 'warning', + * 'message' => 'Use anonymous functions instead please!', + * 'functions' => array( 'eval', 'create_function' ), + * ) + * ) + * + * @return array + */ + public function getGroups() { + return array( + + 'extract' => array( + 'type' => 'error', + 'message' => '%s() usage is highly discouraged, due to the complexity and unintended issues it might cause.', + 'functions' => array( + 'extract', + ), + ), + + ); + } // end getGroups() + +} // end class diff --git a/WordPress/Sniffs/VIP/RestrictedFunctionsSniff.php b/WordPress/Sniffs/VIP/RestrictedFunctionsSniff.php index 0976874c..72111b81 100644 --- a/WordPress/Sniffs/VIP/RestrictedFunctionsSniff.php +++ b/WordPress/Sniffs/VIP/RestrictedFunctionsSniff.php @@ -139,14 +139,6 @@ public function getGroups() { ), ), - 'extract' => array( - 'type' => 'warning', - 'message' => '%s() usage is highly discouraged, due to the complexity and unintended issues it might cause.', - 'functions' => array( - 'extract', - ), - ), - 'custom_role' => array( 'type' => 'error', 'message' => 'Use wpcom_vip_add_role() instead of add_role()', diff --git a/WordPress/Tests/Functions/DontExtractUnitTest.inc b/WordPress/Tests/Functions/DontExtractUnitTest.inc new file mode 100644 index 00000000..f6a12ec6 --- /dev/null +++ b/WordPress/Tests/Functions/DontExtractUnitTest.inc @@ -0,0 +1,9 @@ + 1 ) ); // bad + +// Similarly named functions or methods however are fine. +my_extract(); // Ok. +My_Object::extract(); // Ok. +$this->extract(); // Ok. +$my_object->extract(); // Ok. diff --git a/WordPress/Tests/Functions/DontExtractUnitTest.php b/WordPress/Tests/Functions/DontExtractUnitTest.php new file mode 100644 index 00000000..de482870 --- /dev/null +++ b/WordPress/Tests/Functions/DontExtractUnitTest.php @@ -0,0 +1,53 @@ + + * @author Greg Sherwood + * @author Marc McIntyre + * @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + * @version Release: @package_version@ + * @link http://pear.php.net/package/PHP_CodeSniffer + */ +class WordPress_Tests_Functions_DontExtractUnitTest extends AbstractSniffUnitTest { + + /** + * Returns the lines where errors should occur. + * + * The key of the array should represent the line number and the value + * should represent the number of errors that should occur on that line. + * + * @return array(int => int) + */ + public function getErrorList() { + return array( + 3 => 1, + ); + + } // end getErrorList() + + /** + * Returns the lines where warnings should occur. + * + * The key of the array should represent the line number and the value + * should represent the number of warnings that should occur on that line. + * + * @return array(int => int) + */ + public function getWarningList() { + return array(); + + } // end getWarningList() + +} // end class diff --git a/WordPress/Tests/VIP/RestrictedFunctionsUnitTest.inc b/WordPress/Tests/VIP/RestrictedFunctionsUnitTest.inc index b1c8bb1c..f9726edd 100644 --- a/WordPress/Tests/VIP/RestrictedFunctionsUnitTest.inc +++ b/WordPress/Tests/VIP/RestrictedFunctionsUnitTest.inc @@ -16,7 +16,7 @@ $ch = curl_init(); // bad curl_close( $ch ); // bad -extract( array( 'a' => 1 ) ); // bad +// Empty line - function moved to another sniff. add_role( 'test' ); // bad diff --git a/WordPress/Tests/VIP/RestrictedFunctionsUnitTest.php b/WordPress/Tests/VIP/RestrictedFunctionsUnitTest.php index 7802d8fe..92ca6e2a 100644 --- a/WordPress/Tests/VIP/RestrictedFunctionsUnitTest.php +++ b/WordPress/Tests/VIP/RestrictedFunctionsUnitTest.php @@ -91,7 +91,6 @@ public function getWarningList() { 13 => 1, 15 => 1, 17 => 1, - 19 => 1, 58 => 1, 59 => 1, 61 => 1,