phpro / grumphp

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

Weird behavior when running git commit -am #1030

Closed hocrattot closed 2 years ago

hocrattot commented 2 years ago
Q A
Version 1.13.0
Bug? no
New feature? no
Question? yes
Documentation? no
Related tickets -

Hi,

I have installed GrumpPHP on my project and configured only two tasks for the moment (php lint and git_blacklist). It seemed to be all good but then, after modifying a file to create a parse error, I ran git commit -am "test" and the tasks passed.

My configuration

# grumphp.yml
grumphp:
    hooks_dir: .
    git_hook_variables:
        EXEC_GRUMPHP_COMMAND: 'docker run --rm -t -v $(pwd):/var/www -w /var/www -e "TERM=xterm-256color" web'
        ENV: {}
    stop_on_failure: false
    ignore_unstaged_changes: false
    hide_circumvention_tip: true
    ascii:
        failed: ~
        succeeded: ~
    parallel:
        enabled: true
        max_workers: 32
    fixer:
        enabled: true
        fix_by_default: false
    environment:
        files: []
        variables: {}
        paths: []
    tasks:
        phplint: ~
        git_blacklist:
            keywords:
                - "dd\\("
                - "var_dump\\("
            triggered_by: [ 'php' ]
            regexp_type: E
    testsuites: []
    extensions: []

Steps to reproduce:

Modify a php file adding a line with just :

echo 'test'

So that it will make a parse error as there is no ";"
Then :

git commit -am "Test"

Result:

GrumPHP detected a pre-commit command.
GrumPHP is sniffing your code!

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

Running task 1/2: phplint...
Running task 2/2: git_blacklist...
GrumPHP detected a commit-msg command.
GrumPHP is sniffing your code!
[branch-git-check 3aa9b83b3] test
 1 file changed, 1 insertion(+), 1 deletion(-)

If I run git add . and then git commit -m "test", GrumPHP will give the expected results and will not let me commit the file. All other ways like git add . && git commit -m "test" will also work as expected.

hocrattot commented 2 years ago

Ok, I could finally figure it out. In my EXEC_GRUMPHP_COMMAND option, my docker command was ran with the -t flag. I needed to use the -i flag so that docker keeps the STDIN

So I replaced this

docker run --rm -t -v $(pwd):/var/www -w /var/www -e "TERM=xterm-256color" web

by this :

docker run --rm -i -v $(pwd):/var/www -w /var/www web

It is now all good