[DockerComposeConfigurator] Fix duplicate top-level key on reconfigure

`composer recipes:install --reset --force` re-runs configure on an already-marked
compose.yaml. For any recipe that updates a non-last top-level section (e.g. a
recipe contributing only `services:` on a file that also has `volumes:`), this
produced a duplicate `volumes:` line, yielding invalid YAML.

Two interacting bugs:

1. The parser accumulated the transitioning key line under the OLD `$node` (it's
   added to `$nodesLines[$node][$i]` before the node-transition logic runs). When
   that section's content was later spliced back in, the next section's header
   traveled with the replacement, while the original header in `$lines` wasn't
   removed by the splice range.

2. The position-adjustment math after `array_splice` used `$at - $length - 1`
   for `startAt` and `$at - $length` for `endAt`. Since splice removes `$length`
   items and inserts 1 (the replacement string cast to a one-element array), the
   correct shift is `-($length - 1)`.

The two bugs masked each other in normal flows, which is why existing tests pass
on either side individually but the regression only appears with both fixed
together. Fixed together, with a regression test that fails on `2.x` without
the source change.

Author: mistyfiky

src/Configurator/DockerComposeConfigurator.php

@@ -40,7 +40,7 @@ public function __construct(Composer $composer, IOInterface $io, Options $option
         $this->filesystem = new Filesystem();
     }
 
