Skip to content
New issue

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

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

Already on GitHub? # to your account

Add support for multiple phpmd rulesets #164

Merged
merged 1 commit into from
Apr 8, 2019
Merged

Add support for multiple phpmd rulesets #164

merged 1 commit into from
Apr 8, 2019

Conversation

guvra
Copy link
Contributor

@guvra guvra commented Apr 3, 2019

Hi!

Currently, the .phpqa file only allows to specify one PHPMD ruleset, whereas phpmd allows to specify multiple rulesets.

For example: bin/phpmd src/ xml cleancode,codesize,design,naming,unusedcode --suffixes php

This PR adds support for specifying multiple rulesets, and preserves backwards-compatibility.
To allow specifying default rulesets (e.g. unusedcode), I also removed the call to the path function.

The following configurations would all work:

phpmd:
    standard: vendor/path/to/a/ruleset.xml
phpmd:
    standard:
        - vendor/path/to/a/ruleset.xml
        - vendor/path/to/another/ruleset.xml
phpmd:
    standard:
        - cleancode
        - codesize
        - design
        - naming
        - unusedcode

Copy link
Member

@zdenekdrahos zdenekdrahos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks useful. Review:

@guvra
Copy link
Contributor Author

guvra commented Apr 4, 2019

Hi @zdenekdrahos,

I made the following changes:

  • added documentation in the README.md
  • added a comment in the .phpqa.xml file
  • used escapePath function to make sure the argument value is quoted in the command line

For example, with the following config:

phpmd:
  standard:
    - test with spaces/naming.xml
    - unusedcode

The argument in the command line will be:

"test with spaces/naming.xml,unusedcode"

@zdenekdrahos zdenekdrahos self-requested a review April 6, 2019 08:07
@zdenekdrahos
Copy link
Member

zdenekdrahos commented Apr 6, 2019

I'm sorry I haven't noticed it before, but dropping $this->config->path is BC. File path is relative to .phpqa.yml file.

Experiment

#!/bin/sh

TEST_DIR="phpmdtest"
STANDARD=${1:-"phpmd.xml"}

run () {
    init_dir
    configure_phpqa
    run_phpqa
}

init_dir() {
    rm -rf $TEST_DIR
    mkdir $TEST_DIR
}

configure_phpqa () {
    cp app/phpmd.xml $TEST_DIR
    cat > $TEST_DIR/.phpqa.yml << EOL
phpmd:
    standard: $STANDARD
EOL
}

run_phpqa () {
    ./phpqa --verbose --report --config $TEST_DIR --tools phpmd:0
}

run

Current results

$ bash phpmd-test.sh 
 [FileSystem\CleanDir] Cleaned build/
 [FileSystem\FilesystemStack] mkdir ["build\/"]
 [Edge\QA\Task\ParallelExec] "/root-dir/vendor/bin/phpmd" "./" xml "phpmd.xml"  --exclude /vendor/ --suffixes php --reportfile "build//phpmd.xml"
 1/1 [============================] 100%
 [Edge\QA\Task\ParallelExec] Output for  "/root-dir/vendor/bin/phpmd" "./" xml "phpmd.xml"  --exclude /vendor/ --suffixes php --reportfile "build//phpmd.xml" 

Cannot find specified rule-set "phpmd.xml".

Expected results

$ bash phpmd-test.sh 
 [FileSystem\CleanDir] Cleaned build/
 [FileSystem\FilesystemStack] mkdir ["build\/"]
 [Edge\QA\Task\ParallelExec] "/root-dir/vendor/bin/phpmd" "./" xml "/root-dir/phpmdtest/phpmd.xml"  --exclude /vendor/ --suffixes php --reportfile "build//phpmd.xml"
 1/1 [============================] 100%
 [Edge\QA\Task\ParallelExec] Output for  "/root-dir/vendor/bin/phpmd" "./" xml "/root-dir/phpmdtest/phpmd.xml"  --exclude /vendor/ --suffixes php --reportfile "build//phpmd.xml" 
$ bash phpmd-test.sh "[unusedcode,phpmd.xml]"
 [FileSystem\CleanDir] Cleaned build/
 [FileSystem\FilesystemStack] mkdir ["build\/"]
 [Edge\QA\Task\ParallelExec] "/root-dir/vendor/bin/phpmd" "./" xml "unusedcode,/root-dir/phpmdtest/phpmd.xml"  --exclude /vendor/ --suffixes php --reportfile "build//phpmd.xml"
 1/1 [============================] 100%
 [Edge\QA\Task\ParallelExec] Output for  "/root-dir/vendor/bin/phpmd" "./" xml "unusedcode,/root-dir/phpmdtest/phpmd.xml"  --exclude /vendor/ --suffixes php --reportfile "build//phpmd.xml" 

Possible Implementation

I would add this method below path function in Config.php. Then you can call \Edge\QA\escapePath(implode(',', $this->config->pathsOrValues('phpmd.standard'))) in phpmd class and everything should work as expected.

    public function pathsOrValues($path)
    {
        return $this->get(
            $path,
            function ($values, $dir) {
                return array_map(
                    function ($pathOrValue) use ($dir) {
                        $realpath = realpath("{$dir}{$pathOrValue}");
                        return $realpath ? $realpath : $pathOrValue;
                    },
                    (array) $values
                );
            }
        );
    }

@guvra
Copy link
Contributor Author

guvra commented Apr 8, 2019

@zdenekdrahos Thanks for the input, I updated the commit with the modification you suggested :)

@zdenekdrahos zdenekdrahos merged commit 69e4f61 into EdgedesignCZ:master Apr 8, 2019
@guvra guvra deleted the feat-multiple-phpmd-rulesets branch April 15, 2019 11:36
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants