[Enhancement] DiscouragedFunction rule improvement

Author: lenaorobei

Magento2/Sniffs/PHP/DiscouragedFunctionSniff.php renamed to Magento2/Sniffs/Functions/DiscouragedFunctionSniff.php

@@ -3,9 +3,8 @@
  * Copyright © Magento. All rights reserved.
  * See COPYING.txt for license details.
  */
-namespace Magento2\Sniffs\PHP;
+namespace Magento2\Sniffs\Functions;
 
-use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff;
 
 /**
@@ -20,13 +19,19 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
      */
     protected $patternMatch = true;
 
+    /**
+     * If true, an error will be thrown; otherwise a warning.
+     *
+     * @var boolean
+     */
+    public $error = false;
+
     /**
      * List of patterns for forbidden functions.
      *
      * @var array
      */
     public $forbiddenFunctions = [
-        '^assert$' => null,
         '^bind_textdomain_codeset$' => null,
         '^bindtextdomain$' => null,
         '^bz.*$' => null,
@@ -52,7 +57,6 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
         '^dirname$' => null,
         '^dngettext$' => null,
         '^domxml_.*$' => null,
-        '^exec$' => null,
         '^fbsql_.*$' => null,
         '^fdf_add_doc_javascript$' => null,
         '^fdf_open$' => null,
@@ -93,7 +97,6 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
         '^parse_str$' => null,
         '^parse_url$' => null,
         '^parsekit_compile_string$' => null,
-        '^passthru$' => null,
         '^pathinfo$' => null,
         '^pcntl_.*$' => null,
         '^posix_.*$' => null,
@@ -122,14 +125,12 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
         '^setcookie$' => null,
         '^setlocale$' => null,
         '^setrawcookie$' => null,
-        '^shell_exec$' => null,
         '^sleep$' => null,
         '^socket_.*$' => null,
         '^stream_.*$' => null,
         '^sybase_.*$' => null,
         '^symlink$' => null,
         '^syslog$' => null,
-        '^system$' => null,
         '^touch$' => null,
         '^trigger_error$' => null,
         '^unlink$' => null,
@@ -220,34 +221,5 @@ class DiscouragedFunctionSniff extends ForbiddenFunctionsSniff
         '^is_null$' => 'strict comparison "=== null"',
         '^intval$' => '(int) construction',
         '^strval$' => '(string) construction',
-        '^md5$' => 'improved hash functions (SHA-256, SHA-512 etc.)',
-        '^serialize$' => 'json_encode',
-        '^unserialize$' => 'json_decode',
     ];
-
-    /**
-     * Generates warning for this sniff.
-     *
-     * @param 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
-     */
-    protected function addError($phpcsFile, $stackPtr, $function, $pattern = null)
-    {
-        $data = [$function];
-        $warningMessage = 'The use of function %s() is discouraged';
-        $warningCode = 'Found';
-        if ($pattern === null) {
-            $pattern = $function;
-        }
-        if ($this->forbiddenFunctions[$pattern] !== null) {
-            $warningCode .= 'WithAlternative';
-            $data[] = $this->forbiddenFunctions[$pattern];
-            $warningMessage .= '; use %s instead.';
-        }
-        $phpcsFile->addWarning($warningMessage, $stackPtr, $warningCode, $data);
-    }
 }

Magento2/Sniffs/Security/InsecureFunctionSniff.php

@@ -0,0 +1,37 @@
+<?php
+/**
+ * Copyright © Magento. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento2\Sniffs\Functions;
+
+use PHP_CodeSniffer\Standards\Generic\Sniffs\PHP\ForbiddenFunctionsSniff;
+
+/**
+ * Detects the use of insecure functions.
+ */
+class InsecureFunctionSniff extends ForbiddenFunctionsSniff
+{
+    /**
+     * If true, an error will be thrown; otherwise a warning.
+     *
+     * @var boolean
+     */
+    public $error = false;
+
+    /**
+     * List of patterns for forbidden functions.
+     *
+     * @var array
+     */
+    public $forbiddenFunctions = [
+        'assert' => null,
+        'exec' => null,
+        'passthru' => null,
+        'shell_exec' => null,
+        'system' => null,
+        'md5' => 'improved hash functions (SHA-256, SHA-512 etc.)',
+        'serialize' => 'json_encode',
+        'unserialize' => 'json_decode',
+    ];
+}

Magento2/Tests/PHP/DiscouragedFunctionUnitTest.inc renamed to Magento2/Tests/Functions/DiscouragedFunctionUnitTest.inc

