phpro / grumphp

A PHP code-quality tool
MIT License
4.11k stars 429 forks source link

Grumphp config using default PHPCS XML and other standard's sniff #1118

Closed carltondickson closed 5 months ago

carltondickson commented 5 months ago
Q A
Version vendor/bin/grumphp -V
Bug? no
New feature? no
Question? yes
Documentation? no
Related tickets comma-separated list of related tickets

I've added another standard's rule to my .phpcs.xml file and can see phpcs CLI will tell me there's an error. However on commit grumphp seems to let commit through...is it possible to use just one sniff from another standard and does grumphp know how to recognise this?

My configuration

# grumphp.yml

# Not specifying a "standard" so it falls back to `.phpcs.xml` which it definitely does. I can change the standard in that file to some nonsense (e.g. PSR69) and it will error out

    tasks:
        phpcs:
            standard: []
            severity: ~
            error_severity: ~
            warning_severity: 0
            tab_width: ~
            report: full
            report_width: ~
            whitelist_patterns: []
            encoding: ~
            ignore_patterns: [ ]
            sniffs: []
            triggered_by: [ php ]
            exclude: [ ]

## .phpcs.xml

    <!-- Define rules below -->
    <rule ref="PSR2"/>

    <!-- relative path from PHPCS source location -->
    <config name="installed_paths" value="../../slevomat/coding-standard"/>

    <!-- Commenting -->
    <rule ref="SlevomatCodingStandard.Commenting.UselessFunctionDocComment">
        <include-pattern>src/SomeClass</include-pattern>
    </rule>

## Other notes

# I can do something like this and the standard is recognised by grumphp but there are too many strict rules in that standard to use it as is.

    <rule ref="PSR2"/>
    <rule ref="SlevomatCodingStandard"/>

Steps to reproduce:


# PHPCS fails as expected

vendor/bin/phpcs --report=full src/SomeClass/

E. 2 / 2 (100%)

FILE: /app/src/SomeClass.php
-----------------------------------------------------------------------------------------------------------------
FOUND 13 ERRORS AFFECTING 12 LINES
-----------------------------------------------------------------------------------------------------------------
 226 | ERROR | [x] Method SomeClass::aTest() does not need documentation comment.
 236 | ERROR | [x] Method SomeClass::bTest() does not need documentation comment.

Result:

# Please add the result of the run or git commit actions here.

## Grumphp doesn't seem to utilise my grumphp config and the .phpcsl.xml rulset as phpcs does.

./vendor/bin/grumphp run --tasks phpcs -vvv

GrumPHP is sniffing your code!

Running tasks with priority 0!
==============================

Running task 1/1: phpcs... 

Running task 1/1: phpcs... ✔
veewee commented 5 months ago

Hello @carltondickson,

GrumPHP only passes the arguments to phpcs. So assuming you don't provide any standard in grumphp, it will take the standards that are defined in the configuration file. I'm not sure what is going wrong with the given information.

Can you debug what phpcs command is being executed exactly from grumphp and what happens if you run that exact same command with phpcs directly?

You should be able to figure it out by:

That should give you the same results and a starting point from where to figure out what is going wrong.

carltondickson commented 5 months ago

@veewee thanks for your response, it helped me get to the issue.

I went through the process with a new project and could see that the call to phpcs looked fine after debugging grumphp commandline that was created.

As I was stripping out old phpcs config and directories I deleted <config name="ignore_errors_on_exit" value="1"/> and think this was the issue.

https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#ignoring-errors-when-generating-the-exit-code

I ported CLI args from our build --runtime-set ignore_warnings_on_exit 1 --runtime-set ignore_errors_on_exit 1 to a .phpcs.xml config file a long time ago which is why I think they are in there.

I think this flag was initially added because phpcs wasn't reporting to Github (will need to investigate this)