From 4b11b60f9b5f3cafbb0d9f258e91c3e68e4ef5af Mon Sep 17 00:00:00 2001
From: Rodrigo Primo <rodrigosprimo@gmail.com>
Date: Wed, 25 Jun 2025 17:04:00 -0300
Subject: [PATCH 1/3] Files/FileList: add basic tests for the
 FileList::__construct() method

This is just an initial set of tests. It does not fully cover the
FileList::__construct() method.
---
 .../FileList/AbstractFileListTestCase.php     |  55 ++++++++++
 tests/Core/Files/FileList/ConstructTest.php   | 103 ++++++++++++++++++
 tests/Core/Files/FileList/FilterDouble.php    |  31 ++++++
 tests/Core/Files/FileList/Fixtures/file1.php  |   3 +
 tests/Core/Files/FileList/Fixtures/file2.php  |   3 +
 5 files changed, 195 insertions(+)
 create mode 100644 tests/Core/Files/FileList/AbstractFileListTestCase.php
 create mode 100644 tests/Core/Files/FileList/ConstructTest.php
 create mode 100644 tests/Core/Files/FileList/FilterDouble.php
 create mode 100644 tests/Core/Files/FileList/Fixtures/file1.php
 create mode 100644 tests/Core/Files/FileList/Fixtures/file2.php

