squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

Extra spaces in function declaration causes php fatal error crash #3713

Closed mehdi-wb closed 1 year ago

mehdi-wb commented 1 year ago

When declaring a function, if at least one extra space is added next to a parameter, the phpcs engine will crash with a php fatal error. See code snippet bellow and notice that there are 2 spaces next to the $values parameter.

Code sample

function my_function_or_method( $value  ) { return $value; }

Custom ruleset

<?xml version="1.0"?>
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="Ultimate Image Optimization Helpers" xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/squizlabs/PHP_CodeSniffer/master/phpcs.xsd">

    <description>A custom set of rules to check for a WPized WordPress project</description>

    <config name="installed_paths" value="../../phpcompatibility/php-compatibility,../../phpcompatibility/phpcompatibility-paragonie,../../phpcompatibility/phpcompatibility-wp,../../wp-coding-standards/wpcs" />

    <!-- Include the WordPress-Extra standard. -->
    <rule ref="WordPress-Extra">
        <!--
        We may want a middle ground though. The best way to do this is add the
        entire ruleset, then rule by rule, remove ones that don't suit a project.
        We can do this by running `phpcs` with the '-s' flag, which allows us to
        see the names of the sniffs reporting errors.
        Once we know the sniff names, we can opt to exclude sniffs which don't
        suit our project like so.
        The below two examples just show how you can exclude rules.
        They are not intended as advice about which sniffs to exclude.
        -->

        <!--
        <exclude name="WordPress.WhiteSpace.ControlStructureSpacing"/>
        <exclude name="WordPress.Security.EscapeOutput"/>
        -->
    </rule>

    <!-- Let's also check that everything is properly documented. -->
    <rule ref="WordPress-Docs"/>

    <!-- Add in some extra rules from other standards. -->
    <rule ref="Generic.CodeAnalysis.UnusedFunctionParameter"/>
    <rule ref="Generic.Commenting.Todo"/>

    <!-- Check for PHP cross-version compatibility. -->
    <!--
    To enable this, the PHPCompatibilityWP standard needs
    to be installed.
    See the readme for installation instructions:
    https://github.com/PHPCompatibility/PHPCompatibilityWP
    For more information, also see:
    https://github.com/PHPCompatibility/PHPCompatibility
    -->

    <config name="testVersion" value="7.2-"/>
    <rule ref="PHPCompatibilityWP">
        <include-pattern>*\.php$</include-pattern>
    </rule>

    <!--
    To get the optimal benefits of using WPCS, we should add a couple of
    custom properties.
    Adjust the values of these properties to fit our needs.
    For information on additional custom properties available, check out
    the wiki:
    https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/wiki/Customizable-sniff-properties
    -->
    <config name="minimum_supported_wp_version" value="5.4"/>

    <rule ref="WordPress.WP.I18n">
        <properties>
            <property name="text_domain" type="array">
                <element value="wmp"/>
                <element value="wm"/>
                <element value="wp"/>
                <element value="wm_podium"/>
                <element value="wp_podium"/>
                <element value="Divi"/>
                <element value="et_builder"/>
                <element value="wbchapp"/>
            </property>
        </properties>
    </rule>

    <rule ref="WordPress.NamingConventions.PrefixAllGlobals">
        <properties>
            <property name="prefixes" type="array">
                <element value="wmp_"/>
                <element value="wm_"/>
                <element value="wb2be_"/>
                <element value="wp_"/>
                <element value="wm_podium_"/>
                <element value="wp_podium_"/>
                <element value="et_"/>
                <element value="wbchapp_"/>
            </property>
        </properties>
    </rule>

    <!-- Exclude WP Core folders and files from being checked. -->
    <exclude-pattern>/docroot/wp-admin/*</exclude-pattern>
    <exclude-pattern>/docroot/wp-includes/*</exclude-pattern>
    <exclude-pattern>/docroot/wp-*.php</exclude-pattern>
    <exclude-pattern>/docroot/index.php</exclude-pattern>
    <exclude-pattern>/docroot/xmlrpc.php</exclude-pattern>
    <exclude-pattern>/docroot/wp-content/plugins/*</exclude-pattern>

    <!-- Exclude the Composer Vendor directory. -->
    <exclude-pattern>/vendor/*</exclude-pattern>

    <!-- Exclude the Node Modules directory. -->
    <exclude-pattern>/node_modules/*</exclude-pattern>

    <!-- Exclude minified files. -->
    <exclude-pattern>*.min.js</exclude-pattern>
    <exclude-pattern>*.min.css</exclude-pattern>

    <!-- Disable some unwanted rules. ref="" must be SPECIFICALLY otherwise it will override WP rules -->
    <rule ref="Generic.Arrays.DisallowShortArraySyntax.Found">
        <exclude name="Generic.Arrays.DisallowShortArraySyntax.Found"/>
    </rule>
    <rule ref="Squiz.Commenting.InlineComment.InvalidEndChar">
        <exclude name="Squiz.Commenting.InlineComment.InvalidEndChar"/>
    </rule>
    <rule ref="Squiz.Commenting.FunctionComment.EmptyThrows">
        <exclude name="Squiz.Commenting.FunctionComment.EmptyThrows"/>
    </rule>
    <rule ref="Squiz.Commenting.FunctionComment.ParamCommentFullStop">
        <exclude name="Squiz.Commenting.FunctionComment.ParamCommentFullStop"/>
    </rule>
    <rule ref="WordPress.NamingConventions.PrefixAllGlobals.ForbiddenPrefixPassed">
        <exclude name="WordPress.NamingConventions.PrefixAllGlobals.ForbiddenPrefixPassed"/>
    </rule>
    <rule ref="WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase">
        <exclude name="WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase"/>
    </rule>
</ruleset>

To reproduce Steps to reproduce the behavior:

  1. Add an extra space next to a parameter of a function/method declaration and save the file
  2. Run phpcs on your php file
  3. See the error message displayed bellow
    phpcs: PHP Fatal error: Uncaught TypeError: vsprintf(): Argument #2 ($values) must be of type array, string given in /Users/myuser/project/vendor/squizlabs/php_codesniffer/src/Files/File.php:1056 Stack trace: #0 /Users/myuser/project/vendor/squizlabs/php_codesniffer/src/Files/File.php(1056): vsprintf('Expected exactl...', ' ') #1 /Users/mesmyuserlem/project/vendor/squizlabs/php_codesniffer/src/Files/File.php(672): PHP_CodeSniffer\Files\File->addMessage(true, 'Expected exactl...', 77, 16, 'ExtraSpaceBefor...', ' ', 5, true) 

Expected behavior PHPCS will fail and the file will not be checked/processed.

Versions:

composer.json

{
  "require-dev": {
    "squizlabs/php_codesniffer": "3.7.1",
    "dealerdirect/phpcodesniffer-composer-installer": "v0.7.2",
    "phpmd/phpmd": "2.13.0",
    "wp-coding-standards/wpcs": "2.3.0",
    "phpcompatibility/phpcompatibility-wp": "2.1.4"
  },
  "scripts": {
    "phpcs": "phpcs --standard=phpcs.xml"
  },
  "require": {
    "ext-json": "*",
    "ext-openssl": "*",
    "ext-curl": "*",
    "ext-mysqli": "*"
  },
  "config": {
    "allow-plugins": {
      "dealerdirect/phpcodesniffer-composer-installer": true
    }
  }
}
gsherwood commented 1 year ago

I can't replicate this with any of the included standards so this may be an issue in one of the others you are using. The stacktrace also looks limited so I can't see which sniff the error is coming from.

Try replicating the issue with the WordPress and PHPCompat standards removed so you are just using the built-in standards. If that works, you'll need to report this to one of the other projects to take a look at. If you can get the full stacktrace of the error, we could work it out from there as well.

mehdi-wb commented 1 year ago

Thanks for your response. You are right, phpcs itself is not causing the issue... I found the sniff that is causing the bug and excluded it in my ruleset - all works fine now: <exclude name="WordPress.WhiteSpace.ControlStructureSpacing"/>