Files/FileList: adding the same file twice should not increment FileList::$numFiles

This commit fixes the `FileList::__construct()` and
`FileList::addFile()` methods to ensure that, when there is an attempt
to add the same file twice, the `FileList::$numFiles` variable is not
incremented.

The code was already handling duplicates correctly in the sense that
duplicated files were not added to `FileList::$files`. However,
`FileList::$numFiles` was being incorrectly incremented.

There is some duplicated logic in `FileList::__construct()` and
`FileList::addFile()`. I considered refactoring the duplicated code to a
private method before adding the check that is added in this commit. I
decided not to do it as there are some subtle differences between the
logic in the two methods.

`FileList::__construct()` always sets the value of an entry in the
`FileList::$files` to `null` and the key to whatever is returned by
`SplFileInfo::getPathname()`. While `FileList::addFile()` sets the value
of an entry in the `FileList::$files` to `null` or an object passed to
the method and the key to the path passed to the method.

I decided to leave this refactor to remove the duplication to the future
and focus this commit on fixing the issue with handling duplicated
files.

Author: rodrigoprimo

src/Files/FileList.php

@@ -91,7 +91,14 @@ public function __construct(Config $config, Ruleset $ruleset)
                 $iterator = new RecursiveIteratorIterator($filter);
 
                 foreach ($iterator as $file) {
-                    $this->files[$file->getPathname()] = null;
+                    $pathname = $file->getPathname();
+
+                    if (array_key_exists($pathname, $this->files) === true) {
+                        // The path has already been added.
+                        continue;
+                    }
+
+                    $this->files[$pathname] = null;
                     $this->numFiles++;
                 }
             } else {
@@ -132,6 +139,11 @@ public function addFile($path, $file=null)
         $iterator = new RecursiveIteratorIterator($filter);
 
         foreach ($iterator as $path) {
+            if (array_key_exists($path, $this->files) === true) {
+                // The path has already been added.
+                continue;
+            }
+
             $this->files[$path] = $file;
             $this->numFiles++;
         }

tests/Core/Files/FileList/AddFileTest.php

@@ -102,4 +102,36 @@ public static function dataAddFile()
     }//end dataAddFile()
 
 
+    /**
+     * Test that it is not possible to add the same file twice.
+     *
+     * @return void
+     */
+    public function testAddFileShouldNotAddTheSameFileTwice()
+    {
+        $file1         = 'test1.php';
+        $file2         = 'test2.php';
+        $expectedFiles = [
+            $file1,
+            $file2,
+        ];
+
+        // Add $file1 once.
+        $this->fileList->addFile($file1);
+        $this->assertCount(1, $this->fileList);
+        $this->assertSame([$file1], array_keys(iterator_to_array($this->fileList)));
+
+        // Try to add $file1 again. Should be ignored.
+        $this->fileList->addFile($file1);
+        $this->assertCount(1, $this->fileList);
+        $this->assertSame([$file1], array_keys(iterator_to_array($this->fileList)));
+
+        // Add $file2. Should be added.
+        $this->fileList->addFile($file2);
+        $this->assertCount(2, $this->fileList);
+        $this->assertSame($expectedFiles, array_keys(iterator_to_array($this->fileList)));
+
+    }//end testAddFileShouldNotAddTheSameFileTwice()
+
+
 }//end class

tests/Core/Files/FileList/ConstructTest.php

@@ -100,11 +100,11 @@ public static function dataConstruct()
         $fixturesDir = __DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR;
 
         return [
-            'No files'    => [
+            'No files'                                     => [
                 'files'         => [],
                 'expectedFiles' => [],
             ],
-            'Two files'   => [
+            'Two files'                                    => [
                 'files'         => [
                     'file1.php',
                     'file2.php',
@@ -114,13 +114,32 @@ public static function dataConstruct()
                     'file2.php',
                 ],
             ],
-            'A directory' => [
+            'A directory'                                  => [
                 'files'         => [$fixturesDir],
                 'expectedFiles' => [
                     $fixturesDir.'file1.php',
                     $fixturesDir.'file2.php',
                 ],
             ],
+            'Same file twice'                              => [
+                'files'         => [
+                    'file1.php',
+                    'file1.php',
+                ],
+                'expectedFiles' => [
+                    'file1.php',
+                ],
+            ],
+            'File and then directory containing that file' => [
+                'files'         => [
+                    $fixturesDir.'file1.php',
+                    $fixturesDir,
+                ],
+                'expectedFiles' => [
+                    $fixturesDir.'file1.php',
+                    $fixturesDir.'file2.php',
+                ],
+            ],
         ];
 
     }//end dataConstruct()