search

YouweGit / testing-suite

Collection of PHP code testing packages.
MIT License
11 stars 9 forks source link

False positive with 'alert' on PHP files #18

Closed viniciusfabri closed 1 year ago

viniciusfabri commented 1 year ago

Steps to reproduce

  1. Add the following line to a PHP class 
    $this->logger->alert('bla');
  2. Run testing suite

Expected result

Testing suite passses

Actual result

It fails with a 'git_blacklist' error

Additional info

There's a generic 'alert(' blacklist that should only apply to JS files I think: https://github.com/YouweGit/testing-suite/blob/master/config/magento2/grumphp.yml#L11

rutgerrademaker commented 1 year ago

@viniciusfabri

Maybe we could add some regex to allow for ->alert( while disallowing alert( Played a little bit around with this, but could not get it to work properly I'm no regex hero, but this should be correct I think (?<![->]{2})alert( or (?<![->]{2})alert[\x28] as it seems grumphp has some issues with regxexes that needs to be defined /escaped in yaml

See also https://github.com/phpro/grumphp/blob/master/doc/tasks/git_blacklist.md

Another Idea I came up with would be to split the tasks / lists to be used per language (php/js/phtml) Have not tested if that is a feasible option yet

viniciusfabri commented 1 year ago

New false positive: image