Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 3.7.2

Doc Bug #20369 Unclear usage of regexes in exclude patterns
Submitted: 2014-08-11 12:57 UTC
From: pittiplatsch Assigned: squiz
Status: Closed Package: PHP_CodeSniffer (version 2.0.0RC1)
PHP Version: Irrelevant OS:
Roadmaps: (Not assigned)    
Subscription  


 [2014-08-11 12:57 UTC] pittiplatsch (Lars Heber)
Description: ------------ Hello, due to quite incomplete docs I'm trying to find out the exact possibilities of exclude patterns by inspecting the sources. However, some questions remain: 1. Obviously, exclude patterns are rendered as regexes. In some issues concerning excl-pat's regular expression syntax is recommended (e.g. issue #19648). However, what's the point then of hard-coded string-replace of "\\," to "," and "*" to ".*" before processing the regex? a) I see some convenience to simply use "*" - but this leads to some regexes failing where they shouldn't: The regex "^application/test.*\.php" is changed to "^application/test..*\.php" which leads the file "application/test.php" to fail although my original regex includes this file! b) What about the "\\," to "," thing? 2. Why are exclude patterns processed differently for global use and per-sniff? I have the concrete problem that (using Windows) for global usage both patterns "application\\" and "application/" work perfectly fine, while on per-sniff basis only "application\\" works which obviously prevents portability to *NIX systems.

Comments

 [2014-08-14 11:44 UTC] squiz (Greg Sherwood)
-Status: Open +Status: Feedback -Assigned To: +Assigned To: squiz
1. Comma is used to separate patterns on the command line, so you need to escape any commas you use in the patterns themselves. So I remove the escape chars for commas only. The * -> .* change is just a convenience thing, as was the norm when I added this feature years ago. It's easier to just do -- exclude=*/tests/*. Removing this would be a huge backwards compatibly break, so I'd never do it. 2. I assume you mean this code from where files are excluded: if (DIRECTORY_SEPARATOR === '\\') { $replacements['/'] = '\\\\'; } That code isn't present when doing sniff exclusions. I don't think there is any reason for that and it can be added in easily. Before I do, is this what you are talking about specifically?
 [2014-08-18 17:07 UTC] pittiplatsch
1. Of course I see your reason to avoid the BC break. However, as I stated out, there are cases where the current implementaion leads to serious misbehaviour :-( I already thought about a solution. What about adding a new attribute to the "exclude pattern" element, which - if not set - for BC is something like "convenience" or "easyPattern" or sth. like this, whereas you can explicitly set it to "plain", "regex" or similar which leads to given exclude pattern being handled via regex evaluation as-is. 2. Yep, that's exactly what I mean. I already discovered what you just wrote where the DIR-SEP replacement doesn't take place. From architectural point of view, this functionality should just be consolidated into a single method :-) [As a side note: This behaviour could be made configurable as well via xml element attribute ("cleverDirSep" or the like)]
 [2014-08-21 04:38 UTC] squiz (Greg Sherwood)
1. Sorry, but I'm not going to change this. I don't want to add even more complexity. If you know that * is replaced by .* you should be able to work around it with the current system. I don't think there is serious misbehaviour here. 2. I'll get these checks working the same way.
 [2014-08-21 05:01 UTC] squiz (Greg Sherwood)
I committed the change for part 2 here: https://github.com/squizlabs/PHP_CodeSniffer/commit/587c70b23740802ae029df6a5d 0ddfd7d4f39a1b
 [2014-08-22 15:09 UTC] pittiplatsch
1. Ok, than the usage of exclude patterns (and its limitations) should be clearly documented I think. Due to CS's wide usage - even for auto-deployment - its usage and powerful capabilities shouldn't be hazy ;-) 2. Thx, so this will be available for final 2.0? Thank you very much :-)
 [2014-08-22 16:44 UTC] squiz (Greg Sherwood)
-Status: Feedback +Status: Closed
1. I had actually never documented that ignore patterns can actually be regular expressions (although I've always supported them), but I have done so now and put in a note about the * replacement. The docs are here: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-files- and-folders 2. Yes, it will be in the next 2.0RC2 (probably September) and then in 2.0 stable (probably Oct).
 [2014-08-27 14:51 UTC] pittiplatsch
Thx a lot :-)