phpro / grumphp

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

git_blacklist never runs? #1139

Open cerw opened 2 weeks ago

cerw commented 2 weeks ago
Q A
Version GrumPHP 2.5.0
Bug? yes
New feature? no
Question? no
Documentation? no
Related tickets

We noticed that the git_blacklist is not executing it seem its because canRunInContext always return false?

My configuration

grumphp:
  stop_on_failure: true
  process_timeout: 210
  parallel:
    enabled: true
    max_workers: 32
  tasks:
    file_size:
      max_size: 3M
      ignore_patterns: []
    phpcs:
      standard: PSR2
      warning_severity: 0
      error_severity: 1
      ignore_patterns:
        - ./tests
        - ./resources/*
        - ./database
        - ./bootstrap
        - ./public
        - ./config/
    git_branch_name:
      # whitelist:
      #   - '/([0-9]+)-/'
      #   - '/bugfix-*/'
      blacklist:
        - "/(shit|fuck|crap)/"
      # - "develop"
      # - "master"
    git_commit_message:
      allow_empty_message: false
      enforce_capitalized_subject: false
      enforce_no_subject_trailing_period: true
      enforce_single_lined_subject: false
      max_body_width: 400
      max_subject_width: 200
      case_insensitive: true
      multiline: true
      additional_modifiers: ""
    jsonlint: null
    phplint: null
    securitychecker_enlightn:
      lockfile: ./composer.lock
    git_blacklist:
      keywords:
        - "die("
        - "dump("
        - "dd("
        - "var_dump("
        - "exit;"
      # whitelist_patterns:
      #   - add(
      regexp_type: G
      match_word: true
    composer:

Steps to reproduce:

Put dd(); in PHP code, do git add -y run following

nanospa git:(last_bits_on_spa) ✗ git diff --cached app

─────────────────────────────────────────────────────────────────────────────────┐
app/Listeners/LogUserHasLoggedOff.php:29: public function handle(Logout $logout) │
─────────────────────────────────────────────────────────────────────────────────┘
 29 ⋮ 29 │            // no user is logged in
 30 ⋮ 30 │            return;
 31 ⋮ 31 │        }
    ⋮ 32 │        dd();
 32 ⋮ 33 │        activity()
 33 ⋮ 34 │            ->useLog('auth')
 34 ⋮ 35 │            ->event('logout')
./vendor/bin/grumphp run 

# Your actions
# Please add the steps on how to reproduce the issue here.

# Run GrumPHP:
git add -A && git commit -m"Test"
# or
./vendor/bin/grumphp run

Result:

➜  nanospa git:(last_bits_on_spa) ✗ ./vendor/bin/grumphp run
GrumPHP is sniffing your code!

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

