Skip to content

Commit 39afa23

Browse files
committed
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.
1 parent eb8c379 commit 39afa23

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

tests/Core/Files/FileList/ConstructTest.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ public function testConstruct($files, $expectedFiles)
9797
*/
9898
public static function dataConstruct()
9999
{
100+
$fixturesDir = __DIR__.DIRECTORY_SEPARATOR.'Fixtures'.DIRECTORY_SEPARATOR;
101+
100102
return [
101103
'No files' => [
102104
'files' => [],
@@ -113,10 +115,10 @@ public static function dataConstruct()
113115
],
114116
],
115117
'A directory' => [
116-
'files' => [__DIR__.'/Fixtures'],
118+
'files' => [$fixturesDir],
117119
'expectedFiles' => [
118-
__DIR__.'/Fixtures/file1.php',
119-
__DIR__.'/Fixtures/file2.php',
120+
$fixturesDir.'file1.php',
121+
$fixturesDir.'file2.php',
120122
],
121123
],
122124
'Same file twice' => [
@@ -130,12 +132,12 @@ public static function dataConstruct()
130132
],
131133
'File and then directory containing that file' => [
132134
'files' => [
133-
__DIR__.'/Fixtures/file1.php',
134-
__DIR__.'/Fixtures',
135+
$fixturesDir.'file1.php',
136+
$fixturesDir,
135137
],
136138
'expectedFiles' => [
137-
__DIR__.'/Fixtures/file1.php',
138-
__DIR__.'/Fixtures/file2.php',
139+
$fixturesDir.'file1.php',
140+
$fixturesDir.'file2.php',
139141
],
140142
],
141143
];

0 commit comments

Comments
 (0)