@@ -3,7 +3,7 @@
  * Copyright © Magento. All rights reserved.
  * See COPYING.txt for license details.
  */
-namespace Magento2\Tests\PHP;
+namespace Magento2\Tests\Functions;
 
 use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
 
@@ -26,7 +26,6 @@ public function getErrorList()
     public function getWarningList()
     {
         return [
-            3 => 1,
             6 => 1,
             7 => 1,
             9 => 1,
@@ -63,7 +62,6 @@ public function getWarningList()
             65 => 1,
             67 => 1,
             69 => 1,
-            71 => 1,
             73 => 1,
             74 => 1,
             76 => 1,
@@ -122,7 +120,6 @@ public function getWarningList()
             166 => 1,
             169 => 1,
             171 => 1,
-            173 => 1,
             175 => 1,
             177 => 1,
             179 => 1,
@@ -152,7 +149,6 @@ public function getWarningList()
             229 => 1,
             231 => 1,
             233 => 1,
-            235 => 1,
             237 => 1,
             239 => 1,
             241 => 1,
@@ -162,7 +158,6 @@ public function getWarningList()
             247 => 1,
             249 => 1,
             251 => 1,
-            253 => 1,
             255 => 1,
             258 => 1,
             261 => 1,
@@ -257,7 +252,6 @@ public function getWarningList()
             458 => 1,
             460 => 1,
             462 => 1,
-            464 => 1,
         ];
     }
 }

Magento2/Tests/PHP/DiscouragedFunctionUnitTest.php renamed to Magento2/Tests/Functions/DiscouragedFunctionUnitTest.php

@@ -0,0 +1,18 @@
+<?php
+
+assert($a === true);
+
+exec('echo 1;');
+
+passthru('echo 1;');
+
+shell_exec('echo 1;');
+
+system('echo 1;');
+
+md5($text);
+
+serialize([]);
+
+unserialize('');
+

Magento2/Tests/Security/InsecureFunctionUnitTest.inc

@@ -0,0 +1,39 @@
+<?php
+/**
+ * Copyright © Magento. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento2\Tests\Functions;
+
+use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
+
+/**
+ * Class InsecureFunctionUnitTest
+ */
+class InsecureFunctionUnitTest extends AbstractSniffUnitTest
+{
+    /**
+     * @inheritdoc
+     */
+    public function getErrorList()
+    {
+        return [];
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function getWarningList()
+    {
+        return [
+            3 => 1,
+            5 => 1,
+            7 => 1,
+            9 => 1,
+            11 => 1,
+            13 => 1,
+            15 => 1,
+            17 => 1,
+        ];
+    }
+}

Magento2/Tests/Security/InsecureFunctionUnitTest.php

@@ -50,6 +50,7 @@
     <rule ref="Magento2.Security.IncludeFile">
         <severity>10</severity>
         <type>error</type>
+        <exclude-pattern>*/Test/*</exclude-pattern>
     </rule>
     <rule ref="Magento2.Security.LanguageConstruct">
         <severity>10</severity>
@@ -86,7 +87,7 @@
         <severity>9</severity>
         <type>warning</type>
     </rule>
-    <rule ref="Magento2.PHP.DiscouragedFunction">
+    <rule ref="Magento2.Security.InsecureFunction">
         <severity>9</severity>
         <type>warning</type>
     </rule>
@@ -116,6 +117,7 @@
     <rule ref="Magento2.Classes.ObjectInstantiation">
         <severity>8</severity>
         <type>warning</type>
+        <exclude-pattern>*/Test/*</exclude-pattern>
     </rule>
     <rule ref="Magento2.Exceptions.DirectThrow">
         <severity>8</severity>
@@ -125,9 +127,16 @@
         <severity>8</severity>
         <type>warning</type>
     </rule>
+    <rule ref="Magento2.Functions.DiscouragedFunction">
+        <severity>8</severity>
+        <type>warning</type>
+        <exclude-pattern>*/lib/*</exclude-pattern>
+        <exclude-pattern>*/Test/*</exclude-pattern>
+    </rule>
     <rule ref="Magento2.Functions.StaticFunction">
         <severity>8</severity>
         <type>warning</type>
+        <exclude-pattern>*/Test/*</exclude-pattern>
     </rule>
     <rule ref="Magento2.Files.LineLength">
         <severity>8</severity>
@@ -226,6 +235,7 @@
     <rule ref="Squiz.Functions.GlobalFunction">
         <severity>7</severity>
         <type>warning</type>
+        <exclude-pattern>*/Test/*</exclude-pattern>
     </rule>
     <rule ref="Squiz.Operators.IncrementDecrementUsage">
         <severity>7</severity>