phpro / grumphp

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

Use of multiple matchers not quite right #391

Closed phptek closed 6 years ago

phptek commented 7 years ago
Q A
Version 0.11.6
Bug? yes
New feature? no
Question? no
Documentation? yes
Related tickets N/A

I would like to limit the no. chars used in a commit message as well as use sub-patterns to dictate the message format. This cannot be done in PCRE (The interval quantifier can only be used against the preceding pattern). As such, according to the GrumPHP docs, I can use multiple patterns in the YML matchers array. The problem is that these would seem to be logically OR'd rather than AND'd as I had naively assumed. (See YML file below for use-case).

My configuration

# grumphp.yml
parameters:
  bin_dir: "./vendor/bin"
  git_dir: "." 
  stop_on_failure: false
  ignore_unstaged_changes: true
  tasks:
    # First everything which lints, secures or verifies something
    jsonlint:
      metadata:
        priority: 100 
    xmllint:
      metadata:
        priority: 100 
    git_blacklist:
      keywords:
        - "die"
        - "var_"
        - "exit"
        - "Debug::"
        - "_GET["
        - "_POST["
        - "_REQUEST["
        - "localhost"
        - ".dev"
        - ".local"
        - "phpinfo"
        - "<<<<"
        - "===="
        - ">>>>"
        - "gitlab"
      metadata:
        priority: 90
    # See also ./ruleset.xml
    phpcs:
      standard: 'ruleset.xml'
      encoding: 'UTF-8'
      show_warnings: false
      # Filename patterns to ignore
      ignore_patterns:
        - "spec/*Spec.php"
        - "tests/*.php"
      metadata:
        priority: 90
        # Ensure commit messages are standard. Helps us when future debugging to see the story to which the commit relates
        # See here for an interesting thread on commit-message length: https://stackoverflow.com/questions/2290016/git-commit-messages-50-72-formatting
    git_conflict: ~
    git_commit_message:
      matchers:
        # PCRE does not allow use interval quantifiers against sub-patterns so we use x2 matchers:
        # 1). Check format
        # 2). Check length
        - '/^(INIT|NEW|MINOR|FIX|API):\s(RM|WR)\#[\d]+\s.+$/'
        - '/^.{20,72}$/'
      multiline: true
  extensions: []

Steps to reproduce:

# Generate empty folder
mkdir tmp
cd tmp
git init
echo "vendor" > .gitignore
pbpaste > grumphp.yml
composer require --dev phpro/grumphp

# 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:

# Please add the result of the run or git commit actions here.

Using the following commit message which is 73 characters long, Grumphp allows it, but according to rule 2 of matchers, this should fail:

NEW: ID#1235 This is a test. This is a test this is a test this is a test
veewee commented 7 years ago

Hello @phptek,

There must be something wrong in the configuration. As you can see here, we do check for all patterns: https://github.com/phpro/grumphp/blob/master/src/Task/Git/CommitMessage.php#L136-L142

Since v0.12, we also have some extra options to validate the message length etc. You can probably solve your configuration by using one of the new configuration options. More information can be found here: https://github.com/phpro/grumphp/blob/master/doc/tasks/git_commit_message.md

veewee commented 6 years ago

Hi @phptek , Since this question hasn't had an update for more then a half year, I am closing this issue. Feel free to provide any additional feedback!