phpro / grumphp

A PHP code-quality tool
MIT License
4.15k stars 431 forks source link

PHPMD task does not check all file extensions #1098

Closed malcomio closed 1 year ago

malcomio commented 1 year ago
Q A
Version 2.0.0
Bug? yes
New feature? no
Question? no
Documentation? no
Related tickets comma-separated list of related tickets

My configuration See https://github.com/malcomio/grumphp-test/blob/main/grumphp.yml

Steps to reproduce:

  1. clone https://github.com/malcomio/grumphp-test
  2. composer install in the repo directory
  3. run vendor/bin/grumphp run --tasks phpmd

Expected result Problems should be reported in test.module and test.php, as they are when running PHPMD directly with the --suffixes flag:

➜  grumphp-test git:(main) ✗ vendor/bin/phpmd docroot ansi phpmd.xml --suffixes 'php,module'                                                                  16:05:55

FILE: /Users/mayoung/Code/grumphp-test/docroot/test.module
----------------------------------------------------------
 4 | VIOLATION | Avoid variables with short names like $i. Configured minimum length is 3.
 4 | VIOLATION | Avoid unused local variables such as '$i'.

FILE: /Users/mayoung/Code/grumphp-test/docroot/test.php
-------------------------------------------------------
 4 | VIOLATION | Avoid variables with short names like $i. Configured minimum length is 3.
 4 | VIOLATION | Avoid unused local variables such as '$i'.

Found 4 violations and 0 errors in 223ms

Actual result Problems are only reported in test.php

phpmd
=====

FILE: /Users/mayoung/Code/grumphp-test/docroot/test.php
-------------------------------------------------------
 4 | VIOLATION | Avoid variables with short names like $i. Configured minimum length is 3.

Found 1 violation and 0 errors in 222ms
malcomio commented 1 year ago

This seems to be specific to certain file extensions - .inc is checked as expected, but .module and .install are not

veewee commented 1 year ago

The phpmd task currently has no support for the --suffixes flag. It shouldn't be too hard to add it. Care to give it a spin?

https://github.com/phpro/grumphp/blob/v2.x/src/Task/PhpMd.php

malcomio commented 1 year ago

As far as I can see, the changes in https://github.com/phpro/grumphp/compare/v2.x...malcomio:grumphp:extensions?expand=1 should work, but the other file types are still not being checked

debugging inside \GrumPHP\Process\ProcessBuilder::buildProcess, I can see the arguments being set as I'd expect:

    [commandline:Symfony\Component\Process\Process:private] => Array
        (
            [0] => /Users/mayoung/Code/grumphp-test/vendor/bin/phpmd
            [1] => docroot/test.inc,docroot/test.install,docroot/test.module,docroot/test.php
            [2] => ansi
            [3] => phpmd.xml
            [4] => --suffixes 'php,module,inc,install,test,profile,theme'
        )

I must be missing something...

veewee commented 1 year ago

The process building handles escaping for you per argument. You don't need to add quotes yourself. You probably need to make sure that --suffixes and the list of extensions are 2 separate arguments? (I don't use that tool, so you might need to consult the docs of phpmd on how to use that flag)

malcomio commented 1 year ago

Thanks - they did indeed need to be separate arguments

I've created #1103 with these changes

relevant PHPMD docs are at https://phpmd.org/documentation/index.html

--suffixes - Comma-separated string of valid source code filename extensions, e.g. php, phtml.