diff --git a/tests/Core/Files/FileList/AbstractFileListTestCase.php b/tests/Core/Files/FileList/AbstractFileListTestCase.php
new file mode 100644
index 0000000000..abced07317
--- /dev/null
+++ b/tests/Core/Files/FileList/AbstractFileListTestCase.php
@@ -0,0 +1,55 @@
+<?php
+/**
+ * Abstract testcase class for testing FileList methods.
+ *
+ * @copyright 2025 PHPCSStandards Contributors
+ * @license   https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
+ */
+
+namespace PHP_CodeSniffer\Tests\Core\Files\FileList;
+
+use PHP_CodeSniffer\Ruleset;
+use PHP_CodeSniffer\Tests\ConfigDouble;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Base functionality and utilities for testing FileList methods.
+ */
+abstract class AbstractFileListTestCase extends TestCase
+{
+
+    /**
+     * The Config object.
+     *
+     * @var \PHP_CodeSniffer\Config
+     */
+    protected static $config;
+
+    /**
+     * The Ruleset object.
+     *
+     * @var \PHP_CodeSniffer\Ruleset
+     */
+    protected static $ruleset;
+
+
+    /**
+     * Initialize the config and ruleset objects only once.
+     *
+     * @beforeClass
+     *
+     * @return void
+     */
+    public static function initializeConfigAndRuleset()
+    {
+        // Wrapped in an `isset()` as the properties may have been set already (via a call to this method from a dataprovider).
+        if (isset(self::$ruleset) === false) {
+            self::$config         = new ConfigDouble();
+            self::$config->filter = __DIR__.'/FilterDouble.php';
+            self::$ruleset        = new Ruleset(self::$config);
+        }
+
+    }//end initializeConfigAndRuleset()
+
+
+}//end class
diff --git a/tests/Core/Files/FileList/ConstructTest.php b/tests/Core/Files/FileList/ConstructTest.php
new file mode 100644
index 0000000000..1d11638a62
--- /dev/null
+++ b/tests/Core/Files/FileList/ConstructTest.php
@@ -0,0 +1,103 @@
+<?php
+/**
+ * Tests for the \PHP_CodeSniffer\Files\FileList::__construct method.
+ *
+ * @copyright 2025 PHPCSStandards Contributors
+ * @license   https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
+ */
+
+namespace PHP_CodeSniffer\Tests\Core\Files\FileList;
+
+use PHP_CodeSniffer\Files\FileList;
+
+/**
+ * Tests for the \PHP_CodeSniffer\Files\FileList::__construct method.
+ *
+ * @covers \PHP_CodeSniffer\Files\FileList::__construct
+ */
+final class ConstructTest extends AbstractFileListTestCase
+{
+
+
+    /**
+     * Test the __construct() method.
+     *
+     * @param array<string> $files         List of file paths in the Config class.
+     * @param array<string> $expectedFiles List of expected file paths in the FileList.
+     *
+     * @dataProvider dataConstruct
+     *
+     * @return void
+     */
+    public function testConstruct($files, $expectedFiles)
+    {
+        self::$config->files = $files;
+
+        $fileList = new FileList(self::$config, self::$ruleset);
+
+        $this->assertSame(self::$config, $fileList->config, 'Config object mismatch');
+        $this->assertSame(self::$ruleset, $fileList->ruleset, 'Ruleset object mismatch');
+
+        $this->assertCount(count($expectedFiles), $fileList, 'File count mismatch');
+
+        $i = 0;
+
+        // Sort the values to make the tests stable as different OSes will read directories
+        // in a different order and the order is not relevant for these tests. Just the values.
+        $fileListArray = iterator_to_array($fileList);
+        ksort($fileListArray);
+
+        foreach ($fileListArray as $filePath => $fileObject) {
+            $this->assertSame(
+                $expectedFiles[$i],
+                $filePath,
+                sprintf('File path mismatch: expected "%s", got "%s"', $expectedFiles[$i], $filePath)
+            );
+            $this->assertInstanceOf(
+                'PHP_CodeSniffer\Files\File',
+                $fileObject,
+                sprintf('File object for "%s" is not an instance of PHP_CodeSniffer\Files\File', $filePath)
+            );
+            $i++;
+        }
+
+    }//end testConstruct()
+
+
+    /**
+     * Data provider for testConstruct.
+     *
+     * @return array<string, array<string, array<string>>>
+     */
+    public static function dataConstruct()
+    {
+        $fixturesDir = __DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR;
+
+        return [
+            'No files'    => [
+                'files'         => [],
+                'expectedFiles' => [],
+            ],
+            'Two files'   => [
+                'files'         => [
+                    'file1.php',
+                    'file2.php',
+                ],
+                'expectedFiles' => [
+                    'file1.php',
+                    'file2.php',
+                ],
+            ],
+            'A directory' => [
+                'files'         => [$fixturesDir],
+                'expectedFiles' => [
+                    $fixturesDir.'file1.php',
+                    $fixturesDir.'file2.php',
+                ],
+            ],
+        ];
+
+    }//end dataConstruct()
+
+
+}//end class
diff --git a/tests/Core/Files/FileList/FilterDouble.php b/tests/Core/Files/FileList/FilterDouble.php
new file mode 100644
index 0000000000..99952ef63c
--- /dev/null
+++ b/tests/Core/Files/FileList/FilterDouble.php
@@ -0,0 +1,31 @@
+<?php
+/**
+ * Double of the filter class that will accept every file. Used in the FileList tests.
+ *
+ * @copyright 2025 PHPCSStandards Contributors
+ * @license   https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
+ */
+
+namespace PHP_CodeSniffer\Tests\Core\Files\FileList;
+
+use PHP_CodeSniffer\Filters\Filter;
+use ReturnTypeWillChange;
+
+final class FilterDouble extends Filter
+{
+
+
+    /**
+     * Accepts every file.
+     *
+     * @return true
+     */
+    #[ReturnTypeWillChange]
+    public function accept()
+    {
+        return true;
+
+    }//end accept()
+
+
+}//end class
diff --git a/tests/Core/Files/FileList/Fixtures/file1.php b/tests/Core/Files/FileList/Fixtures/file1.php
new file mode 100644
index 0000000000..ddec29a689
--- /dev/null
+++ b/tests/Core/Files/FileList/Fixtures/file1.php
@@ -0,0 +1,3 @@
+<?php
+
+// Empty file for testing purposes.
diff --git a/tests/Core/Files/FileList/Fixtures/file2.php b/tests/Core/Files/FileList/Fixtures/file2.php
new file mode 100644
index 0000000000..ddec29a689
--- /dev/null
+++ b/tests/Core/Files/FileList/Fixtures/file2.php
@@ -0,0 +1,3 @@
+<?php
+
+// Empty file for testing purposes.

From e2400af12f59a9735c956377925cb25e461fde57 Mon Sep 17 00:00:00 2001
From: Rodrigo Primo <rodrigosprimo@gmail.com>
Date: Wed, 25 Jun 2025 17:04:23 -0300
Subject: [PATCH 2/3] Files/FileList: add basic tests for the
 FileList::addFile() method

This is just an initial set of tests. It does not fully cover the
FileList::addFile() method.
---
 tests/Core/Files/FileList/AddFileTest.php | 106 ++++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 tests/Core/Files/FileList/AddFileTest.php

