phpro / grumphp

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

Check phpstan errors for all files, not only for the commited ones #393

Closed wagnerferreirasp closed 7 years ago

wagnerferreirasp commented 7 years ago
Q A
Version GrumPHP 0.11.6
Bug? yes
New feature? no
Question? no
Documentation? no
Related tickets comma-separated list of related tickets

My configuration

# grumphp.yml
parameters:
    git_dir: .
    bin_dir: vendor/bin
    tasks:     
        phpstan:
            autoload_file: ~
            configuration: "./config/phpstan.neon"
            level: 5
            triggered_by: ['php']

Steps to reproduce:

Your actions

  1. Change a method signature of class A, forcing an error that phpstan catches on other classes.
  2. Commit the class A successfully

Run GrumpHP:

./vendor/bin/grumphp run

Result: The run shows erros for files that were not changed, but uses the method of class A that changed. However, the commit was "All good", probably because the commited file has no errors, but the other were affected by the change of class A.

Bilge commented 7 years ago

GrumPHP's job is to check changed files on commit. If you want to check all files you just provided the command to do this. The point being, you either need to check all files manually, or automatically but using some trigger other than the commit hook (e.g. your CI process). Recommend closing as invalid.

wagnerferreirasp commented 7 years ago

Maybe I was wrong and the issue could be about a new feature. It makes sense that GrumPHP checks changed files only and this apply to many tasks, like copy-paste detector, git_blacklist, phpcs. However, for phpstan, a change in a committed file may affect others and cause a bug we don't want to commit.

The phpstan task inside GrumPHP let me do a lot of things that I do via phpstan's command line, but "run for all files" is not one of them.

I already managed to get it work with GrumPHP, using a composer script that runs phpstan for all files. Now, any change that causes the code to fail at phpstan, GrumPHP won't let me commit and that is great.

Would be nice to have on the phpstan task configuration the list of directories to run, or let me choose for the default (run for changed files).

Thanks anyway!

veewee commented 7 years ago

@wagnerferreirasp,

Thanks for reporting. Currently this is indeed not possible. Since the phpstan task is rather new, I created a ticket to improve the phpstan task (#400). We'll continue the discussion from there.