phpro / grumphp

A PHP code-quality tool
MIT License
4.14k stars 430 forks source link

Grumphp Stripping away important signs for phpcsFixer2 rules (@ and making it lowercase) #281

Closed j3rrey closed 7 years ago

j3rrey commented 7 years ago
Q A
Branch master
Bug? yes
New feature? no
Question? yes/no
Documentation? no

I opened the issue on the PHP CS Fixer Repo

https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/2518

I'm digging into the Grumphp code trying to find the place where the @ get's stripped away

My configuration

parameters:
    bin_dir: "./vendor/bin"
    git_dir: "."
    hooks_dir: ~
    hooks_preset: local
    stop_on_failure: false
    ignore_unstaged_changes: false
    process_async_limit: 10
    process_async_wait: 1000
    process_timeout: 60
    ascii:
        failed: grumphp-grumpy.txt
        succeeded: grumphp-happy.txt
    tasks:
        git_blacklist:
            keywords:
                - "die("
                - "var_dump("
                - "print_f("
                - "dump("
                - "dd("
                - "exit;"
            triggered_by: ["php"]
        git_commit_message:
            matchers:
                - /SER-([0-9]*)/
            case_insensitive: true
            multiline: true
            additional_modifiers: ''
        git_conflict: ~
        phpcsfixer2:
            allow_risky: false
            cache_file: ~
            config: ~
            rules: ["@Symfony"]
            using_cache: false
            path_mode: ~
            verbose: true
        phpmd:
            exclude: []
            ruleset: ['cleancode', 'codesize', 'naming']
            triggered_by: ["php"]
        phpparser:
            ignore_patterns: []
            kind: php7
            triggered_by: ["php"]
            visitors: {}
        phpunit: ~
        phpversion:
            project: "7.0"
        shell: ~

    testsuites: []
    extensions: []

One can make a workaround with a config file!

php_cs.dist

<?php

$finder = PhpCsFixer\Finder::create()
    ->exclude(['exclude/path'])
    ->name('*.php');

return PhpCsFixer\Config::create()
    ->setRules([
        '@Symfony' =>  true,
        'encoding' => false,
        'psr0' => false,
        'psr4' => false,
        'self_accessor' => false,
        'no_short_echo_tag' => true,
        'array_syntax' => ['syntax' => 'short'],
    ])
    ->setFinder($finder);

This will execute the cs fixer with the given values and won't strip away any of the rules important characters!

Updated grumphp.yml

parameters:
    bin_dir: "./vendor/bin"
    git_dir: "."
    hooks_dir: ~
    hooks_preset: local
    stop_on_failure: false
    ignore_unstaged_changes: false
    process_async_limit: 10
    process_async_wait: 1000
    process_timeout: 60
    ascii:
        failed: grumphp-grumpy.txt
        succeeded: grumphp-happy.txt
    tasks:
        git_blacklist:
            keywords:
                - "die("
                - "var_dump("
                - "print_f("
                - "dump("
                - "dd("
                - "exit;"
            triggered_by: ["php"]
        git_commit_message:
            matchers:
                - /SER-([0-9]*)/
            case_insensitive: true
            multiline: true
            additional_modifiers: ''
        git_conflict: ~
        phpcsfixer2:
            allow_risky: false
            cache_file: ~
            config: config/php_cs.dist
            rules: []
            using_cache: false
            path_mode: ~
            verbose: true
        phpmd:
            exclude: []
            ruleset: ['cleancode', 'codesize', 'naming']
            triggered_by: ["php"]
        phpparser:
            ignore_patterns: []
            kind: php7
            triggered_by: ["php"]
            visitors: {}
        phpunit: ~
        phpversion:
            project: "7.0"
        shell: ~

    testsuites: []
    extensions: []
veewee commented 7 years ago

Thanks for reporting @j3rrey, GrumPHP is using the Symfony service container. This means that you'll have to escape the @ sign with a double @@ sign.

http://symfony.com/doc/current/service_container.html#service-parameters If you want to use a string that starts with an @ sign as a parameter value (e.g. a very safe mailer password) in a YAML file, you need to escape it by adding another @ sign (this only applies to the YAML format)

Can you validate if this solution is working for you? Is this a common notation for the php-cs-fixer rules? We might add this to the documentation of that parameter.

j3rrey commented 7 years ago

Hey @veewee,

yes this works, thx! It's new in the cs fixer v2 they changed parameters and rules now you use --rule=@PSR2 so on and to make it work with Grumphp you need to escape it as you said with another @ e.g. @@PSR2

My updated grumphp.yml:

parameters:
    bin_dir: "./vendor/bin"
    git_dir: "."
    hooks_dir: ~
    hooks_preset: local
    stop_on_failure: false
    ignore_unstaged_changes: false
    process_async_limit: 10
    process_async_wait: 1000
    process_timeout: 60
    ascii:
        failed: grumphp-grumpy.txt
        succeeded: grumphp-happy.txt
    tasks:
        git_blacklist:
            keywords:
                - "die("
                - "var_dump("
                - "print_f("
                - "dump("
                - "dd("
                - "exit;"
            triggered_by: ["php"]
        git_commit_message:
            matchers:
                - /SER-([0-9]*)/
            case_insensitive: true
            multiline: true
            additional_modifiers: ''
        git_conflict: ~
        phpcsfixer2:
            allow_risky: false
            cache_file: ~
            config: ~
            rules: ['@@Symfony']
            using_cache: false
            path_mode: ~
            verbose: true
        phpmd:
            exclude: []
            ruleset: ['cleancode', 'codesize', 'naming']
            triggered_by: ["php"]
        phpparser:
            ignore_patterns: []
            kind: php7
            triggered_by: ["php"]
            visitors: {}
        phpunit: ~
        phpversion:
            project: "7.0"
        shell: ~

    testsuites: []
    extensions: []