Skip to content

Commit 46b5d10

Browse files
sarjonasarjona
authored
Merge pull request #141 from stronk7/forbidden_and_deprecated_functions
Forbidden and deprecated functions
2 parents 86c7a01 + 09e69be commit 46b5d10

File tree

6 files changed

+180
-16
lines changed

6 files changed

+180
-16
lines changed

‎moodle/Sniffs/PHP/DeprecatedFunctionsSniff.php‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php
2+
// This file is part of Moodle - http://moodle.org/
3+
//
4+
// Moodle is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// Moodle is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU General Public License
15+
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
16+
17+
/**
18+
* Sniff for various Moodle deprecated functions which uses should be replaced.
19+
*
20+
* Note that strictly speaking we don't need to extend the Generic Sniff,
21+
* just configure it in the ruleset.xml like this, for example:
22+
*
23+
* <rule ref="Generic.PHP.DeprecatedFunctions">
24+
* <properties>
25+
* <property name="forbiddenFunctions" type="array">
26+
* <element key="xxx" value="yyy"/>
27+
* </property>
28+
* </properties>
29+
* </rule>
30+
*
31+
* But we have decided to, instead, extend and keep the functions
32+
* together with the Sniff. Also, this enables to test the Sniff
33+
* without having to perform any configuration in the fixture files.
34+
* (because unit tests DO NOT parse the ruleset.xml details, like
35+
* properties, excludes... and other info).
36+
*
37+
* @package local_codechecker
38+
* @copyright 2021 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
39+
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
40+
*/
41+
42+
namespace MoodleCodeSniffer\moodle\Sniffs\PHP;
43+
44+
use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\DeprecatedFunctionsSniff as GenericDeprecatedFunctionsSniff;
45+
use PHP_CodeSniffer\Sniffs\Sniff;
46+
use PHP_CodeSniffer\Files\File;
47+
48+
class DeprecatedFunctionsSniff extends GenericDeprecatedFunctionsSniff {
49+
50+
/**
51+
* If true, an error will be thrown; otherwise a warning.
52+
*
53+
* @var boolean
54+
*/
55+
public $error = false; // Consider deprecations just warnings.
56+
57+
/**
58+
* A list of forbidden functions with their alternatives.
59+
*
60+
* The value is NULL if no alternative exists. IE, the
61+
* function should just not be used.
62+
*
63+
* @var array<string, string|null>
64+
*/
65+
public $forbiddenFunctions = [
66+
// Moodle deprecated functions.
67+
'print_error' => 'throw new moodle_exception() (using lang strings only if meant to be shown to final user)',
68+
// Note that, apart from these Moodle explicitly set functions, also, all the internal PHP functions
69+
// that are deprecated are detected automatically, {@see Generic\Sniffs\PHP\DeprecatedFunctionsSniff}.
70+
];
71+
72+
/**
73+
* Generates the error or warning for this sniff.
74+
*
75+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
76+
* @param int $stackPtr The position of the forbidden function
77+
* in the token array.
78+
* @param string $function The name of the forbidden function.
79+
* @param string $pattern The pattern used for the match.
80+
*
81+
* @return void
82+
*
83+
* @todo: This method can be removed once/if this PR accepted:
84+
* https://github.com/squizlabs/PHP_CodeSniffer/pull/3295
85+
*/
86+
protected function addError($phpcsFile, $stackPtr, $function, $pattern=null)
87+
{
88+
$data = [$function];
89+
$error = 'Function %s() has been deprecated';
90+
$type = 'Deprecated';
91+
92+
if ($pattern === null) {
93+
$pattern = strtolower($function);
94+
}
95+
96+
if ($this->forbiddenFunctions[$pattern] !== null
97+
&& $this->forbiddenFunctions[$pattern] !== 'null'
98+
) {
99+
$type .= 'WithAlternative';
100+
$data[] = $this->forbiddenFunctions[$pattern];
101+
$error .= '; use %s() instead';
102+
}
103+
104+
if ($this->error === true) {
105+
$phpcsFile->addError($error, $stackPtr, $type, $data);
106+
} else {
107+
$phpcsFile->addWarning($error, $stackPtr, $type, $data);
108+
}
109+
110+
}//end addError()
111+
}

‎moodle/Sniffs/PHP/ForbiddenFunctionsSniff.php‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,23 @@
1717
/**
1818
* Sniff for debugging and other functions that we don't want used in finished code.
1919
*
20+
* Note that strictly speaking we don't need to extend the Generic Sniff,
21+
* just configure it in the ruleset.xml like this, for example:
22+
*
23+
* <rule ref="Generic.PHP.ForbiddenFunctions">
24+
* <properties>
25+
* <property name="forbiddenFunctions" type="array">
26+
* <element key="xxx" value="yyy"/>
27+
* </property>
28+
* </properties>
29+
* </rule>
30+
*
31+
* But we have decided to, instead, extend and keep the functions
32+
* together with the Sniff. Also, this enables to test the Sniff
33+
* without having to perform any configuration in the fixture files.
34+
* (because unit tests DO NOT parse the ruleset.xml details, like
35+
* properties, excludes... and other info).
36+
*
2037
* @package local_codechecker
2138
* @copyright 2011 The Open University
2239
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
@@ -30,17 +47,22 @@
3047

3148
class ForbiddenFunctionsSniff extends GenericForbiddenFunctionsSniff {
3249

33-
/** Constructor. */
34-
public function __construct() {
35-
$this->forbiddenFunctions = array(
36-
// Usual development debugging functions.
37-
'sizeof' => 'count',
38-
'delete' => 'unset',
39-
'error_log' => null,
40-
'print_r' => null,
41-
'print_object' => null,
42-
// Dangerous functions. From coding style.
43-
'extract' => null,
44-
);
45-
}
50+
/**
51+
* A list of forbidden functions with their alternatives.
52+
*
53+
* The value is NULL if no alternative exists. IE, the
54+
* function should just not be used.
55+
*
56+
* @var array<string, string|null>
57+
*/
58+
public $forbiddenFunctions = [
59+
// Usual development debugging functions.
60+
'sizeof' => 'count',
61+
'delete' => 'unset',
62+
'error_log' => null,
63+
'print_r' => null,
64+
'print_object' => null,
65+
// Dangerous functions. From coding style.
66+
'extract' => null,
67+
];
4668
}

‎moodle/ruleset.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
<rule ref="Generic.NamingConventions.ConstructorName"/>
2525
<rule ref="Generic.NamingConventions.UpperCaseConstantName"/>
2626

27-
<rule ref="Generic.PHP.DeprecatedFunctions"/>
2827
<rule ref="Generic.PHP.DisallowShortOpenTag"/>
2928
<rule ref="Generic.PHP.LowerCaseConstant"/>
3029

‎moodle/tests/fixtures/moodle_php_deprecatedfunctions.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures.
3+
4+
// Let's try various deprecated functions.
5+
6+
// Moodle own ones.
7+
print_error('error');
8+
9+
// PHP internal ones.
10+
print_r(mbsplit("\s", "hello world"));

‎moodle/tests/moodlestandard_test.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,28 @@ public function test_generic_whitespace_scopeindent() {
260260
$this->verify_cs_results();
261261
}
262262

263+
public function test_moodle_php_deprecatedfunctions() {
264+
265+
// Define the standard, sniff and fixture to use.
266+
$this->set_standard('moodle');
267+
$this->set_sniff('moodle.PHP.DeprecatedFunctions');
268+
$this->set_fixture(__DIR__ . '/fixtures/moodle_php_deprecatedfunctions.php');
269+
270+
// Define expected results (errors and warnings). Format, array of:
271+
// - line => number of problems, or
272+
// - line => array of contents for message / source problem matching.
273+
// - line => string of contents for message / source problem matching (only 1).
274+
$this->set_errors(array());
275+
$warnings = array(7 => 'print_error() has been deprecated; use throw new moodle_exception()');
276+
if (PHP_VERSION_ID >= 70300 && PHP_VERSION_ID < 80000) {
277+
$warnings[10] = 'mbsplit() has been deprecated';
278+
}
279+
$this->set_warnings($warnings);
280+
281+
// Let's do all the hard work!
282+
$this->verify_cs_results();
283+
}
284+
263285
public function test_moodle_php_forbiddenfunctions() {
264286

265287
// Define the standard, sniff and fixture to use.

‎tests/local_codechecker_testcase.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ protected function set_errors(array $errors) {
191191
// Let's normalize numeric, empty and string errors.
192192
foreach ($this->errors as $line => $errordef) {
193193
if (is_int($errordef) and $errordef > 0) {
194-
$this->errors[$line] = array_fill(0, $errordef, null);
194+
$this->errors[$line] = array_fill(0, $errordef, $errordef);
195195
} else if (empty($errordef)) {
196196
$this->errors[$line] = array();
197197
} else if (is_string($errordef)) {
@@ -210,7 +210,7 @@ protected function set_warnings(array $warnings) {
210210
// Let's normalize numeric, empty and string warnings.
211211
foreach ($this->warnings as $line => $warningdef) {
212212
if (is_int($warningdef) and $warningdef > 0) {
213-
$this->warnings[$line] = array_fill(0, $warningdef, null);
213+
$this->warnings[$line] = array_fill(0, $warningdef, $warningdef);
214214
} else if (empty($warningdef)) {
215215
$this->warnings[$line] = array();
216216
} else if (is_string($warningdef)) {

0 commit comments

Comments
 (0)