Fixed handling the "exclude-pattern" parameter on Windows

As of squizlabs/PHP_CodeSniffer#1488, Filter::accept() doesn't strictly check the real path of the files, so intstead of using reflection and calling its protected methods, we can use the filter itself. Closes #4.
diff-sniffer · Mar 1, 2019 · 33730c0 · 33730c0
1 parent b7bd346
commit 33730c0
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 48 deletions.
diff --git a/src/Filter.php b/src/Filter.php
diff --git a/src/Iterator.php b/src/Iterator.php
@@ -2,15 +2,17 @@
 
 namespace DiffSniffer;
 
-use EmptyIterator;
+use function iterator_to_array;
 use IteratorIterator;
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\DummyFile;
 use PHP_CodeSniffer\Files\File;
-use PHP_CodeSniffer\Filters\Filter as BaseFilter;
+use PHP_CodeSniffer\Filters\Filter;
 use PHP_CodeSniffer\Ruleset;
 use RecursiveArrayIterator;
 use Traversable;
+use const DIRECTORY_SEPARATOR;
+use function str_replace;
 
 /**
  * Changeset iterator
@@ -47,15 +49,28 @@ public function __construct(Traversable $files, Changeset $changeSet, Ruleset $r
 
     public function getIterator() : Traversable
     {
-        $it = new IteratorIterator($this->files);
-        $it = new Filter($it, new BaseFilter(
-            new RecursiveArrayIterator(
-                new EmptyIterator()
-            ),
-            '',
+        // PHP_CodeSniffer expects file paths to contain the native directory separator on Windows when matching them
+        // against the exclude pattern but Git and GitHub REST API will return forward slashes regardless of the OS
+        if (DIRECTORY_SEPARATOR === '\\') {
+            $it = (function () : iterable {
+                foreach ($this->files as $file) {
+                    yield str_replace('/', DIRECTORY_SEPARATOR, $file);
+                }
+            })();
+        } else {
+            $it = new IteratorIterator($this->files);
+        }
+
+        $it = new RecursiveArrayIterator(
+            iterator_to_array($it)
+        );
+
+        $it = new Filter(
+            $it,
+            "\0",
             $this->config,
             $this->ruleSet
-        ));
+        );
 
         foreach ($it as $path) {
             yield $this->createFile(

diff --git a/tests/ApplicationTest.php b/tests/ApplicationTest.php
@@ -7,6 +7,7 @@
 use DiffSniffer\Command;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use const DIRECTORY_SEPARATOR;
 
 class ApplicationTest extends TestCase
 {
@@ -16,9 +17,9 @@ class ApplicationTest extends TestCase
      */
     public function testUseCase(string $useCase, int $expectedExitCode)
     {
-        $dir = __DIR__ . '/fixtures/' . $useCase;
+        $dir = __DIR__ . DIRECTORY_SEPARATOR . 'fixtures' . DIRECTORY_SEPARATOR . $useCase;
         $changeset = new FixtureChangeset($dir);
-        chdir($dir . '/tree');
+        chdir($dir . DIRECTORY_SEPARATOR . 'tree');
 
         /** @var Command|MockObject $command */
         $command = $this->createMock(Command::class);
@@ -27,7 +28,7 @@ public function testUseCase(string $useCase, int $expectedExitCode)
             ->willReturn($changeset);
         $app = new Application();
 
-        $expectedOutput = file_get_contents($dir . '/output.txt');
+        $expectedOutput = file_get_contents($dir . DIRECTORY_SEPARATOR . 'output.txt');
         $expectedOutput = str_replace("\n", PHP_EOL, $expectedOutput);
 
         $this->expectOutputString($expectedOutput);
@@ -47,6 +48,10 @@ public static function useCaseProvider()
                 'excluded-rule-cache',
                 0,
             ],
+            'exclude-pattern' => [
+                'exclude-pattern',
+                1,
+            ],
         ];
     }
 }
diff --git a/tests/FixtureChangeset.php b/tests/FixtureChangeset.php
@@ -3,6 +3,7 @@
 namespace DiffSniffer\Tests;
 
 use DiffSniffer\Changeset;
+use const DIRECTORY_SEPARATOR;
 
 /**
  * Fixture changeset
@@ -29,7 +30,7 @@ public function __construct(string $dir)
      */
     public function getDiff() : string
     {
-        return file_get_contents($this->dir . '/changeset.diff');
+        return file_get_contents($this->dir . DIRECTORY_SEPARATOR . 'changeset.diff');
     }
 
     /**

diff --git a/tests/fixtures/exclude-pattern/changeset.diff b/tests/fixtures/exclude-pattern/changeset.diff
@@ -0,0 +1,18 @@
+diff --git a/main.php b/main.php
+`--- /dev/null
++++ b/main.php
+@@ -0,0 +1,5 @@
++<?php
++
++if ( ! isset($foo)) {
++    $foo = 'bar';
++}
+diff --git a/excluded/excluded.php b/excluded/excluded.php
+`--- /dev/null
++++ b/excluded/excluded.php
+@@ -0,0 +1,5 @@
++<?php
++
++if ( ! isset($foo)) {
++    $foo = 'bar';
++}
diff --git a/tests/fixtures/exclude-pattern/output.txt b/tests/fixtures/exclude-pattern/output.txt
@@ -0,0 +1,10 @@
+
+FILE: main.php
+----------------------------------------------------------------------
+FOUND 1 ERROR AFFECTING 1 LINE
+----------------------------------------------------------------------
+ 3 | ERROR | [x] Expected 0 spaces after opening bracket; 1 found
+----------------------------------------------------------------------
+PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
+----------------------------------------------------------------------
+
diff --git a/tests/fixtures/exclude-pattern/tree/excluded/excluded.php b/tests/fixtures/exclude-pattern/tree/excluded/excluded.php
@@ -0,0 +1,5 @@
+<?php
+
+if ( ! isset($foo)) {
+    $foo = 'bar';
+}
diff --git a/tests/fixtures/exclude-pattern/tree/main.php b/tests/fixtures/exclude-pattern/tree/main.php
@@ -0,0 +1,5 @@
+<?php
+
+if ( ! isset($foo)) {
+    $foo = 'bar';
+}
diff --git a/tests/fixtures/exclude-pattern/tree/phpcs.xml b/tests/fixtures/exclude-pattern/tree/phpcs.xml
@@ -0,0 +1,5 @@
+<?xml version="1.0"?>
+<ruleset>
+    <exclude-pattern>excluded/*</exclude-pattern>
+    <rule ref="PSR2"/>
+</ruleset>