Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHPMD task does not check all file extensions #1098

Closed
malcomio opened this issue Jul 13, 2023 · 5 comments
Closed

PHPMD task does not check all file extensions #1098

malcomio opened this issue Jul 13, 2023 · 5 comments

Comments

@malcomio
Copy link
Contributor

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 malcomio changed the title PHPMD task does not check other file extensions PHPMD task does not check all file extensions Jul 13, 2023
@malcomio
Copy link
Contributor Author

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

@veewee
Copy link
Contributor

veewee commented Jul 19, 2023

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 added a commit to malcomio/grumphp that referenced this issue Jul 28, 2023
@malcomio
Copy link
Contributor Author

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
Copy link
Contributor

veewee commented Jul 28, 2023

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 added a commit to malcomio/grumphp that referenced this issue Aug 3, 2023
@malcomio
Copy link
Contributor Author

malcomio commented Aug 3, 2023

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.

@veewee veewee closed this as completed in affbb6c Sep 5, 2023
EwenQuim pushed a commit to EwenQuim/grumphp that referenced this issue Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants