WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.56k stars 487 forks source link

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

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 the code snippet below and notice that there are 2 spaces next to the $values parameter.

Excluding the WordPress.WhiteSpace.ControlStructureSpacing WordPress-Extra ruleset fixes the issue.

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
    }
  }
}
jrfnl commented 1 year ago

Closing as duplicate of numerous previous issues. This has been fixed a long time ago in develop and we're working hard on getting WPCS 3.0.0 released.

mehdi-wb commented 1 year ago

Thanks but I tried using the develop branch "wp-coding-standards/wpcs": "dev-develop" and I still experience the same issue...

jrfnl commented 1 year ago

That's impossible. Could it be that you have multiple PHPCS installs and that even though you installed develop, when you triggered the command it used a different install which still uses 2.x ?

mehdi-wb commented 1 year ago

hmmm ok then let me double-check everything :)

jrfnl commented 1 year ago

@mehdi-wb Try using phpcs --config-show and vendor/bin/phpcs --config-show and look at the location of the config file, that should give you a good indication of which PHPCS install each is using. The output should also allow you to see which install of WPCS is being used, so you can double-check if it is the version you intended to use.

mehdi-wb commented 1 year ago

I will do that but in the meantime, I tried starting from scratch and removed my composer-loc.json file and vendor folder But looks like your develop branch is requiring a dependency version that does not exist for phpcsstandards/phpcsextra... I have Composer version 2.4.2

Updating dependencies  Your requirements could not be resolved to an installable set of packages.  wp-coding-standards/wpcs dev-develop requires phpcsstandards/phpcsextra ^1.0 -> found phpcsstandards/phpcsextra[dev-develop, 1.0.0-alpha1, 1.0.0-alpha2, 1.0.0-alpha3, 1.x-dev (alias of dev-develop)] but it does not match your minimum-stability.

mehdi-wb commented 1 year ago

So yeah looks like my package-lock.json was probably preventing the correct version from being installed...

mehdi-wb commented 1 year ago

I'll figure out a way to fix this and will report back Thanks for your help!

jrfnl commented 1 year ago

Yes, as it is develop, you will need to set "minimum-stability": "dev" in your composer.json and things should then install without issue. You may want to combine it with "prefer-stable": true.

Obviously, that will be resolved before the 3.0.0 release.

mehdi-wb commented 1 year ago

I was able to install the develop branch and all works well now thanks! Looking forward to the v3 release :)

jrfnl commented 1 year ago

@mehdi-wb Glad to hear it!