Running task 1/7: file_size... ✔
Running task 2/7: phpcs... ✔
Running task 3/7: git_branch_name... ✔
Running task 4/7: jsonlint... ✔
Running task 5/7: phplint... ✔
Running task 6/7: securitychecker_enlightn... ✔
Running task 7/7: composer... ✔
             ▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
           ▄▄▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
         ▄▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
        ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
        ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
  ▄▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
 ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
 ▐█▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
   ▀█▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
     ▀▀▓▓▓▓▓▓▓▓▓▓▓▓█▀▀▀▀▀▀▀▀▀▀▀▀▀▀████████████▄
      ▄████████▀▀▀▀▀                 ██████████
     ███████▀                         ██████▀
      ▐████      ██▌          ██       ████▌
        ▐█▌                            ███
         █▌           ▄▄ ▄▄           ▐███
        ███       ▄▄▄▄▄▄▄▄▄▄▄▄       ▐███
         ██▄ ▐███████████████████████████
        █▀█████████▌▀▀▀▀▀▀▀▀▀██████████▌▐
          ███████████▄▄▄▄▄▄▄███████████▌
         ▐█████████████████████████████
          █████████████████████████████
           ██ █████████████████████▐██▀
            ▀ ▐███████████████████▌ ▐▀
                ████▀████████▀▐███
                 ▀█▌  ▐█████  ▐█▌
                        ██▀   ▐▀
       _    _ _                         _ _
      / \  | | |   __ _  ___   ___   __| | |
     / _ \ | | |  / _` |/ _ \ / _ \ / _` | |
    / ___ \| | | | (_| | (_) | (_) | (_| |_|
   /_/   \_\_|_|  \__, |\___/ \___/ \__,_(_)
                  |___/
veewee commented 2 weeks ago

Hello,

It is not executed during grumphp run since it uses the git staged files to detect these keywords. it only runs during pre-commit.

cerw commented 2 weeks ago

Hello,

It is not executed during grumphp run since it uses the git staged files to detect these keywords. it only runs during pre-commit.

Hello there,

Thanks for getting to me so fast, true ! but still not failing ?

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

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

Running task 1/8: file_size... ✔
Running task 2/8: phpcs... ✔
Running task 3/8: git_branch_name... ✔
Running task 4/8: jsonlint...
Running task 5/8: phplint... ✔
Running task 6/8: securitychecker_enlightn...
Running task 7/8: git_blacklist...
Running task 8/8: composer...
🤖 Skipping gptcommit because the githook isn't set up for the "Message" commit mode.
GrumPHP detected a commit-msg command.
GrumPHP is sniffing your code!

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

Running task 1/1: git_commit_message... ✔

Changes:

➜ nanospa git:(last_bits_on_spa) git diff b31aeafbf0a35c506cd4d3828113e7b7f192865b

─────────────────────────────────────────────────────────────────────────────────┐ app/Listeners/LogUserHasLoggedOff.php:29: public function handle(Logout $logout) │ ─────────────────────────────────────────────────────────────────────────────────┘ 29 ⋮ 29 │ // no user is logged in 30 ⋮ 30 │ return; 31 ⋮ 31 │ } 32 ⋮ │ dd(1); ⋮ 32 │ dd(2); 33 ⋮ 33 │ activity() 34 ⋮ 34 │ ->useLog('auth') 35 ⋮ 35 │ ->event('logout')

─────────────────────────┐ grumphp.yml:48: grumphp: │ ─────────────────────────┘ 48 ⋮ 48 │ - "dd(" 49 ⋮ 49 │ - "var_dump(" 50 ⋮ 50 │ - "exit;" 51 ⋮ │ # whitelist_patterns: 52 ⋮ │ # - add( ⋮ 51 │ whitelist_patterns: ⋮ 52 │ - add( 53 ⋮ 53 │ regexp_type: G 54 ⋮ │ match_word: true ⋮ 54 │ match_word: false 55 ⋮ 55 │ composer:



What has changed?
veewee commented 2 weeks ago

It might be so, that you need to apply keyword escaping, since ( is a reserved regex character. See examples here: https://github.com/phpro/grumphp/blob/v2.x/doc/tasks/git_blacklist.md

cerw commented 2 weeks ago

Hmm can not get it work, it runs but it has no tick?

image

Tried with " without, with match_word on and off..

grumphp:
  stop_on_failure: true
  process_timeout: 210
  parallel:
    enabled: true
    max_workers: 32
  tasks:
    file_size:
      max_size: 3M
      ignore_patterns: []
    phpcs:
      standard: PSR2
      warning_severity: 0
      error_severity: 1
      ignore_patterns:
        - ./tests
        - ./resources/*
        - ./database
        - ./bootstrap
        - ./public
        - ./config/
    git_branch_name:
      # whitelist:
      #   - '/([0-9]+)-/'
      #   - '/bugfix-*/'
      blacklist:
        - "/(shit|fuck|crap)/"
      # - "develop"
      # - "master"
    git_commit_message:
      allow_empty_message: false
      enforce_capitalized_subject: false
      enforce_no_subject_trailing_period: true
      enforce_single_lined_subject: false
      max_body_width: 400
      max_subject_width: 200
      case_insensitive: true
      multiline: true
      additional_modifiers: ""
    jsonlint: null
    phplint: null
    securitychecker_enlightn:
      lockfile: ./composer.lock
    git_blacklist:
      keywords:
        - die\\(
        - dump\\(
        - dd\\(
        - var_dump\\(
        - exit;
      whitelist_patterns:
        - add(
      regexp_type: G
      match_word: false
    composer:
veewee commented 2 weeks ago

Not sure what's going wrong there, it seems to be working on my specific setup. Maybe you'dd rather use something like https://github.com/phpro/grumphp/blob/v2.x/doc/tasks/phpparser.md

Or phpstan with https://github.com/ekino/phpstan-banned-code.

Both use PHP's AST to figure out if it's allowed or not which is less error prone on input like dump[space]( Whilst the blacklist task only checks for a regex match.