[FEATURE] Report exclude entries which did not take effect

README.md

@@ -658,9 +658,37 @@ path to your custom configuration file to the environment variable
 
 The entries are plain directory and file names, matched case-insensitively -
 directories against the beginning of the path, files against the end of the
-filename. Nested directories can be written as they appear on disk
-(`Resources/Private/Build`); slashes escaped as `Resources\/Private\/Build`
-are still accepted and describe the very same directory.
+filename. Nested directories are written as they appear on disk, without a
+trailing slash and without a leading `./`:
+
+```php
+return [
+    'directories' => [
+        'Resources/Private/Build',
+    ],
+    'files' => [
+        'gulpfile.js',
+    ],
+];
+```
+
+Escaping the slashes (`Resources\/Private\/Build`) is still accepted and
+describes the very same directory, but is not required any more.
+
+An entry which did not take effect is reported when the archive is created -
+that is, the archive still contains what the entry names. An exclude entry
+that quietly does nothing is the reason for this: it used to end up in a
+published archive carrying the very directory it was supposed to keep out.
+
+```
+[WARNING] The exclude entry "Resources/Private/Build/" did not take effect, the
+          archive contains "Resources/Private/Build/gulpfile.js". Directory
+          names are matched without a trailing slash, remove it.
+```
+
+Entries for directories and files the extension does not contain are not
+reported. Exclude configurations are usually shared between extensions and
+carry entries only some of them need.
 
 ## Overview of all available commands
 

src/Command/AbstractClientRequestCommand.php

@@ -43,6 +43,9 @@ abstract class AbstractClientRequestCommand extends Command
     /** @var InputInterface */
     protected $input;
 
+    /** @var SymfonyStyle */
+    protected $io;
+
     /** @var HttpClientInterface|null */
     private $httpClient;
 
@@ -66,6 +69,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
     {
         $this->input = $input;
         $io = new SymfonyStyle($input, $output);
+        $this->io = $io;
 
         if ($this->confirmationRequired
             && !$io->askQuestion(new ConfirmationQuestion($this->getMessages()->getConfirmation()))

src/Command/Extension/CreateExtensionArtefactCommand.php

@@ -81,6 +81,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             $versionService->createZipArchiveFromPath(getcwd() ?: './');
         }
 
+        $excludeWarnings = $versionService->getExcludeWarnings();
+
+        if ($excludeWarnings !== []) {
+            $io->warning($excludeWarnings);
+        }
+
         $io->success(sprintf('Extension artefact successfully generated: %s', $versionService->getVersionFilePath()));
 
         return 0;

src/Command/Extension/UploadExtensionVersionCommand.php

@@ -123,6 +123,12 @@ protected function getFormDataPart(array $options): FormDataPart
             $versionService->createZipArchiveFromPath(getcwd() ?: './');
         }
 