diff --git a/tests/Core/Files/FileList/AddFileTest.php b/tests/Core/Files/FileList/AddFileTest.php
new file mode 100644
index 0000000000..7b4db4144a
--- /dev/null
+++ b/tests/Core/Files/FileList/AddFileTest.php
@@ -0,0 +1,106 @@
+<?php
+/**
+ * Tests for the \PHP_CodeSniffer\Files\FileList::addFile method.
+ *
+ * @copyright 2025 PHPCSStandards Contributors
+ * @license   https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
+ */
+
+namespace PHP_CodeSniffer\Tests\Core\Files\FileList;
+
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Files\FileList;
+use PHP_CodeSniffer\Ruleset;
+use PHP_CodeSniffer\Tests\ConfigDouble;
+
+/**
+ * Tests for the \PHP_CodeSniffer\Files\FileList::addFile method.
+ *
+ * @covers \PHP_CodeSniffer\Files\FileList::addFile
+ */
+final class AddFileTest extends AbstractFileListTestCase
+{
+
+    /**
+     * The FileList object.
+     *
+     * @var \PHP_CodeSniffer\Files\FileList
+     */
+    private $fileList;
+
+
+    /**
+     * Initialize the FileList object.
+     *
+     * @before
+     *
+     * @return void
+     */
+    protected function initializeFileList()
+    {
+        self::$config->files = [];
+        $this->fileList      = new FileList(self::$config, self::$ruleset);
+
+    }//end initializeFileList()
+
+
+    /**
+     * Test adding a file to the list.
+     *
+     * @param string      $fileName   The name of the file to add.
+     * @param object|null $fileObject An optional file object to add instead of creating a new one.
+     *
+     * @dataProvider dataAddFile
+     *
+     * @return void
+     */
+    public function testAddFile($fileName, $fileObject=null)
+    {
+        $this->assertCount(0, $this->fileList);
+
+        $this->fileList->addFile($fileName, $fileObject);
+
+        $fileListArray = iterator_to_array($this->fileList);
+
+        $this->assertCount(1, $this->fileList, 'File count mismatch');
+        $this->assertArrayHasKey($fileName, $fileListArray, 'File not found in list');
+
+        if (isset($fileObject) === true) {
+            $this->assertSame($fileObject, $fileListArray[$fileName], 'File object mismatch');
+        } else {
+            $this->assertInstanceOf(
+                'PHP_CodeSniffer\Files\File',
+                $fileListArray[$fileName],
+                'File object not found in list'
+            );
+        }
+
+    }//end testAddFile()
+
+
+    /**
+     * Data provider for testAddFile.
+     *
+     * @return array<string, array<string, string|object>>
+     */
+    public static function dataAddFile()
+    {
+        self::initializeConfigAndRuleset();
+
+        return [
+            'Regular file'                  => [
+                'fileName' => 'test1.php',
+            ],
+            'STDIN'                         => [
+                'fileName' => 'STDIN',
+            ],
+            'Regular file with file object' => [
+                'fileName'   => 'test1.php',
+                'fileObject' => new File('test1.php', self::$ruleset, self::$config),
+            ],
+        ];
+
+    }//end dataAddFile()
+
+
+}//end class

From fa148a923effec15e0fe3d9bc8917f23115978a2 Mon Sep 17 00:00:00 2001
From: Rodrigo Primo <rodrigosprimo@gmail.com>
Date: Wed, 25 Jun 2025 17:45:29 -0300
Subject: [PATCH 3/3] 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.
---
 src/Files/FileList.php                      |  7 ++++-
 tests/Core/Files/FileList/AddFileTest.php   | 32 +++++++++++++++++++++
 tests/Core/Files/FileList/ConstructTest.php | 25 ++++++++++++++--
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/src/Files/FileList.php b/src/Files/FileList.php
index ab52e33888..48c86c5861 100644
--- a/src/Files/FileList.php
+++ b/src/Files/FileList.php
@@ -92,7 +92,6 @@ public function __construct(Config $config, Ruleset $ruleset)
 
                 foreach ($iterator as $file) {
                     $this->files[$file->getPathname()] = null;
-                    $this->numFiles++;
                 }
             } else {
                 $this->addFile($path);
@@ -100,6 +99,7 @@ public function __construct(Config $config, Ruleset $ruleset)
         }//end foreach
 
         reset($this->files);
+        $this->numFiles = count($this->files);
 
     }//end __construct()
 
@@ -132,6 +132,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++;
         }
diff --git a/tests/Core/Files/FileList/AddFileTest.php b/tests/Core/Files/FileList/AddFileTest.php
index 7b4db4144a..4346fbbf95 100644
--- a/tests/Core/Files/FileList/AddFileTest.php
+++ b/tests/Core/Files/FileList/AddFileTest.php
@@ -103,4 +103,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
diff --git a/tests/Core/Files/FileList/ConstructTest.php b/tests/Core/Files/FileList/ConstructTest.php
index 1d11638a62..f40a400993 100644
--- a/tests/Core/Files/FileList/ConstructTest.php
+++ b/tests/Core/Files/FileList/ConstructTest.php
@@ -74,11 +74,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',
@@ -88,13 +88,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()