WPBP / WordPress-Plugin-Boilerplate-Powered

Wordpress Plugin Boilerplate but Powered with examples and a generator!
https://wpbp.github.io/
GNU General Public License v3.0
791 stars 114 forks source link

[BUG] Is pbpcbf expected to work for SlevomatCodingStandard? #224

Closed MikeiLL closed 1 year ago

MikeiLL commented 1 year ago

Describe the bug Seems like phpcbf doesn't repair whitespace errors with SlevomatCodingStandards

To Reproduce

PHP 8.0.26
PHP_CodeSniffer version 3.7.1
Composer version 2.5.

Steps to reproduce the behavior:

  1. run vendor/bin/phpcs --standard=CodeatCodingStandard --extensions=php --report=full -s somefile.php

Output:

FILE: somefile.php
------------------------------------------------
// removed for brevity
 136 | ERROR   | [x] There must be one empty line before class closing brace.
     |         |     (SlevomatCodingStandard.Classes.EmptyLinesAroundClassBraces.NoEmptyLineBeforeClosingBrace)
------------------------------------------------
PHPCBF CAN FIX THE 36 MARKED SNIFF VIOLATIONS AUTOMATICALLY
FOUND 33 ERRORS AND 8 WARNINGS AFFECTING 28 LINES
  1. run vendor/bin/phpcbf --standard=CodeatCodingStandard --extensions=php --report=full -s somefile.php

Output:

PHPCBF RESULT SUMMARY
----------------------------------------------------------------------------------------------------------------
FILE                                                                                            FIXED  REMAINING
----------------------------------------------------------------------------------------------------------------
somefile.php     FAILED TO FIX
----------------------------------------------------------------------------------------------------------------
A TOTAL OF 35 ERRORS WERE FIXED IN 1 FILE
----------------------------------------------------------------------------------------------------------------
PHPCBF FAILED TO FIX 1 FILE
----------------------------------------------------------------------------------------------------------------
  1. Re-run vendor/bin/phpcs it returns all the same errors and warnings as before phpcbf.
Mte90 commented 1 year ago

It isn't something that we can fix as the slevomat that are not able to fix the issue.

It isn't the first time, https://github.com/slevomat/coding-standard/issues/1057

You should open a ticket to https://github.com/slevomat/coding-standard with the file that is not able to fix.

Anyway I am wondering if there is just a conflict with the ruleset from wordpress and slevomat that together are creating this issue.

MikeiLL commented 1 year ago

Anyway I am wondering if there is just a conflict with the ruleset from wordpress and slevomat that together are creating this issue.

Perhaps I'll mention that in the issue. Not sure how I would go about figuring it out. Last time I started a project with this (fantastic) template, the Ruleset didn't include slevomat, which looks good, but I'm feeling a little like I'm chasing my tail serving the linter.

Mte90 commented 1 year ago

If I can get a minimal file that generate the issue I can investigate next week.

MikeiLL commented 1 year ago

That would be very much appreciated, man.

Here's one that it crashes on:

<?php
/**
 * Demo page with a class.
 *
 * This does nothing.
 *
 * @package Demo
 */

/**
 * Demo
 *
 * This does nothing.
 */
class Demo {

    /**
     * Nothing
     *
     * This may be set for no reason.
     *
     * @since  2.4.7
     * @access protected
          * @var int $nothing does anything here. <- tab replaced with four spaces, inconsistent with rest of codebase.
     */
    protected $nothing;

    /**
     * Nothing method
     *
     * This may be set for no reason.
     *
     * @since  2.4.7
     * @access public
     * @var int $nothing does anything here.
     */
    public function nothing() {
        $this->nothing = empty( false ) ? 1 : 0;
    }

}

Workflow in php8.0 and php8.1:

FILE: /pathto/demo.php
------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------------------
 38 | ERROR | [x] Ternary operator should be reformatted to more lines.
    |       |     (SlevomatCodingStandard.ControlStructures.RequireMultiLineTernaryOperator.MultiLineTernaryOperatorNotUsed)