+        $excludeWarnings = $versionService->getExcludeWarnings();
+
+        if ($excludeWarnings !== []) {
+            $this->io->warning($excludeWarnings);
+        }
+
         return new FormDataPart([
             'description' => (string)$options['comment'],
             'gplCompliant' => '1',

src/Service/VersionService.php

@@ -38,6 +38,9 @@ class VersionService
     /** @var array */
     protected $excludeConfiguration = [];
 
+    /** @var list<string> The paths the last created archive contains, relative to the extension directory */
+    protected $packagedPaths = [];
+
     public function __construct(string $version, string $extension, string $transactionPath)
     {
         $this->version = $version;
@@ -65,6 +68,7 @@ public function createZipArchiveFromPath(string $path): string
         $zipArchive->open($this->getVersionFilename(), \ZipArchive::CREATE | \ZipArchive::OVERWRITE);
 
         $emConfValidationErrors = [EmConfValidationError::NOT_FOUND];
+        $this->packagedPaths = [];
 
         $iterator = new \RecursiveDirectoryIterator($fullPath, \FilesystemIterator::SKIP_DOTS);
         $files = new \RecursiveIteratorIterator(
@@ -115,7 +119,9 @@ public function createZipArchiveFromPath(string $path): string
             }
 
             // Add the files including their directories
-            $zipArchive->addFile($fileRealPath, substr($fileRealPath, strlen($fullPath) + 1));
+            $packagedPath = substr($fileRealPath, strlen($fullPath) + 1);
+            $this->packagedPaths[] = $packagedPath;
+            $zipArchive->addFile($fileRealPath, $packagedPath);
         }
 
         if ($emConfValidationErrors !== []) {
@@ -144,6 +150,125 @@ protected function quoteExcludePattern(string $excludeEntry): string
         return preg_quote(str_replace('\\/', '/', $excludeEntry), '/');
     }
 
+    /**
+     * Warnings about the configured exclude entries.
+     *
+     * An entry is reported when the archive still carries what the entry names: a
+     * `Resources/Private/Build/` written with a trailing slash reads like a valid
+     * exclude and packages the directory anyway. That silent packaging is what the
+     * filter exists to prevent, so it is reported instead of being guessed straight.
+     *
+     * Entries which name something the extension does not contain are not reported.
+     * Exclude configurations are usually shared between extensions and carry entries
+     * for directories and files only some of them have - the shipped default
+     * configuration most of all.
+     *
+     * @return list<string> The warnings, empty if there is nothing to report
+     */
+    public function getExcludeWarnings(): array
+    {
+        $warnings = [];
+
+        foreach (['directories', 'files'] as $type) {
+            foreach ($this->excludeConfiguration[$type] as $excludeEntry) {
+                $excludeEntry = (string)$excludeEntry;
+
+                if (str_contains($excludeEntry, '\\/')) {
+                    $warnings[] = sprintf(
+                        'The exclude entry "%s" escapes its slashes. This is no longer required, write it as "%s".',
+                        $excludeEntry,
+                        str_replace('\\/', '/', $excludeEntry)
+                    );
+                }
+
+                $packagedPath = $this->getPackagedPathFor($type, $excludeEntry);
+
+                if ($packagedPath === '') {
+                    continue;
+                }
+
+                $warnings[] = trim(sprintf(
+                    'The exclude entry "%s" did not take effect, the archive contains "%s". %s',
+                    $excludeEntry,
+                    $packagedPath,
+                    $this->getExcludeEntryHint($type, $excludeEntry)
+                ));
+            }
+        }
+
+        return $warnings;
+    }
+
+    /**
+     * Find a packaged path the given exclude entry names but did not keep out of the
+     * archive. Since the entry is compared against what was packaged, an entry which
+     * is covered by another one - `Resources/Private` next to `Resources/Private/Build` -
+     * has nothing left to report.
+     *
+     * @param string $type         Either `directories` or `files`
+     * @param string $excludeEntry The configured directory or file name
+     *
+     * @return string The packaged path, empty if the entry has nothing to complain about
+     */
+    protected function getPackagedPathFor(string $type, string $excludeEntry): string
+    {
+        $entryPath = trim(str_replace(['\\/', '\\'], '/', $excludeEntry), '/');
+
+        if (str_starts_with($entryPath, './')) {
+            $entryPath = substr($entryPath, 2);
+        }
+
+        if ($entryPath === '') {
+            return '';
+        }
+
+        // A file entry is matched against the filename, a path in it can never match
+        if ($type === 'files' && !str_contains($entryPath, '/')) {
+            return '';
+        }
+
+        foreach ($this->packagedPaths as $packagedPath) {
+            if ($type === 'files' && strcasecmp($packagedPath, $entryPath) === 0) {
+                return $packagedPath;
+            }
+
+            if ($type === 'directories' && stripos($packagedPath, $entryPath . '/') === 0) {
+                return $packagedPath;
+            }
+        }
+
+        return '';
+    }
+
+    /**
+     * Hint about the most likely reason for an exclude entry not to take effect.
+     *
+     * @param string $type         Either `directories` or `files`
+     * @param string $excludeEntry The configured directory or file name
+     *
+     * @return string The hint, empty if the entry looks the way it is documented
+     */
+    protected function getExcludeEntryHint(string $type, string $excludeEntry): string
+    {
+        if ($type === 'files') {
+            return 'File entries are matched against the filename, they can not contain a path.';
+        }
+
+        if (str_ends_with($excludeEntry, '/')) {
+            return 'Directory names are matched without a trailing slash, remove it.';
+        }
+
+        if (str_starts_with($excludeEntry, './')) {
+            return 'Directory names are matched relative to the extension directory, remove the leading "./".';
+        }
+
+        if (str_contains(str_replace('\\/', '/', $excludeEntry), '\\')) {
+            return 'Use "/" as directory separator.';
+        }
+
+        return '';
+    }
+
     /**
      * Extract the given artefact (from either local or remote),
      * store it in a temporary transaction path and finally call

tests/Unit/Command/Extension/CreateExtensionArtefactCommandTest.php

@@ -104,6 +104,24 @@ public function excludedFilesAreNotPackaged(): void
         self::assertNotContains('vendor/autoload.php', $names);
     }
 
+    #[Test]
+    public function ineffectiveExcludeEntryIsReported(): void
+    {
+        $this->writeExtensionFile('Resources/Private/Build/gulpfile.js', '// build only');
+        $this->setEnvironment([
+            'TYPO3_EXCLUDE_FROM_PACKAGING' => __DIR__ . '/../../Fixtures/ExcludeFromPackaging/config_ineffective_directory.php',
+        ]);
+
+        $tester = $this->tester();
+        $tester->execute([
+            'version' => '1.2.3',
+            'extensionkey' => 'my_ext',
+            '--path' => $this->extensionDirectory,
+        ]);
+
+        self::assertDisplayContains('The exclude entry "Resources/Private/Build/" did not take effect', $tester);
+    }
+
     #[Test]
     public function versionMismatchInEmConfIsRejected(): void
     {

tests/Unit/Command/Extension/UploadExtensionVersionCommandTest.php

@@ -110,6 +110,20 @@ public function transactionDirectoryIsRemovedAfterwards(): void
         self::assertDirectoryDoesNotExist($this->workingDirectory . '/tailor-version-upload');
     }
 
+    #[Test]
+    public function ineffectiveExcludeEntryIsReportedBeforeTheUpload(): void
+    {
+        $this->writeExtensionFile('Resources/Private/Build/gulpfile.js', '// build only');
+        $this->setEnvironment([
+            'TYPO3_EXCLUDE_FROM_PACKAGING' => __DIR__ . '/../../Fixtures/ExcludeFromPackaging/config_ineffective_directory.php',
+        ]);
+
+        $tester = $this->apiTester($this->command(), self::jsonResponse([], 201));
+
+        self::assertSame(0, $tester->execute($this->uploadArguments()));
+        self::assertDisplayContains('The exclude entry "Resources/Private/Build/" did not take effect', $tester);
+    }
+
     #[Test]
     public function failingRequestReturnsFailure(): void
     {

tests/Unit/Fixtures/ExcludeFromPackaging/config_absent_directory.php

@@ -0,0 +1,11 @@
+<?php
+
+return [
+    'directories' => [
+        'Tests',
+        'Documentation',
+    ],
+    'files' => [
+        'phpstan.neon',
+    ],
+];

tests/Unit/Fixtures/ExcludeFromPackaging/config_covered_directory.php

@@ -0,0 +1,9 @@
+<?php
+
+return [
+    'directories' => [
+        'Resources/Private',
+        'Resources/Private/Build',
+    ],
+    'files' => [],
+];

tests/Unit/Fixtures/ExcludeFromPackaging/config_ineffective_directory.php

@@ -0,0 +1,8 @@
+<?php
+
+return [
+    'directories' => [
+        'Resources/Private/Build/',
+    ],
+    'files' => [],
+];

tests/Unit/Fixtures/ExcludeFromPackaging/config_ineffective_file.php

@@ -0,0 +1,8 @@
+<?php
+
+return [
+    'directories' => [],
+    'files' => [
+        'Resources/Private/Build/gulpfile.js',
+    ],
+];