Forbidden and deprecated functions

moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php

@@ -0,0 +1,111 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * 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:
+ *
+ * <rule ref="Generic.PHP.DeprecatedFunctions">
+ *   <properties>
+ *     <property name="forbiddenFunctions" type="array">
+ *       <element key="xxx" value="yyy"/>
+ *     </property>
+ *   </properties>
+ * </rule>
+ *
+ * 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<string, string|null>
+     */
+    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()
+}

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:
+ *
+ * <rule ref="Generic.PHP.ForbiddenFunctions">
+ *   <properties>
+ *     <property name="forbiddenFunctions" type="array">
+ *       <element key="xxx" value="yyy"/>
+ *     </property>
+ *   </properties>
+ * </rule>
+ *
+ * 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<string, string|null>
+     */
+    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,
+    ];
 }

moodle/ruleset.xml

@@ -24,7 +24,6 @@
     <rule ref="Generic.NamingConventions.ConstructorName"/>
     <rule ref="Generic.NamingConventions.UpperCaseConstantName"/>
 
-    <rule ref="Generic.PHP.DeprecatedFunctions"/>
     <rule ref="Generic.PHP.DisallowShortOpenTag"/>
     <rule ref="Generic.PHP.LowerCaseConstant"/>
 

moodle/tests/fixtures/moodle_php_deprecatedfunctions.php

@@ -0,0 +1,10 @@
+<?php
+defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures.
+
+// Let's try various deprecated functions.
+
+// Moodle own ones.
+print_error('error');
+
+// PHP internal ones.
+print_r(mbsplit("\s", "hello world"));

moodle/tests/moodlestandard_test.php

@@ -260,6 +260,28 @@ public function test_generic_whitespace_scopeindent() {
         $this->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.

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)) {