------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 230ms; Memory: 20MB

Then

vendor/bin/phpcbf --standard=CodeatCodingStandard --extensions=php --report=full -s demo.php
E 1 / 1 (100%)

PHPCBF RESULT SUMMARY
---------------------------------------------------------------------------------------------------
FILE                                                                               FIXED  REMAINING
---------------------------------------------------------------------------------------------------
/pathto/demo.php     FAILED TO FIX
---------------------------------------------------------------------------------------------------
A TOTAL OF -1 ERRORS WERE FIXED IN 1 FILE
---------------------------------------------------------------------------------------------------
PHPCBF FAILED TO FIX 1 FILE
---------------------------------------------------------------------------------------------------

Time: 3.04 secs; Memory: 20MB
Mte90 commented 1 year ago

I can replicate the issue but I don't understand what for Slevomat means a Ternary operator on multiple lines and in their readme suggest to disable the autofix in case the strict mode is enabled. To avoid troubles I updated the CodeatCS package with a new version to disable the ternary operators.

MikeiLL commented 1 year ago

in case the strict mode is enabled

Enabled where, please?

Mte90 commented 1 year ago

https://github.com/slevomat/coding-standard/blob/master/doc/control-structures.md#slevomatcodingstandardcontrolstructuresdisallowshortternaryoperator- I thinks that is talking about the php strict mode

MikeiLL commented 1 year ago

Thanks, man. Updated. Now using:

codeatcode/codeatcs  1.0.20
squizlabs/php_codesniffer 3.7.1
slevomat/coding-standard 8.8.0

Seem to be getting same result:

vendor/bin/phpcs --standard=CodeatCodingStandard --extensions=php --report=full -s demo.php
PHP Warning:  Cannot declare class Amp\Parallel\Sync\SerializationException, because the name is already in use in path_to/vendor/amphp/parallel/lib/Sync/functions.php on line 9 (runs twice)

FILE: demo.php
------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------------------
 38 | ERROR | [x] Ternary operator should be reformatted to more lines.
    |       |     (SlevomatCodingStandard.ControlStructures.RequireMultiLineTernaryOperator.MultiLineTernaryOperatorNotUsed)
------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 235ms; Memory: 20MB

┌───────────────────>

vendor/bin/phpcbf --standard=CodeatCodingStandard --extensions=php --report=full -s demo.php
PHP Warning:  (again, runs twice)
E 1 / 1 (100%)

PHPCBF RESULT SUMMARY
---------------------------------------------------------------------------------------------------
FILE                                                                               FIXED  REMAINING
---------------------------------------------------------------------------------------------------
path_to/demo.php     FAILED TO FIX
---------------------------------------------------------------------------------------------------
A TOTAL OF -1 ERRORS WERE FIXED IN 1 FILE
---------------------------------------------------------------------------------------------------
PHPCBF FAILED TO FIX 1 FILE
---------------------------------------------------------------------------------------------------

Time: 3.23 secs; Memory: 22MB
MikeiLL commented 1 year ago

Using phpcbf with SlevomatStandards is also replacing tabs with four spaces, but only in certain places, so that it generates a bunch of these:

from:

    /**
     * Nothing method
     *
     * This may be set for no reason.
     *
     * @since  2.4.7
     *
     * @param string $nothing does anything here.
     *
     * @access public
     * @var int $nothing does anything here.
     */
    public function nothing($nothing) {
        $this->nothing = $nothing;
    }

Gets "fixed" to:

    /**
     * Nothing method
     *
     * This may be set for no reason.
     *
     * @since  2.4.7
     * @param string $nothing does anything here.
     * @access public
     * @var int $nothing does anything here.
     */

But I think this is because standards recommend four spaces as opposed to tabs. So, that's my bad. Will update VS Code settings.