From f546b8db719d261581ffec99654e6a2a2303ee2f Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 11 Apr 2021 18:09:59 +0200 Subject: [PATCH 1/2] Some small changes to existing ForbiddenFunctions sniff Nothing important, just switching from constructor to properties and adding some explanations about why we have that Sniff extending the Generic one instead of reusing it. --- moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php | 48 ++++++++++++++----- tests/local_codechecker_testcase.php | 4 +- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php b/moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php index 6b7ef25d..c03a8b98 100644 --- a/moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php +++ b/moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php @@ -17,6 +17,23 @@ /** * Sniff for debugging and other functions that we don't want used in finished code. * + * Note that strictly speaking we don't need to extend the Generic Sniff, + * just configure it in the ruleset.xml like this, for example: + * + * + * + * + * + * + * + * + * + * But we have decided to, instead, extend and keep the functions + * together with the Sniff. Also, this enables to test the Sniff + * without having to perform any configuration in the fixture files. + * (because unit tests DO NOT parse the ruleset.xml details, like + * properties, excludes... and other info). + * * @package local_codechecker * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later @@ -30,17 +47,22 @@ class ForbiddenFunctionsSniff extends GenericForbiddenFunctionsSniff { - /** Constructor. */ - public function __construct() { - $this->forbiddenFunctions = array( - // Usual development debugging functions. - 'sizeof' => 'count', - 'delete' => 'unset', - 'error_log' => null, - 'print_r' => null, - 'print_object' => null, - // Dangerous functions. From coding style. - 'extract' => null, - ); - } + /** + * A list of forbidden functions with their alternatives. + * + * The value is NULL if no alternative exists. IE, the + * function should just not be used. + * + * @var array + */ + public $forbiddenFunctions = [ + // Usual development debugging functions. + 'sizeof' => 'count', + 'delete' => 'unset', + 'error_log' => null, + 'print_r' => null, + 'print_object' => null, + // Dangerous functions. From coding style. + 'extract' => null, + ]; } diff --git a/tests/local_codechecker_testcase.php b/tests/local_codechecker_testcase.php index 43a42e64..dac7757a 100644 --- a/tests/local_codechecker_testcase.php +++ b/tests/local_codechecker_testcase.php @@ -191,7 +191,7 @@ protected function set_errors(array $errors) { // Let's normalize numeric, empty and string errors. foreach ($this->errors as $line => $errordef) { if (is_int($errordef) and $errordef > 0) { - $this->errors[$line] = array_fill(0, $errordef, null); + $this->errors[$line] = array_fill(0, $errordef, $errordef); } else if (empty($errordef)) { $this->errors[$line] = array(); } else if (is_string($errordef)) { @@ -210,7 +210,7 @@ protected function set_warnings(array $warnings) { // Let's normalize numeric, empty and string warnings. foreach ($this->warnings as $line => $warningdef) { if (is_int($warningdef) and $warningdef > 0) { - $this->warnings[$line] = array_fill(0, $warningdef, null); + $this->warnings[$line] = array_fill(0, $warningdef, $warningdef); } else if (empty($warningdef)) { $this->warnings[$line] = array(); } else if (is_string($warningdef)) { From 09e69be5542814c48ee39ea48dc2d1565e7e3e1b Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 11 Apr 2021 18:11:49 +0200 Subject: [PATCH 2/2] Deprecate print_error() To be able to add our own deprecations and messages we have extended the existing Generic DeprecatedFunctions Sniff. This version shows the alternatives and is able to process together both the internal (PHP) deprecated functions and our own ones (print_error). Covered with tests. This fixes #137 --- .../Sniffs/PHP/DeprecatedFunctionsSniff.php | 111 ++++++++++++++++++ moodle/ruleset.xml | 1 - .../moodle_php_deprecatedfunctions.php | 10 ++ moodle/tests/moodlestandard_test.php | 22 ++++ 4 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php create mode 100644 moodle/tests/fixtures/moodle_php_deprecatedfunctions.php diff --git a/moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php b/moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php new file mode 100644 index 00000000..b461fa55 --- /dev/null +++ b/moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php @@ -0,0 +1,111 @@ +. + +/** + * Sniff for various Moodle deprecated functions which uses should be replaced. + * + * Note that strictly speaking we don't need to extend the Generic Sniff, + * just configure it in the ruleset.xml like this, for example: + * + * + * + * + * + * + * + * + * + * But we have decided to, instead, extend and keep the functions + * together with the Sniff. Also, this enables to test the Sniff + * without having to perform any configuration in the fixture files. + * (because unit tests DO NOT parse the ruleset.xml details, like + * properties, excludes... and other info). + * + * @package local_codechecker + * @copyright 2021 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace MoodleCodeSniffer\moodle\Sniffs\PHP; + +use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\DeprecatedFunctionsSniff as GenericDeprecatedFunctionsSniff; +use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Files\File; + +class DeprecatedFunctionsSniff extends GenericDeprecatedFunctionsSniff { + + /** + * If true, an error will be thrown; otherwise a warning. + * + * @var boolean + */ + public $error = false; // Consider deprecations just warnings. + + /** + * A list of forbidden functions with their alternatives. + * + * The value is NULL if no alternative exists. IE, the + * function should just not be used. + * + * @var array + */ + public $forbiddenFunctions = [ + // Moodle deprecated functions. + 'print_error' => 'throw new moodle_exception() (using lang strings only if meant to be shown to final user)', + // Note that, apart from these Moodle explicitly set functions, also, all the internal PHP functions + // that are deprecated are detected automatically, {@see Generic\Sniffs\PHP\DeprecatedFunctionsSniff}. + ]; + + /** + * Generates the error or warning for this sniff. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the forbidden function + * in the token array. + * @param string $function The name of the forbidden function. + * @param string $pattern The pattern used for the match. + * + * @return void + * + * @todo: This method can be removed once/if this PR accepted: + * https://github.com/squizlabs/PHP_CodeSniffer/pull/3295 + */ + protected function addError($phpcsFile, $stackPtr, $function, $pattern=null) + { + $data = [$function]; + $error = 'Function %s() has been deprecated'; + $type = 'Deprecated'; + + if ($pattern === null) { + $pattern = strtolower($function); + } + + if ($this->forbiddenFunctions[$pattern] !== null + && $this->forbiddenFunctions[$pattern] !== 'null' + ) { + $type .= 'WithAlternative'; + $data[] = $this->forbiddenFunctions[$pattern]; + $error .= '; use %s() instead'; + } + + if ($this->error === true) { + $phpcsFile->addError($error, $stackPtr, $type, $data); + } else { + $phpcsFile->addWarning($error, $stackPtr, $type, $data); + } + + }//end addError() +} diff --git a/moodle/ruleset.xml b/moodle/ruleset.xml index 962f3104..212084f8 100644 --- a/moodle/ruleset.xml +++ b/moodle/ruleset.xml @@ -24,7 +24,6 @@ - diff --git a/moodle/tests/fixtures/moodle_php_deprecatedfunctions.php b/moodle/tests/fixtures/moodle_php_deprecatedfunctions.php new file mode 100644 index 00000000..863893c8 --- /dev/null +++ b/moodle/tests/fixtures/moodle_php_deprecatedfunctions.php @@ -0,0 +1,10 @@ +verify_cs_results(); } + public function test_moodle_php_deprecatedfunctions() { + + // Define the standard, sniff and fixture to use. + $this->set_standard('moodle'); + $this->set_sniff('moodle.PHP.DeprecatedFunctions'); + $this->set_fixture(__DIR__ . '/fixtures/moodle_php_deprecatedfunctions.php'); + + // Define expected results (errors and warnings). Format, array of: + // - line => number of problems, or + // - line => array of contents for message / source problem matching. + // - line => string of contents for message / source problem matching (only 1). + $this->set_errors(array()); + $warnings = array(7 => 'print_error() has been deprecated; use throw new moodle_exception()'); + if (PHP_VERSION_ID >= 70300 && PHP_VERSION_ID < 80000) { + $warnings[10] = 'mbsplit() has been deprecated'; + } + $this->set_warnings($warnings); + + // Let's do all the hard work! + $this->verify_cs_results(); + } + public function test_moodle_php_forbiddenfunctions() { // Define the standard, sniff and fixture to use.