-    public function configure(Recipe $recipe, $config, Lock $lock, array $options = [])
+    public function configure(Recipe $recipe, $config, Lock $lock, array $options = []): void
     {
         if (!self::shouldConfigureDockerRecipe($this->composer, $this->io, $recipe)) {
             return;
@@ -51,7 +51,7 @@ public function configure(Recipe $recipe, $config, Lock $lock, array $options =
         $this->write('Docker Compose definitions have been modified. Please run "docker compose up --build" again to apply the changes.');
     }
 
-    public function unconfigure(Recipe $recipe, $config, Lock $lock)
+    public function unconfigure(Recipe $recipe, $config, Lock $lock): void
     {
         $rootDir = $this->options->get('root-dir');
         foreach ($this->normalizeConfig($config) as $file => $extra) {
@@ -276,6 +276,13 @@ private function configureDockerCompose(Recipe $recipe, array $config, bool $upd
 
                 // Keep end in memory (check break line on previous line)
                 $endAt[$node] = !$i || '' !== trim($lines[$i - 1]) ? $i : $i - 1;
+                // Prune nodesLines[old node] to match the range the splice will replace.
+                if (isset($nodesLines[$node][$i])) {
+                    unset($nodesLines[$node][$i]);
+                    if ('' === trim($lines[$i - 1])) {
+                        unset($nodesLines[$node][$i - 1]);
+                    }
+                }
                 $node = $matches[1];
                 if (!isset($nodesLines[$node])) {
                     $nodesLines[$node] = [];
@@ -303,14 +310,15 @@ private function configureDockerCompose(Recipe $recipe, array $config, bool $upd
                     array_splice($lines, $startAt[$key], $length, ltrim($updatedContents, "\n"));
 
                     // reset any start/end positions after this to the new positions
+                    $shift = $length - 1;
                     foreach ($startAt as $sectionKey => $at) {
                         if ($at > $originalEndAt) {
-                            $startAt[$sectionKey] = $at - $length - 1;
+                            $startAt[$sectionKey] = $at - $shift;
                         }
                     }
                     foreach ($endAt as $sectionKey => $at) {
                         if ($at > $originalEndAt) {
-                            $endAt[$sectionKey] = $at - $length;
+                            $endAt[$sectionKey] = $at - $shift;
                         }
                     }
 
@@ -386,7 +394,7 @@ private static function askDockerSupport(IOInterface $io, Recipe $recipe): strin
 
         return $io->askAndValidate(
             $question,
-            function ($value) {
+            static function ($value) {
                 if (null === $value) {
                     return 'y';
                 }

tests/Configurator/DockerComposeConfiguratorTest.php

@@ -188,7 +188,7 @@ public static function dockerComposerFileProvider(): iterable
     /**
      * @dataProvider dockerComposerFileProvider
      */
-    public function testConfigure(string $fileName)
+    public function testConfigure(string $fileName): void
     {
         $dockerComposeFile = FLEX_TEST_DIR."/$fileName";
         file_put_contents($dockerComposeFile, self::ORIGINAL_CONTENT);
@@ -224,7 +224,38 @@ public function testConfigure(string $fileName)
         $this->assertEquals(self::ORIGINAL_CONTENT, file_get_contents($dockerComposeFile));
     }
 
-    public function testNotConfiguredIfConfigSet()
+    /**
+     * Re-configuring a recipe that only touches a non-last top-level key (e.g. `services`)
+     * on a file that also has a `volumes:` key must not duplicate the `volumes:` line.
+     * This is what happens during `composer recipes:install --reset --force`.
+     */
+    public function testReconfigureDoesNotDuplicateLaterTopLevelKey(): void
+    {
+        // Recipe that contributes BOTH services and volumes (sets up the file shape).
+        $this->configurator->configure($this->recipeDb, self::CONFIG_DB, $this->lock);
+
+        // Recipe that only contributes `services` (e.g. shopware/docker-dev, symfony/mailer).
+        $servicesOnlyRecipe = $this->getMockBuilder(Recipe::class)->disableOriginalConstructor()->getMock();
+        $servicesOnlyRecipe->method('getName')->willReturn('acme/services-only');
+        $servicesOnlyConfig = [
+            'services' => [
+                'cache:',
+                '  image: redis:alpine',
+            ],
+        ];
+
+        $this->configurator->configure($servicesOnlyRecipe, $servicesOnlyConfig, $this->lock);
+        $afterFirstConfigure = file_get_contents(FLEX_TEST_DIR.'/compose.yaml');
+
+        // Simulate `recipes:install --reset --force` re-running configure on the marked file.
+        $this->configurator->configure($servicesOnlyRecipe, $servicesOnlyConfig, $this->lock, ['force' => true]);
+        $afterSecondConfigure = file_get_contents(FLEX_TEST_DIR.'/compose.yaml');
+
+        $this->assertSame(1, substr_count($afterSecondConfigure, "\nvolumes:\n"), 'compose.yaml must contain exactly one top-level "volumes:" key');
+        $this->assertSame($afterFirstConfigure, $afterSecondConfigure, 'configure must be idempotent when re-run');
+    }
+
+    public function testNotConfiguredIfConfigSet(): void
     {
         $this->package->setExtra(['symfony' => ['docker' => false]]);
         $this->configurator->configure($this->recipeDb, self::CONFIG_DB, $this->lock);
@@ -235,7 +266,7 @@ public function testNotConfiguredIfConfigSet()
     /**
      * @dataProvider getInteractiveDockerPreferenceTests
      */
-    public function testPreferenceAskedInteractively(string $userInput, bool $expectedIsConfigured, bool $expectedIsComposerJsonUpdated)
+    public function testPreferenceAskedInteractively(string $userInput, bool $expectedIsConfigured, bool $expectedIsComposerJsonUpdated): void
     {
         $composerJsonPath = FLEX_TEST_DIR.'/composer.json';
         file_put_contents($composerJsonPath, json_encode(['name' => 'test/app']));
@@ -270,7 +301,7 @@ public function getInteractiveDockerPreferenceTests()
         yield 'no_forever' => ['x', false, true];
     }
 
-    public function testEnvVarUsedForDockerConfirmation()
+    public function testEnvVarUsedForDockerConfirmation(): void
     {
         $composerJsonPath = FLEX_TEST_DIR.'/composer.json';
         file_put_contents($composerJsonPath, json_encode(['name' => 'test/app']));
@@ -290,7 +321,7 @@ public function testEnvVarUsedForDockerConfirmation()
         $this->assertTrue($composerJsonData['extra']['symfony']['docker']);
     }
 
-    public function testConfigureFileWithExistingVolumes()
+    public function testConfigureFileWithExistingVolumes(): void
     {
         $originalContent = self::ORIGINAL_CONTENT.<<<'YAML'
 
@@ -341,7 +372,7 @@ public function testConfigureFileWithExistingVolumes()
         );
     }
 
-    public function testConfigureFileWithExistingMarks()
+    public function testConfigureFileWithExistingMarks(): void
     {
         $originalContent = self::ORIGINAL_CONTENT.<<<'YAML'
 
@@ -435,7 +466,7 @@ public function testConfigureFileWithExistingMarks()
         $this->assertEquals(self::ORIGINAL_CONTENT, file_get_contents($dockerComposeFile));
     }
 
-    public function testUnconfigureFileWithManyMarks()
+    public function testUnconfigureFileWithManyMarks(): void
     {
         $originalContent = self::ORIGINAL_CONTENT.<<<'YAML'
 
@@ -507,7 +538,7 @@ public function testUnconfigureFileWithManyMarks()
         $this->assertStringEqualsFile($dockerComposeFile, $contentWithoutDoctrine);
     }
 
-    public function testConfigureMultipleFiles()
+    public function testConfigureMultipleFiles(): void
     {
         $dockerComposeFile = FLEX_TEST_DIR.'/docker-compose.yml';
         file_put_contents($dockerComposeFile, self::ORIGINAL_CONTENT);
@@ -548,7 +579,7 @@ public function testConfigureMultipleFiles()
         $this->assertStringEqualsFile($dockerComposeOverrideFile, self::ORIGINAL_CONTENT);
     }
 
-    public function testConfigureEnvVar()
+    public function testConfigureEnvVar(): void
     {
         @mkdir(FLEX_TEST_DIR.'/child/');
         $dockerComposeFile = FLEX_TEST_DIR.'/child/docker-compose.yml';
@@ -593,7 +624,7 @@ public function testConfigureEnvVar()
         $this->assertStringEqualsFile($dockerComposeOverrideFile, self::ORIGINAL_CONTENT);
     }
 
-    public function testConfigureFileInParentDir()
+    public function testConfigureFileInParentDir(): void
     {
         $this->configurator = new DockerComposeConfigurator(
             $this->composer,
@@ -636,7 +667,7 @@ public function testConfigureFileInParentDir()
         $this->assertEquals(self::ORIGINAL_CONTENT, file_get_contents($dockerComposeFile));
     }
 
-    public function testConfigureWithoutExistingDockerComposeFiles()
+    public function testConfigureWithoutExistingDockerComposeFiles(): void
     {
         $dockerComposeFile = FLEX_TEST_DIR.'/compose.yaml';
 
@@ -672,7 +703,7 @@ public function testConfigureWithoutExistingDockerComposeFiles()
         $this->assertEquals('', file_get_contents($dockerComposeFile));
     }
 
-    public function testUpdate()
+    public function testUpdate(): void
     {
         $recipe = $this->createMock(Recipe::class);
         $recipe->method('getName')